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

Accessing vim.current.window (through python-client) breaks CursorHoldI?! #3757

Closed
blueyed opened this issue Nov 29, 2015 · 18 comments
Closed
Labels
api libnvim, Nvim RPC API bug issues reporting wrong behavior has:plan has:repro issue contains minimal reproducing steps
Milestone

Comments

@blueyed
Copy link
Contributor

blueyed commented Nov 29, 2015

I have found a strange bug when using deoplete, where accessing self.vim.current.window through a Python RPC call seems to break the CursorHoldI handling?!

See Shougo/deoplete.nvim#81 for more information and a test case (based on deoplete).

I have also tried the latest python-client from Git (https://github.com/neovim/python-client/tree/6c0ec04307442dedc851873504b379f4dcdb33a0) without success.

Why might that happen?
Where can I look further to debug this?

This breaks the feature to display call signatures (g:jedi#show_call_signatures) in jedi-vim.

@blueyed blueyed changed the title Accessing vim.current.window breaks CursorHoldI?! Accessing vim.current.window (through python-client) breaks CursorHoldI?! Nov 29, 2015
@blueyed
Copy link
Contributor Author

blueyed commented Nov 29, 2015

Just calling self.vim.eval("1") seems to trigger this issue already (when used as the first statement in
https://github.com/Shougo/deoplete.nvim/blob/fed61ada642b86cf6bb70cb6914628fbe0ac902e/rplugin/python3/deoplete/__init__.py#L46).

/cc @Shougo

@tarruda
Copy link
Member

tarruda commented Nov 29, 2015

Why might that happen?

I have recently changed how cursor hold is triggered, but I can't immediately say what's causing the issue

Where can I look further to debug this?

cursorhold is triggered in the os_inchar function when trigger_cursorhold() && !typebuf_changed(tb_change_cnt) evaluates to true, so you could start by investigating what's causing it to always evaluate to false. There's a chance that watching the did_cursorhold global variable will reveal the cause.

@blueyed
Copy link
Contributor Author

blueyed commented Nov 29, 2015

Seems to be related to && s->ca.cmdchar != K_EVENT at

if (s->ca.cmdchar != K_IGNORE && s->ca.cmdchar != K_EVENT) {
(added in e5165ba).

@blueyed
Copy link
Contributor Author

blueyed commented Nov 29, 2015

Also commenting this line fixes it (but breaks maybe something else?):

did_cursorhold = true;

blueyed added a commit to blueyed/neovim that referenced this issue Nov 29, 2015
This does not seem to break the use case mentioned in the comment: there
is no CursorHoldI event being fired after `CTRL-V` in insert mode.

Ref: neovim#3757 (comment)
@tarruda
Copy link
Member

tarruda commented Nov 29, 2015

Also commenting this line fixes it (but breaks maybe something else?):

According to the comment, cursorhold should not trigger while processing multi-key commands such as <c-v>

Seems to be related to && s->ca.cmdchar != K_EVENT at

Close, I think the line which causes the bug is this one: e5165ba#diff-ce1f54750dd541783db1cf9a24120ba8R1221

That is because the same "special key" is used for both cursorhold and other events, and we need a way to distinguish so did_cursorhold will be set to false properly. I need to think about this for a while

@Shougo
Copy link
Contributor

Shougo commented Dec 3, 2015

Reproduced.
CursorHoldI autocmd is not triggered using neovim Job API....

@jakubgs
Copy link

jakubgs commented Dec 3, 2015

+1 for this issue.

@blueyed
Copy link
Contributor Author

blueyed commented Jan 18, 2016

@tarruda
Any new insights / update on this front?

It's a very subtle, but very annoying bug.

@blueyed
Copy link
Contributor Author

blueyed commented Mar 17, 2016

@tarruda
FYI, a similar issue regarding the popup menu was fixed in #4461 - in case that helps to narrow this one down.

@justinmk justinmk added the bug issues reporting wrong behavior label Apr 10, 2016
@justinmk justinmk added the api libnvim, Nvim RPC API label Apr 10, 2016
@justinmk justinmk added this to the 0.2 milestone May 27, 2016
@justinmk
Copy link
Member

The root cause of this issue may also affect timers: #4624 (comment)

blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 7, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 25, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 31, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 31, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Feb 11, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Mar 7, 2017
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
@justinmk justinmk removed this from the 0.2 milestone Apr 1, 2017
@gelguy
Copy link
Contributor

gelguy commented May 30, 2019

Smaller vimrc to reproduce the issue by using a timer instead of rpc

set noshowmode
set updatetime=100

let s:init = 0

function! F()
  if !s:init
    call timer_start(0, {-> 1})
  endif
  let s:init = 1
endfunction

augroup test
  au CursorHoldI * echom 'CHI'
  au InsertEnter * call F()
augroup END

Refer to

neovim/src/nvim/edit.c

Lines 643 to 644 in c6cd608

// Don't want K_EVENT with cursorhold for the second key, e.g., after CTRL-V.
did_cursorhold = true;

K_EVENT prevents CursorHold events from triggering. If this is changed to

  if (key != K_EVENT) {
    did_cursorhold = true;
  }

the above vimrc will work as expected.

What I'm worried about is the comment about K_EVENT... I'm not well-versed enough in that area of code to understand the implications of changing the code here.

@blueyed
Copy link
Contributor Author

blueyed commented Jun 28, 2019

@gelguy
Thanks for this. I hope to look into this during the next week.

@justinmk justinmk added has:plan has:repro issue contains minimal reproducing steps labels Jun 29, 2019
blueyed added a commit to blueyed/neovim that referenced this issue Jul 4, 2019
This does not seem to break the use case mentioned in the comment: there
is no CursorHoldI event being fired after `CTRL-V` in insert mode.

Ref: neovim#3757 (comment)
blueyed added a commit to blueyed/neovim that referenced this issue Jul 8, 2019
blueyed added a commit to blueyed/neovim that referenced this issue Jul 8, 2019
This does not seem to break the use case mentioned in the comment: there
is no CursorHoldI event being fired after `CTRL-V` in insert mode.

Ref: neovim#3757 (comment)
blueyed added a commit to blueyed/neovim that referenced this issue Jul 8, 2019
@blueyed
Copy link
Contributor Author

blueyed commented Jul 8, 2019

Actually with the above vimrc from @gelguy you can also see it without a timer already: it is not triggered while staying in insert mode, inserting something, and then waiting..?!
Added the above patch to #3758.

I agree with @tarruda that this should likely be refactored / fixed properly by adding this to the state, and using a timer instead - OTOH the timeout in os_inchar is the updatetime already, so it kind of makes sense to handle it right there, in case of reading nothing because of a timeout.

blueyed added a commit to blueyed/neovim that referenced this issue Jul 12, 2019
blueyed added a commit to blueyed/neovim that referenced this issue Jul 15, 2019
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jul 20, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
timeyyy pushed a commit to timeyyy/neovim that referenced this issue Aug 9, 2019
blueyed added a commit to blueyed/jedi-vim that referenced this issue Aug 18, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Aug 18, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Oct 16, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Oct 21, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Dec 5, 2019
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 14, 2020
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Feb 3, 2020
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Feb 27, 2020
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jul 17, 2020
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jul 17, 2020
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 2, 2021
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
blueyed added a commit to blueyed/jedi-vim that referenced this issue Jan 17, 2022
This appears to fix the issue where CursorHoldI is broken when used with
deoplete etc (neovim/neovim#3757), and is less
hackish in general.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api libnvim, Nvim RPC API bug issues reporting wrong behavior has:plan has:repro issue contains minimal reproducing steps
Projects
None yet
Development

No branches or pull requests

6 participants