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

Death of clipboard #3061

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@faho
Member

faho commented May 24, 2016

This removes our clipboard integration and replaces it with a set of explicit binding functions.

It should work on OSX via pbcopy/pbpaste (please test!) and on X11 via xsel.

I've explicitly not silenced the errors xsel throws when $DISPLAY is invalid so the user knows that it's not going to work.

This should fix #2894 and #2556 and provide a workaround for #2188.


Tasks:

  • Update release notes (next-2x)
  • Update documentation

@faho faho added the enhancement label May 24, 2016

faho added some commits May 13, 2016

kill: Remove xsel integration
Overwriting the user's clipboard by default is annoying and contributors
don't use it.

This is better served via an explicit binding that calls e.g. `xsel`.
@@ -0,0 +1,7 @@
function fish_clipboard_copy
if type -q pbcopy

This comment has been minimized.

@zanchey

zanchey May 24, 2016

Member

I don't know if it's premature optimisation, but another option would be to use

if type -q pbcopy
function fish_clipboard_copy
   commandline | pbcopy
end
else if type -q xsel
...
@zanchey

zanchey May 24, 2016

Member

I don't know if it's premature optimisation, but another option would be to use

if type -q pbcopy
function fish_clipboard_copy
   commandline | pbcopy
end
else if type -q xsel
...

This comment has been minimized.

@faho

faho May 25, 2016

Member

Yeah, this isn't performance-sensitive at all, and the bulk of the time is likely to be spent in the called command (xsel needs a trip to the X server and back).

Plus it would mean what you get via funced differs from the contents of the file and I like being able to use that for development. It's possible there's more tools to support here (there's also xclip, but I don't know if anything has it but not xsel), so it'd be nice to leave that possibility open.

@faho

faho May 25, 2016

Member

Yeah, this isn't performance-sensitive at all, and the bulk of the time is likely to be spent in the called command (xsel needs a trip to the X server and back).

Plus it would mean what you get via funced differs from the contents of the file and I like being able to use that for development. It's possible there's more tools to support here (there's also xclip, but I don't know if anything has it but not xsel), so it'd be nice to leave that possibility open.

@zanchey

This comment has been minimized.

Show comment
Hide comment
@zanchey

zanchey May 24, 2016

Member

Weirdly, the binding only seems to work every second time on OS X in iTerm 2:

ls<C-y><C-v><C-v>ls<C-v><C-v>ls
Member

zanchey commented May 24, 2016

Weirdly, the binding only seems to work every second time on OS X in iTerm 2:

ls<C-y><C-v><C-v>ls<C-v><C-v>ls
@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 May 24, 2016

Contributor

A quick read didn't reveal any obvious problems. I was just about to tackle this myself. I've been pretty annoyed that every time I use my mouse to highlight and copy some text to paste into a command if I first do something like invoke the delete-word binding it replaces my explicit selection and I have to select that text a second time. So thanks for beating me to it. I'll test it in a couple of hours.

Contributor

krader1961 commented May 24, 2016

A quick read didn't reveal any obvious problems. I was just about to tackle this myself. I've been pretty annoyed that every time I use my mouse to highlight and copy some text to paste into a command if I first do something like invoke the delete-word binding it replaces my explicit selection and I have to select that text a second time. So thanks for beating me to it. I'll test it in a couple of hours.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 May 24, 2016

Contributor

@zanchey: The weird need to press \cV twice is because the tty driver is in cooked mode and has "lnext = ^V". The fix is to undef that character like we already do for \cS (stop) and \cQ (start). In fact, we should undef all the characters except those for "kill", "susp", "intr", "quit" and "eof". I'll open an issue and fix that.

Contributor

krader1961 commented May 24, 2016

@zanchey: The weird need to press \cV twice is because the tty driver is in cooked mode and has "lnext = ^V". The fix is to undef that character like we already do for \cS (stop) and \cQ (start). In fact, we should undef all the characters except those for "kill", "susp", "intr", "quit" and "eof". I'll open an issue and fix that.

@krader1961

This comment has been minimized.

Show comment
Hide comment
@krader1961

krader1961 May 25, 2016

Contributor

Other than the weird \cV behavior, which I'll fix in a separate change, this LGTM.

Contributor

krader1961 commented May 25, 2016

Other than the weird \cV behavior, which I'll fix in a separate change, this LGTM.

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho May 25, 2016

Member

Merged with rebase as 2f51088..53c506f.

Member

faho commented May 25, 2016

Merged with rebase as 2f51088..53c506f.

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jul 3, 2016

Member

I notice there is no mention of pbcopy/pbpaste/xsel working out of the box - it's neat and should be noted in documentation.

Member

floam commented Jul 3, 2016

I notice there is no mention of pbcopy/pbpaste/xsel working out of the box - it's neat and should be noted in documentation.

@floam floam added the docs label Jul 3, 2016

@zanchey zanchey modified the milestones: next-2.x, 2.3.1 Jul 3, 2016

@faho

This comment has been minimized.

Show comment
Hide comment
@faho

faho Jul 3, 2016

Member

@floam: f9edcbb mentions the bindings. Do you think we need more?

Member

faho commented Jul 3, 2016

@floam: f9edcbb mentions the bindings. Do you think we need more?

@floam floam removed the docs label Jul 3, 2016

@floam

This comment has been minimized.

Show comment
Hide comment
@floam

floam Jul 3, 2016

Member

Nope. I missed that. Only noticed the documentation removal of the old stuff.

Member

floam commented Jul 3, 2016

Nope. I missed that. Only noticed the documentation removal of the old stuff.

@faho faho modified the milestones: fish 2.4.0, next-2.x Sep 4, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment