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 buffer change cb does not read bad cursor #16647

Closed
wants to merge 1 commit into from
Closed

Ensure buffer change cb does not read bad cursor #16647

wants to merge 1 commit into from

Conversation

axelf4
Copy link
Contributor

@axelf4 axelf4 commented Dec 14, 2021

In multiple editing operations the cursor may temporarily be out of bounds during calls into user code from buffer update listeners. This means doing anything in a change callback that depends on the cursor position - including calling any function from Vim script - can throw an error. Moving the cursor is also not possible, since there is no way to restore the cursor to its previous invalid position via the public APIs.

This pull request attempts to fix the issue by always saving the location of the cursor before running each change callback with the cursor in a valid spot.

For some background, I fixed one instance of this in #11782, but there are many more, as evidenced by these issue reports for the Neovim version of vim-strip-trailing-whitespace, all of which hit this issue:

In multiple editing operations the cursor may temporarily be out of
bounds during calls into user code from buffer update listeners. This
means doing anything in a change callback that depends on the cursor
position - including calling any function from Vim script - can throw
an error. Moving the cursor is also not possible, since there is no
way to restore the cursor to its previous invalid position via the
public APIs.

This commit fixes the issue by always saving the location of the cursor
before running each change callback with the cursor in a valid spot.

See: #11782
@zeertzjq
Copy link
Member

zeertzjq commented Jan 2, 2022

Related: #16729 #16732

@perrin4869
Copy link
Contributor

@bfredl any chance you can have a look at this? thanks!

@bfredl
Copy link
Member

bfredl commented Jun 22, 2022

Would it possible to write a simple test?

@zeertzjq
Copy link
Member

I have a test in #16732.

@justinmk
Copy link
Member

#16732 looks more complete so let's continue there. Thank you for getting this started @axelf4

@justinmk justinmk closed this Jun 23, 2022
@github-actions github-actions bot removed the request for review from bfredl June 23, 2022 14:10
@axelf4
Copy link
Contributor Author

axelf4 commented Jun 23, 2022

I agree! One thing though that seems to be missing from that other PR which I would like to see is resetting the cursor to e.g. firstline so that user callbacks do not ever see a bad cursor position.

Also, for completeness, here is the test case that I had lying around which currently fails on NVIM v0.7.0:

function! G()
endfunction
function! F()
	call G()
endfunction

let fname = tempname()
call writefile(['', '0'], fname)
silent execute 'edit!' fname
lua vim.api.nvim_buf_attach(0, false, {on_lines = function(...) vim.api.nvim_call_function("F", {}) end})
2delete _
1put ='0'
normal! u

@axelf4 axelf4 deleted the fix-on_lines-bad-cursor branch June 23, 2022 16:25
@zeertzjq
Copy link
Member

function! G()
endfunction
function! F()
	call G()
endfunction

let fname = tempname()
call writefile(['', '0'], fname)
silent execute 'edit!' fname
lua vim.api.nvim_buf_attach(0, false, {on_lines = function(...) vim.api.nvim_call_function("F", {}) end})
2delete _
1put ='0'
normal! u

This failure has already been fixed by #18139.

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

5 participants