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

g:clipboard ineffective after has('unnamedplus') #8017

Closed
agriffis opened this issue Feb 16, 2018 · 9 comments
Closed

g:clipboard ineffective after has('unnamedplus') #8017

agriffis opened this issue Feb 16, 2018 · 9 comments
Labels
clipboard clipboard, paste complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md enhancement feature request
Milestone

Comments

@agriffis
Copy link

  • nvim --version:
NVIM v0.2.3-590-g0851057a8
Build type: RelWithDebInfo
LuaJIT 2.0.5
Compilation: /usr/bin/gcc-5 -Wconversion -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -DNVIM_MSGPACK_HAS_FLOAT32 -DNVIM_UNIBI_HAS_VAR_FROM -O2 -g -DMIN_LOG_LEVEL=3 -Og -g -Wall -Wextra -pedantic -Wno-unused-parameter -Wstrict-prototypes -std=gnu99 -Wvla -fstack-protector-strong -fdiagnostics-color=auto -Wno-array-bounds -DINCLUDE_GENERATED_DECLARATIONS -D_GNU_SOURCE -I/home/travis/build/neovim/bot-ci/build/neovim/build/config -I/home/travis/build/neovim/bot-ci/build/neovim/src -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/.deps/usr/include -I/usr/include -I/home/travis/build/neovim/bot-ci/build/neovim/build/src/nvim/auto -I/home/travis/build/neovim/bot-ci/build/neovim/build/include
Compiled by travis@travis-job-8e635ef7-4bdc-43d6-b064-e4a9149598e9

Features: +acl +iconv +jemalloc +tui
See ":help feature-compile"

   system vimrc file: "$VIM/sysinit.vim"
  fall-back for $VIM: "
/home/travis/build/neovim/bot-ci/build/neovim/build/nvim.AppDir/usr/share/nvim
"

Run :checkhealth for more info
  • Vim (version: ) behaves differently?

N/A since Vim doesn't support clipboard providers

  • Operating system/version:

Fedora 26

  • Terminal name/version:

tmux-256color

Good behavior

nvim -u NORC
:let g:clipboard = {'name': 'foo', 'copy': {'+': 'true'}, 'paste': {'+': 'readlink /proc/self'}}
"+p

generates pids, this is the correct behavior

Bad behavior

nvim -u NORC
:echo has('unnamedplus')
:let g:clipboard = {'name': 'foo', 'copy': {'+': 'true'}, 'paste': {'+': 'readlink /proc/self'}}
"+p

does not generate pids. It seems that looking at has('unnamedplus') causes the setting of g:clipboard to be ineffective

More good behavior

nvim -u NORC
:let g:clipboard = {'name': 'foo', 'copy': {'+': 'true'}, 'paste': {'+': 'readlink /proc/self'}}
:echo has('unnamedplus')
"+p

so it depends on the order.

@bfredl
Copy link
Member

bfredl commented Feb 16, 2018

Yes, g:clipboard is only checked once, at first time any clipboard operation (including has('clipboard') et al) is invoked. But we have a framework for dict watching, so we could add a callback for g:clipboard changed (both the dict itself and its slot in g:) and reload the provider if the value changed.

@danishprakash
Copy link
Contributor

@bfredl is this a low complexity issue? I want to give this a shot if it is. Also, is every has() call a clipboard operation beacause as per the description, has('unnamedplus') affects g:clipboard.

@bfredl
Copy link
Member

bfredl commented Feb 17, 2018

Yes, should be relatively low complexity. Actually could start with supporting manual reconfiguration: there should be a function provider#clipboard#reload that can be called to redo the initialization, i e first checking g:clipboard and then the various default executables. Only obstactle: currently we use whether the autoload itself succeeded or not to determine the value of has('clipboard'), this will not work anymore. Rather there needs to be function provider#clipboard#Enabled (which maybe be merged with existing #Error, thus with loading error still return the error) that is called every time has('clipboard') is invoked. So eval_has_provider needs to be refactored.

@mhinz
Copy link
Member

mhinz commented Feb 17, 2018

Or we simply document that g:clipboard is only read once. I'm unsure if there are so many use cases that warrant all this new code for dynamic reloading.

@bfredl
Copy link
Member

bfredl commented Feb 17, 2018

@mhinz And what is this "all new code" that would need be written? You can already call provide#clipboard#Executable to reconfigure the provider, but the remaining problem is that this can't change whether nvim consider the provider to be valid or not. What is needed is the refactor of the existing code that checks the autoload status (and already on every has('clipboard') call), to instead check a mutable indictor (either function or reserved variable).

I agree it is not a prioritized issue, but it seems weird to have already the 90% code written in the way needed to support dynamic reconfiguration, and then dissuade someone to fix the remaining 10%.

@mhinz
Copy link
Member

mhinz commented Feb 17, 2018

It was merely a suggestion. And changed code is new code as well. ;-)

Everyone who wants to tackle this is free to do so. Let's label this.

@mhinz mhinz added clipboard clipboard, paste complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md labels Feb 17, 2018
@bfredl
Copy link
Member

bfredl commented Feb 17, 2018

True, but if your claim is irrefutable it wasn't very interesting in the first place :) I think it is "simpler" in the long run to change existing code to remove an unnecessary restriction, even if the edge case is rare, rather than "simply" keep the old check and add documentation when the code behaves as expected or not. This is different situation from where significant new code is added only for the edge case, which indeed has a complexity cost that must be warranted by its use cases.

@danishprakash
Copy link
Contributor

@bfredl your suggestion makes sense.

I'm gonna go ahead and work on fixing this issue.

@justinmk justinmk added the enhancement feature request label Mar 25, 2018
@justinmk justinmk added this to the 0.4 milestone Mar 25, 2018
@justinmk justinmk modified the milestones: 0.9, 0.4 Aug 21, 2019
@justinmk
Copy link
Member

could start with supporting manual reconfiguration: there should be a function provider#clipboard#reload that can be called to redo the initialization,

#10161 supports reloading clipboard. Doc for this is in :help provider-reload .

    :unlet g:loaded_clipboard_provider
    :runtime autoload/provider/clipboard.vim

khjorth added a commit to khjorth/neovide that referenced this issue Jul 20, 2022
[Clipboard providers are only loaded once by Neovim](neovim/neovim#8017), causing the previous code to use an obsolete channel id when reconnecting to a remote nvim instance.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clipboard clipboard, paste complexity:low Low-risk, self-contained. Do NOT ask "can I work on this", just read CONTRIBUTING.md enhancement feature request
Projects
None yet
Development

No branches or pull requests

5 participants