Repository navigation
Stop relying on Vim mark for post-action cursor restore - #1682
Merged
Merged
Conversation
`_execute_action` used to set Vim mark `\`` at the cursor pre-action
and consult `getpos("'\`")` afterwards to restore the cursor (and to
detect that the action modified the cursor's line without setting
`snip.cursor`). Neovim clears marks on a line when that line is
replaced via the Python buffer API, so any action that rewrote the
`snippet_end` line raised a spurious "line under the cursor was
modified" — see the skip_if on
`SnippetActions_PostActionModifiesCharAfterSnippet`.
The mark wasn't load-bearing for the actual cursor restore — Vim and
Neovim both auto-adjust `vim.current.window.cursor` across buffer
edits inside the action (lines inserted above shift the cursor down,
etc.), so by the time we return from `_eval_code` the cursor is
already where it should be. All we needed the mark for was the
content-equality check.
Drop the mark machinery in `_execute_action`. Compare
`line_till_cursor` before and after the action directly:
- if `snip.cursor` is set, honor it;
- else, if the line content under the cursor changed, raise the same
PebkacError as before;
- otherwise leave the cursor at whatever Vim/Neovim auto-adjusted to.
`SnippetActions_PostActionCanUseSnippetRange` (action inserts lines
before the snippet) and `SnippetActions_ErrorOnModificationSnippetLine`
(`:normal dd` deletes cursor's line) keep working, and the skip_if on
`SnippetActions_PostActionModifiesCharAfterSnippet` is dropped — the
test now runs on both Vim and Neovim.
CI failed `SnippetActions_ErrorOnBufferModificationThroughCommand` and
relatives because dropping the Vim mark removed the line-shift
adjustment we needed for actions like `vim.command('normal O')`: the
mark used to track the cursor's *content* across line insertions
above, so the line-content equality check below it passed and
`validate_buffer()` got a chance to fire its "changes are
untrackable" PebkacError on context exit.
Reintroduce the mark for the Vim-survives path, but treat a cleared
mark (Neovim's nvim_buf_set_lines drops marks when the cursor's line
is replaced via the buffer API) as "stay where the action left us"
instead of as "cursor invalid". The line_till_cursor comparison
below still catches actions that actually mutated the cursor's line
content without setting `snip.cursor`, so the
`SnippetActions_PostActionModifiesCharAfterSnippet` test that
motivated this branch still passes on both flavours.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_execute_action(pythonx/UltiSnips/snippet/definition/base.py) used to set Vim mark`at the cursor before the action ran and consultgetpos()afterwards to restore the cursor (and to detect that the action mutated the cursor's line without settingsnip.cursor). Neovim clears marks on a line when that line is replaced via the Python buffer API, so anypost_expandaction that rewrote thesnippet_endline raised a spurious "line under the cursor was modified" - the reasonSnippetActions_PostActionModifiesCharAfterSnippetcarried askip_ifon Neovim.The mark wasn't necessary for the actual cursor restore. Vim and Neovim both auto-adjust
vim.current.window.cursoracross buffer edits inside the action (lines inserted above shift the cursor down, etc.), so by the time_eval_codereturns the cursor is already where it should be. The only thing the mark was buying us was the "did the line content under the cursor change?" guard.Drop the mark machinery and compare
line_till_cursorbefore and after the action directly: ifsnip.cursoris set, honor it; else, if the line content under the cursor changed, raise the samePebkacErroras before; otherwise leave the cursor at whatever Vim/Neovim auto-adjusted to.SnippetActions_PostActionCanUseSnippetRange(action inserts lines before the snippet) andSnippetActions_ErrorOnModificationSnippetLine(:normal dddeletes cursor's line) keep their previous behaviour, and theskip_ifonSnippetActions_PostActionModifiesCharAfterSnippetis dropped — the test now runs on both Vim and Neovim.