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

Improve clipboard provider #4150

Closed
wants to merge 1 commit into from
Closed

Improve clipboard provider #4150

wants to merge 1 commit into from

Conversation

Shougo
Copy link
Contributor

@Shougo Shougo commented Feb 2, 2016

  • User can disable clipboard provider by g:loaded_clipboard_provider
  • Check $DISPLAY exists
  • xsel has higher priority than xclip. Because, it is newer

@justinmk
Copy link
Member

justinmk commented Feb 2, 2016

How about g:loaded_provider_clipboard as the convention?

let s:copy['+'] = 'xclip -quiet -i -selection clipboard'
let s:paste['+'] = 'xclip -o -selection clipboard'
let s:copy['*'] = 'xclip -quiet -i -selection primary'
let s:paste['*'] = 'xclip -o -selection primary'
Copy link
Member

Choose a reason for hiding this comment

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

👍 to this change

@Shougo
Copy link
Contributor Author

Shougo commented Feb 3, 2016

OK. I have updated.

finish
endif

let s:loaded_provider_clipboard = 1
Copy link
Member

Choose a reason for hiding this comment

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

For reason mentioned here: #4150 (diff) I think we should not do this. What happens when user sets this and sets clipboard? Too confusing.

We should instead make the message less annoying. I'm working on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not g:loaded_provider_clipboard.
It is just script local variable.

It is like in pythonx.vim.

Copy link
Member

Choose a reason for hiding this comment

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

Why would that be needed for an autoload/ script?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. It is not needed.

@bfredl
Copy link
Member

bfredl commented Feb 4, 2016

I want to remember, that a concern was that nvim could outlive a specific X11 session when running in tmux so $DISPLAY could change, but as long as nvim is started inside X11 whenever one intends to eventually use the X11 clipboard this should still be fine.

The reason for favouring xclip was that it had slightly better NUL semantics, which was a reported consern when implementing #1182, but personally I don't have any strong opinions of this.

so LGTM for my part 👍

justinmk pushed a commit that referenced this pull request Feb 5, 2016
@justinmk
Copy link
Member

justinmk commented Feb 5, 2016

Merged without g:loaded_clipboard_provider

@justinmk justinmk closed this Feb 5, 2016
@Shougo Shougo deleted the clipboard branch February 6, 2016 02:18
@Shougo
Copy link
Contributor Author

Shougo commented Feb 6, 2016

OK, thanks.

justinmk added a commit that referenced this pull request Feb 9, 2016
Features:
  ef66249 tabline: Add %[] atom to the tabline, for random commands on click
  f338ea7 job control: implement jobpid() to get PID of job
  d0d5d17 job control: add 'detach' option to jobstart
  7ad3f07 Add support for binary numbers

Fixes:
  291495a regexp_nfa.c: Speed up find_match_text()
  317d5ca input: Do not set high-bit; preserve ALT modifier.
  3b7c409 shell: Unquote &shell* options before using them

Notable changes:
  49b06a8 encoding: Always use "utf-8" as default for &encoding
  79a6983 ui: revert "gui_running" hack

Other changes:
  b4b4536 version: semver.org compliance
  c6aa716 reproducible builds: Stop using __{DATE,TIME}__
  46bd3c0 clipboard: Check $DISPLAY. Prefer xsel. #4150
  f6ecd12 job control: don't kill PTY processes on exit
  49f0417 clipboard: Detach clipboard helper, so contents is kept after nvim exit
  38435e8 python: Add missing I/O methods to RedirectStream
  d26b01d eval: Use better error messages when failing to dump values
  62d137c Remove swapsync.
@blueyed
Copy link
Contributor

blueyed commented Oct 27, 2017

@Shougo

xsel has higher priority than xclip. Because, it is newer

Can you elaborate on that, please?

As per #5853 (comment) it seems that xclip is newer / better maintained?!

justinmk added a commit to justinmk/neovim that referenced this pull request Dec 1, 2018
The order was swapped in neovim#4150 to prefer `xsel` but there wasn't a clear
explanation.

Let's trying preferring `xclip` again, we've had a few reports of
problems with `xsel`.

closes neovim#7237
@justinmk justinmk mentioned this pull request Dec 1, 2018
justinmk added a commit to justinmk/neovim that referenced this pull request Dec 1, 2018
The order was swapped in neovim#4150 to prefer `xsel` but there wasn't a clear
explanation.  Meanwhile, `xsel` has been neglected upstream.

Let's trying preferring `xclip` again, we've had a few reports of
problems with `xsel`.

closes neovim#7237
ref neovim#5853
ref neovim#7449
justinmk added a commit that referenced this pull request Dec 1, 2018
The order was swapped in #4150 to prefer `xsel` but there wasn't a clear
explanation.  Meanwhile, `xsel` has been neglected upstream.

Let's trying preferring `xclip` again, we've had a few reports of
problems with `xsel`.

closes #7237
ref #5853
ref #7449
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants