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

Avoid changing 'undolevels' when creating an undo break. #1534

Merged
merged 1 commit into from
Aug 5, 2023
Merged

Avoid changing 'undolevels' when creating an undo break. #1534

merged 1 commit into from
Aug 5, 2023

Conversation

drmikehenry
Copy link
Contributor

@drmikehenry drmikehenry commented Aug 5, 2023

Setting the 'undolevels' option to any value has the side effect of creating an undo break. UltiSnips uses the below construct to create an undo break with the goal of leaving 'undolevels' unmodified:

:let &undolevels = &undolevels

However, if a local 'undolevels' option has been set to a different value than the global option, the assignment above has the unintended effect of changing the global 'undolevels' value to the local value.

Use &g:undolevels to explicitly read and modify only the global option value, avoiding undesired option changes.

See the related pull request vim/vim#12729 suggesting related Vim documentation changes.

Currently, UltiSnips crashes on Neovim due in part to the issue described in this pull request: neovim/neovim#24574 (but already fixed by neovim/neovim#24501).

To trigger this crash:

  1. Launch Neovim:

    nvim
    
  2. Split the buffer into two windows:

    wincmd v
    
  3. Launch a substitute command:

    :s/.*/
    
  4. Trigger a snippet, leading to an error similar to:

An error occured. This is either a bug in UltiSnips or a bug in a
snippet definition. If you think this is a bug, please report it to
https://github.com/SirVer/ultisnips/issues/new
Please read and follow:
https://github.com/SirVer/ultisnips/blob/master/CONTRIBUTING.md#reproducing-bugs

Following is the full stack trace:
Traceback (most recent call last):
  File "/home/mike/.vim/bundle/ultisnips/pythonx/UltiSnips/err_to_scratch_buffer.py", line 47, in wrapper
    return func(self, *args, **kwds)
  File "/home/mike/.vim/bundle/ultisnips/pythonx/UltiSnips/snippet_manager.py", line 192, in expand_or_jump
    rv = self._try_expand()
  File "/home/mike/.vim/bundle/ultisnips/pythonx/UltiSnips/snippet_manager.py", line 811, in _try_expand
    vim_helper.command("let &undolevels = &undolevels")
  File "/home/mike/.vim/bundle/ultisnips/pythonx/UltiSnips/vim_helper.py", line 118, in command
    return vim.command(cmd)
  File "/opt/pynvim/lib/python3.10/site-packages/pynvim/api/nvim.py", line 287, in command
    return self.request('nvim_command', string, **kwargs)
  File "/opt/pynvim/lib/python3.10/site-packages/pynvim/api/nvim.py", line 182, in request
    res = self._session.request(name, *args, **kwargs)
  File "/opt/pynvim/lib/python3.10/site-packages/pynvim/msgpack_rpc/session.py", line 102, in request
    raise self.error_wrapper(err)
pynvim.api.common.NvimError: function ExpandSnippetOrJumpOrSkel[1]..UltiSnips#ExpandSnippetOrJump[2]..provider#python3#Call[18]..UltiSnips#ExpandSnippetOrJump, line 2: Vim(let):E474: Invalid argument

In the final line, the error Vim(let):E474: Invalid argument derives from Neovim's cmdpreview feature leaving the buffer-local 'undolevels' value set to LONG_MAX, which (on some machines) is outside the range of int that Neovim currently enforces for all options.

So in addition to properly retaining the global 'undolevels' option value, this change to UltiSnips also avoids the above UltiSnips crash on Neovim.

Setting the 'undolevels' option to any value has the side effect of
creating an undo break.  UltiSnips uses the below construct to create an
undo break with the goal of leaving 'undolevels' unmodified:

```
:let &undolevels = &undolevels
```

However, if a local 'undolevels' option has been set to a different
value than the global option, the assignment above has the unintended
effect of changing the global 'undolevels' value to the local value.

Use `&g:undolevels` to explicitly read and modify only the global option
value, avoiding undesired option changes.
@SirVer
Copy link
Owner

SirVer commented Aug 5, 2023

Great analysis, bug fix and explanation. Thank you very much for taking the time to fixing this!

@SirVer SirVer merged commit aa25c0d into SirVer:master Aug 5, 2023
9 of 12 checks passed
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.

2 participants