Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure $DISPLAY exists for xsel or xclip + add cygwin /dev/clipboard … #70

Closed

Conversation

rfairburn
Copy link

…support.

xsel and xclip both exist for cygwin when cygwin/X is installed, but it is much less common to have the X server running. Ensure that the $DISPLAY environment variable is set when determining which command to pick.

Additionally, cygwin has a built-in virtual device called /dev/clipboard that we can leverage instead of requiring putclip to be installed. Added support for that.

If you would like the test for $DISPLAY to be handled in another manner, I am open to editing this PR to meet those needs, but this worked in both my linux and windows/cygwin environment after the changes as is.

@lafrenierejm
Copy link

The additions of $DISPLAY and cygwin support should two separate commits.

@lafrenierejm
Copy link

PR #58 implements tests for $DISPLAY.

Support still exists for the following if desired:
- `putclip` command <br />
Get the command by installing `cygutils-extra` package with Cygwin's
`setup*.exe`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something simpler like:

If you have `putclip` (from the `cygutils-extra` package) then that will be used.

Otherwise it will fall back to using `/dev/clipboard`.

@@ -110,14 +110,16 @@ clipboard_copy_command() {
else
echo "pbcopy"
fi
elif command_exists "xclip"; then
elif command_exists "xclip" && [ -n "$DISPLAY" ]; then # only works if $DISPLAY set
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove. PR #58 handles this.

local xclip_selection="$(yank_selection)"
echo "xclip -selection $xclip_selection"
elif command_exists "xsel"; then
elif command_exists "xsel" && [ -n "$DISPLAY" ]; then # also required $DISPLAY
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove. PR #58 handles this.

@docwhat docwhat added the bug Something isn't working label May 6, 2017
@docwhat docwhat added this to the 2.3.0 milestone May 6, 2017
@rfairburn rfairburn closed this May 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants