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

fix(lsp): update the changetracking buffer state on reload #28721

Closed
wants to merge 2 commits into from

Conversation

icholy
Copy link
Contributor

@icholy icholy commented May 12, 2024

Fixes #24972

When a buffer is reloaded, each client is detached from that buffer and then re-attached:

changetracking.reset_buf(client, bufnr)
if vim.tbl_get(client.server_capabilities, 'textDocumentSync', 'openClose') then
client.notify(ms.textDocument_didClose, params)
end
client:_text_document_did_open_handler(bufnr)

When there’s only 1 LSP, the change tracking buffer state is deleted and re-created when the file is externally modified. This updates the buf_state.lines with the new buffer contents.

state.buffers[bufnr] = nil

If there are 2 LSPs attached, the buffer state is never deleted because the refs count never hits zero. So the buf state is never updated with the new buffer contents.

buf_state.refs = buf_state.refs - 1
assert(buf_state.refs >= 0, 'refcount on buffer state must not get negative')
if buf_state.refs == 0 then

@github-actions github-actions bot added the lsp label May 12, 2024
@icholy icholy force-pushed the lsp-reload-buf branch 4 times, most recently from d2f40a9 to 3b0a920 Compare May 12, 2024 17:42
@icholy icholy changed the title fix(lsp): reload the change tracking buffer state fix(lsp): update the changetracking buffer state on reload May 12, 2024
@icholy
Copy link
Contributor Author

icholy commented May 12, 2024

I'm not sure how to test this.

@icholy
Copy link
Contributor Author

icholy commented May 13, 2024

I'm not attached to this implementation. If someone wants to suggest an alternate implementation or make a spin-off PR I'd be more than happy to test it out.

@icholy icholy force-pushed the lsp-reload-buf branch 2 times, most recently from 133bc15 to 9cb8ede Compare May 13, 2024 16:28
@icholy
Copy link
Contributor Author

icholy commented May 14, 2024

@mfussenegger I was looking through the history and it seems like you've done the most work in this area of the code. Any feedback would be appreciated.

@icholy
Copy link
Contributor Author

icholy commented May 15, 2024

Another way to fix this would be do change the on_reload callback to detach all the clients and then re-attach them all in a second pass. Removing all the clients would drop buf_state.refs to 0 and result in the lines being re-loaded. But this seems a little fragile.

on_reload = function()
  local params = { textDocument = { uri = uri } }
  local clients = lsp.get_clients({ bufnr = bufnr })
  for _, client in ipairs(clients) do
    changetracking.reset_buf(client, bufnr)
    if vim.tbl_get(client.server_capabilities, 'textDocumentSync', 'openClose') then
      client.notify(ms.textDocument_didClose, params)
    end
  end
  for _, client in ipairs(clients) do
    client:_text_document_did_open_handler(bufnr)
  end
end,

#28875

Problem:  The changetracking state can de-sync when reloading a buffer
          with more than one LSP client attached.
Solution: Update the changetracking buffer state lines when the
          corresponding buffer is reloaded.
@icholy
Copy link
Contributor Author

icholy commented May 21, 2024

Closing in favor of #28875

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

Successfully merging this pull request may close these issues.

error attempt to get length of local 'prev_line'
1 participant