-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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: trigger LspDetach on buffer delete #28795
fix: trigger LspDetach on buffer delete #28795
Conversation
buffer = bufnr, | ||
modeline = false, | ||
data = { client_id = client.id }, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: what happens if the autocmd handler throws an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't thought of that, but are errors actually propagated through nvim_exec_autocmds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested with a simple handler that just unconditionally errors, and it doesn't seem to break anything for a normal LSP. I say normal because it was breaking things with null-ls (none-ls), but the error was coming from the client.notify
call, which null-ls monkey-patches afaik. Not sure if this is something that should be accounted for.
65a9536
to
1fa28fd
Compare
Thanks. I extended a test case to cover the LspDetach event. |
Co-authored-by: Mathias Fussenegger <f.mathias@zignar.net> (cherry picked from commit 5ac8db1)
Successfully created backport PR for |
Co-authored-by: Mathias Fussenegger <f.mathias@zignar.net>
Currently, the LspDetach autocmd is triggered when the attached client quits, or if the
vim.lsp.buf_detach_client
function is called, but not if the buffer is deleted.I've basically just updated the
on_detach
callback to be likebuf_detach_client
. Would it perhaps be better to just callbuf_detach_client
directly? I'm also not sure if thevim.diagnostic.reset
bit is necessary.There should probably be a test for this, but I have no idea how any of that works... this PR is mostly because it was easier than writing up a bug report :) Please feel free to make/suggest any necessary changes.