[PATCH] Allow running clipboard commands if DISPLAY is not set

[devzero]

Hello!

I ran into a restriction related to the options clipboard_store and clipboard_paste.

I'm currently working on a script that brings a transparent clipboard support to any application that supports executing arbitrary commands on the copy and paste actions in the same way as mc does. It also works for applications running on a virtual terminal and falls back to the file-based clipboard storage if an X server is not running or cannot be detected.

Here is the script:
​https://github.com/sde-gui/sde-common-clipboard/blob/master/sde-common-clipboard

I noticed that mc refuses running clipboard_store and clipboard_paste commands if the DISPLAY environment variable is not set. This limitation didn't seem to be explicitly stated in the documentation (especially in the russian manual page -- it lacks any mention of X11 regarding clipboard at all), so I had to check the source code out to make sure that was the case.

My suggestion is to bring a new configuration variable controlling this behavior. The attached patch implements a new option clipboard_force_no_display allowing to override the default behavior.

[andrew_b]

What about get rid of $DISPLAY usage here at all instead of adding yet another micro-option?

[ossi]

the concerns are:

• backwards compatibility of the configurations. that's somewhat minor, as an affected user would somewhat easily spot the problem.
• making the common case more complicated. this could be alleviated by mc shipping a wrapper for xclip itself.
• given that this would be a hidden option that configures two other hidden options, it isn't all that bad to start with. arguably, the usability problem is that this needs such low-level configuration to start with. otoh, the reporter would be certainly unhappy if it wasn't that way ...

[devzero]

Replying to andrew_b:

What about get rid of $DISPLAY usage here at all instead of adding yet another micro-option?

clipboard_file_from_ext_clip() should be reworked in this case.

Running with:

clipboard_store=xclip -i
clipboard_paste=xclip -o

with DISPLAY unset, or just with any failing command:

clipboard_store=false
clipboard_paste=false

breaks the mc clipboard.

If this bug is fixed and there are no other hidden reasons for checking DISPLAY, it seems better to throw the check away.

[andrew_b]

Replying to devzero:

Running with:
[...]
with DISPLAY unset
[...]
breaks the mc clipboard.

Correct. Expected that DISPLAY is set appropriately.

If this bug is fixed

Why this is a bug?

​Look at the vim. It is also requires the DISPLAY variable:

---

DISPLAY environment variable on Linux systems

Make sure your DISPLAY environment variable is set appropriately - otherwise vim can not connect to your x-session to access the clipboard.

---

[ossi]

ehm, i kind of expect mc's clipboard to work on a plain console.

Look at the vim. It is also requires the DISPLAY variable:

yes - when you explicitly ask for the x11 clipboard.

[angel_il]

mc's clipboard is work with global clipboard (X-clipboard) throuth 'xclip' if DISPLAY is set, and local (editor, fields, command line) throuth mc-clip-file.

[devzero]

Why this is a bug?

Huh?

You just suggested "What about get rid of $DISPLAY usage here at all", so how the application will be able to detect if it should use the data returned by clipboard_paste or not, in this case?

In the current implementation, the application rewrites the clipboard data in clipboard_file_from_ext_clip() without checking the exit status of clipboard_paste, and I consider this behaviour as a bug. For example, if a user has not xclip installed or xclip invocation failed for any reason, any paste operation is effectively noop in mcedit, instead of falling back to the internal clipboard implementation.

If you throw away the DISPLAY check, such a rather rare condition will turn into a common case. So clipboard_file_from_ext_clip() should be fixed for checking the non-zero exit status of clipboard_paste before deleting the DISPLAY check.

Please point out why using any data returned by a failed shell command is NOT a bug in your opinion.

[devzero]

Replying to andrew_b:

Why this is a bug?

Sorry for the misinformation. Looks like something was wrong in my configuration the last night, when I detected the "bug". I double checked it now.

You are right, the check for DISPLAY seems unneeded and the deletion of the check seems to have no visible effect on the application functioning.

[andrew_b]

Branch: 4133_clipboard_no_display
changeset:cf29517d41ee11a0dd4b5142b8a1b8c508f5b5c4

[andrew_b]

Merged to master: [2225af84f5579ed2c904005e63d61ff743d82041].