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

tracking issue: new incremental sync (luansnip, 'index out of range', undos) #16274

Closed
clason opened this issue Nov 9, 2021 · 10 comments · Fixed by #16308
Closed

tracking issue: new incremental sync (luansnip, 'index out of range', undos) #16274

clason opened this issue Nov 9, 2021 · 10 comments · Fixed by #16308
Assignees
Labels
bug issues reporting wrong behavior

Comments

@clason
Copy link
Member

clason commented Nov 9, 2021

Neovim version (nvim -v)

0.6.0 commit 2ecf0a4

Vim (not Nvim) behaves the same?

N/A

Operating system/version

macOS 12.0

Terminal name/version

iterm 3.5

$TERM environment variable

xterm-256color

Installation

build from source

How to reproduce the issue

Install texlab LSP server from https://github.com/latex-lsp/texlab and download https://gist.github.com/clason/b9b64a8ceb67d00c245dc58743bb053c

Then

  1. nvim --clean -u minimal test.tex, wait for Ready! to show up
  2. set ft=tex (needed since vim defaults to plaintex for empty .tex files)
  3. eq<tab>foo<tab> (note that a literal tab is inserted instead of jumping to the next placeholder)
  4. <esc>u

Expected behavior

no error (as before 2ecf0a4)

Actual behavior

Error executing lua callback: /usr/local/share/nvim/runtime/lua/vim/lsp/sync.lua:66: index out of range
stack traceback:
        [C]: in function 'str_utfindex'
        /usr/local/share/nvim/runtime/lua/vim/lsp/sync.lua:66: in function 'byte_to_utf'
        /usr/local/share/nvim/runtime/lua/vim/lsp/sync.lua:100: in function 'align_position'
        /usr/local/share/nvim/runtime/lua/vim/lsp/sync.lua:222: in function 'compute_end_range'
        /usr/local/share/nvim/runtime/lua/vim/lsp/sync.lua:351: in function 'compute_diff'
        /usr/local/share/nvim/runtime/lua/vim/lsp.lua:366: in function 'incremental_changes'
        /usr/local/share/nvim/runtime/lua/vim/lsp.lua:398: in function 'fn'
        /usr/local/share/nvim/runtime/lua/vim/lsp.lua:163: in function 'for_each_buffer_client'
        /usr/local/share/nvim/runtime/lua/vim/lsp.lua:1084: in function </usr/local/share/nvim/runtime/lua/vim
/lsp.lua:1076>
@clason clason added the bug issues reporting wrong behavior label Nov 9, 2021
@clason clason self-assigned this Nov 9, 2021
@mjlbach
Copy link
Contributor

mjlbach commented Nov 9, 2021

I'll look into this, and try to have it fixed by tonight.

NB: the bad call is
\end{equation} 14 14 to align_position`

@mjlbach mjlbach changed the title New incremental sync interferes with LuaSnip tracking issue: new incremental sync (luansnip, null-ls, 'index out of range') Nov 10, 2021
@jose-elias-alvarez
Copy link

null-ls had an issue caused by the removal of vim.lsp.util.compute_diff (which we probably shouldn't have relied on in the first place) but it should be fixed now. Since it doesn't actually use incremental sync, I don't think there should be any more issues related to the commit.

@mjlbach
Copy link
Contributor

mjlbach commented Nov 10, 2021

Great! Thanks. If you all need it a diffing algorithm you would need to be sure you are passing the same arguments as we rely on detailed information from the on_lines callback to get the true start/end range.

@mjlbach mjlbach changed the title tracking issue: new incremental sync (luansnip, null-ls, 'index out of range') tracking issue: new incremental sync (luansnip 'index out of range', undos) Nov 10, 2021
@mjlbach
Copy link
Contributor

mjlbach commented Nov 10, 2021

The actual pre and post lines for the diff are:

{ "\\begin{equation}\\label{eq:sr}", "\t", "\\end{equation}" }
{ "\\begin{equation}\\label{eq:sr}", "\t", "\\end{equation}" }

With the info passed to the on_lines callback being:

  firstline = 2,
  lastline = 4,
  new_lastline = 4,

As far as I can see, no change.

The problematic thing is new_lastline and lastline ( -1) are out of the range which speaks to the buffer state captured when on_lines is invoked being out of sync with the info passed to on_lines.

What concretely this means is the "shortcuts" I took to making the extremely efficient (basically we know exactly the line the diff occurs on ahead of time and only have to scan at most one line) cannot rely on the knowledge they are assuming to be accurate

The options are

Long-term: fix the internal undo tracking in neovim
short-term: either A: always send the entire buffer (fine after my cjson changes) or B: scan pre and post buffer, line by line, until we find the diff (Or C: somehow my diagnosis is wrong :) )

The old mechanism was a bit more robust (but still wrong) to firstline/lastline being incorrect for the current buf.

@bfredl
Copy link
Member

bfredl commented Nov 10, 2021

hmm, is the same bug also visible in external buffer updates ( nvim_buf_attach from RPC)? unlike on_bytes we already expect the precise line state to be available in the callback, including for undo operations (because otherwise RPC updates cannot work). If there is a minimal repro available I could try to fix the specific case?

@mjlbach mjlbach changed the title tracking issue: new incremental sync (luansnip 'index out of range', undos) tracking issue: new incremental sync (luansnip, 'index out of range', undos) Nov 11, 2021
@mjlbach
Copy link
Contributor

mjlbach commented Nov 11, 2021

Copying a reproduction from a duplicate issue:

mp.tex:

\begin{document}
\section*{1}
\section*{1}
\section*{1}
\end{document}

3gg$h<C-V>jg<C-A>

Similar situation, on_lines doesn't appear to be locked to the buffer update (not curr_lines == prev_lines (cached buffer from last on_lines call)):

{                                                                                                                              
  curr_lines = { "\\begin{document}", "\\section*{1}", "\\section*{2}", "\\section*{2}", "\\end{document}" },                  
  firstline = 3,                                                                                                               
  lastline = 5,                                                                                                                
  new_lastline = 5,                                                                                                            
  offset_encoding = "utf-16",                                                                                                  
  prev_lines = { "\\begin{document}", "\\section*{1}", "\\section*{2}", "\\section*{2}", "\\end{document}" },                  
  start_range = {                                                                                                              
    byte_idx = 13,                                                                                                             
    char_idx = 13,                                                                                                             
    line_idx = 3                                                                                                               
  }   

@dmitmel
Copy link
Contributor

dmitmel commented Nov 11, 2021

Here's a solid repro involving apply_text_edits. Doesn't need a Language Server running in background, calling wreck_the_buffer is enough. The logger will output a list of on_lines events, the ones with firstline_incorrect and/or lastline_incorrect are the broken ones which will cause problems with the new syncer.

function _G.wreck_the_buffer(bufnr)
  log_on_lines(bufnr)
  vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, {'a', 'a', '', '', 'b', 'b'})
  vim.lsp.util.apply_text_edits({
    {
      range = {
        start   = { line = 1, character = 1 },
        ["end"] = { line = 4, character = 0 },
      }, newText = "\n\n"
    }
  })
end

function _G.log_on_lines(bufnr)
  local old_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, true)
  vim.api.nvim_buf_attach(bufnr, false, {
    on_lines = function(_, _, changedtick, firstline, old_lastline, new_lastline)
      local new_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, true)
      local event = {
        _1_bufnr = bufnr,
        _2_changedtick = changedtick,
        _3_firstline = firstline,
        _4_old_lastline = old_lastline,
        _5_new_lastline = new_lastline,
        _6_old_lines = vim.list_slice(old_lines, firstline + 1, old_lastline),
        _7_new_lines = vim.list_slice(new_lines, firstline + 1, new_lastline),
        _8_firstline_incorrect = old_lines[firstline + 1] == new_lines[firstline + 1],
        _9_lastline_incorrect = old_lines[old_lastline] == new_lines[new_lastline],
      }
      print(vim.inspect(event))
      old_lines = vim.api.nvim_buf_get_lines(bufnr, 0, -1, true)
    end;
  })
end

@dmitmel
Copy link
Contributor

dmitmel commented Nov 11, 2021

Here's another solid repro related to the undo history. To reproduce:

  1. Download and unpack the attached zip file
  2. Open util.lua
  3. :rundo %.undo
  4. Pressing u and CTRL-R repeatedly generates on_lines events which don't actually change anything.

(To check the on_lines events the log_on_lines function from above comment may be used.)

util.lua.zip

@dmitmel
Copy link
Contributor

dmitmel commented Nov 11, 2021

Found an even better repro:

function _G.final_test(bufnr)
  log_on_lines(bufnr)
  vim.api.nvim_buf_set_lines(bufnr, 0, -1, true, {'a', 'b', 'c', 'd', 'e', 'f'})
  vim.api.nvim_buf_set_lines(bufnr, 2, 4, true, {'c', 'z', 'x', 'd'})
end

@dmitmel
Copy link
Contributor

dmitmel commented Nov 13, 2021

Found another repro:

  1. Create the following file:
    int main() {
      // nothing here
    }
  2. Run :%< (from command line prompt)

Strictly speaking, even selecting some code block with Visual mode and repeatedly shifting it with < to the left until it hits the left edge is enough, and this is hardly a unique situation in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues reporting wrong behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants