commit e55e80d51ca5d85770981bffb9254badc3662e0c
parent 20bfdbe83253cf5ec0a42bd75bd4ed7df945ab37
Author: Chris AtLee <chris@atlee.ca>
Date: Tue, 1 Aug 2023 08:13:52 -0400
fix(lsp): inlay hints: "Failed to delete autocmd" when closing buffer #24469
Problem:
"Failed to delete autocmd" error when deleting LspNotify autocmd. #24456
Solution:
Change a few things in the inlay_hint and diagnostic LSP code:
1. Re-introduce the `enabled` flag for the buffer state tables. Previously I was
relying on the presence of an autocmd id in the state table to track whether
inlay_hint / diagnostic was enabled for a buffer. There are two reasons why
this doesn't work well:
- Each time inlay_hint / diagnostic is enabled, we call `nvim_buf_attach` on
the buffer, resulting in multiple `on_reload` or `on_detach` callbacks being
registered.
- Commands like `bwipeout` delete buffer local autocmds, sometimes before our
`on_detach` callbacks have a chance to delete them first. This causes the
- Use module local enabled state for diagnostic as well. bwipeout can race
with on_detach callbacks for deleting autocmds. Error referenced in #24456.
2. Change the `LspDetach` autocmd to run each time (i.e., remove the `once`
flag). Since we're only registering autocmds once per buffer now, we
need to make sure that we set the enabled flag properly each time the LSP
client detaches from the buffer.
- Remove `once` from the LspDetach autocmds for inlay_hint and diagnostic.
We only set up the autocmd once now. Gets removed when buffer is deleted.
3. Have the `LspNotify` handler also refresh the inlay_hint / diagnostics when
receiving the `textDocument/didOpen` event. Before this point, the LSP
backend doesn't have the contents of the buffer, so can't provide inlay hints
or diagnostics.
Downsides of this approach:
* When inlay_hint / diagnostics are disabled on a buffer, it will continue to
receive `LspNotify` events for that buffer. The callback exits early since the
`enabled` flag is false.
Alternatives:
* Can we wrap the call to `nvim_del_autocmd` in `pcall` to swallow any errors
resulting from trying to delete the autocmd?
Fixes #24456
Helped-by: Maria José Solano <majosolano99@gmail.com>
Diffstat:
2 files changed, 67 insertions(+), 53 deletions(-)
diff --git a/runtime/lua/vim/lsp/diagnostic.lua b/runtime/lua/vim/lsp/diagnostic.lua
@@ -392,19 +392,18 @@ local function clear(bufnr)
end
end
---- autocmd ids for LspNotify handlers per buffer
---- @private
---- @type table<integer,integer>
-local _autocmd_ids = {}
+---@class lsp.diagnostic.bufstate
+---@field enabled boolean Whether inlay hints are enabled for this buffer
+---@type table<integer, lsp.diagnostic.bufstate>
+local bufstates = {}
--- Disable pull diagnostics for a buffer
--- @private
local function disable(bufnr)
- if not _autocmd_ids[bufnr] then
- return
+ local bufstate = bufstates[bufnr]
+ if bufstate then
+ bufstate.enabled = false
end
- api.nvim_del_autocmd(_autocmd_ids[bufnr])
- _autocmd_ids[bufnr] = nil
clear(bufnr)
end
@@ -416,38 +415,46 @@ function M._enable(bufnr)
bufnr = api.nvim_get_current_buf()
end
- if _autocmd_ids[bufnr] then
- return
+ if not bufstates[bufnr] then
+ bufstates[bufnr] = { enabled = true }
+
+ api.nvim_create_autocmd('LspNotify', {
+ buffer = bufnr,
+ callback = function(opts)
+ if
+ opts.data.method ~= 'textDocument/didChange'
+ and opts.data.method ~= 'textDocument/didOpen'
+ then
+ return
+ end
+ if bufstates[bufnr] and bufstates[bufnr].enabled then
+ util._refresh('textDocument/diagnostic', { bufnr = bufnr, only_visible = true })
+ end
+ end,
+ group = augroup,
+ })
+
+ api.nvim_buf_attach(bufnr, false, {
+ on_reload = function()
+ if bufstates[bufnr] and bufstates[bufnr].enabled then
+ util._refresh('textDocument/diagnostic', { bufnr = bufnr })
+ end
+ end,
+ on_detach = function()
+ disable(bufnr)
+ end,
+ })
+
+ api.nvim_create_autocmd('LspDetach', {
+ buffer = bufnr,
+ callback = function()
+ disable(bufnr)
+ end,
+ group = augroup,
+ })
+ else
+ bufstates[bufnr].enabled = true
end
-
- _autocmd_ids[bufnr] = api.nvim_create_autocmd('LspNotify', {
- buffer = bufnr,
- callback = function(opts)
- if opts.data.method ~= 'textDocument/didChange' then
- return
- end
- util._refresh('textDocument/diagnostic', { bufnr = bufnr, only_visible = true })
- end,
- group = augroup,
- })
-
- api.nvim_buf_attach(bufnr, false, {
- on_reload = function()
- util._refresh('textDocument/diagnostic', { bufnr = bufnr })
- end,
- on_detach = function()
- disable(bufnr)
- end,
- })
-
- api.nvim_create_autocmd('LspDetach', {
- buffer = bufnr,
- callback = function()
- disable(bufnr)
- end,
- once = true,
- group = augroup,
- })
end
return M
diff --git a/runtime/lua/vim/lsp/inlay_hint.lua b/runtime/lua/vim/lsp/inlay_hint.lua
@@ -3,12 +3,12 @@ local log = require('vim.lsp.log')
local api = vim.api
local M = {}
----@class lsp._inlay_hint.bufstate
+---@class lsp.inlay_hint.bufstate
---@field version integer
---@field client_hint table<integer, table<integer, lsp.InlayHint[]>> client_id -> (lnum -> hints)
---@field applied table<integer, integer> Last version of hints applied to this line
----@field autocmd_id integer The autocmd id for the buffer
----@type table<integer, lsp._inlay_hint.bufstate>
+---@field enabled boolean Whether inlay hints are enabled for this buffer
+---@type table<integer, lsp.inlay_hint.bufstate>
local bufstates = {}
local namespace = api.nvim_create_namespace('vim_lsp_inlayhint')
@@ -31,6 +31,9 @@ function M.on_inlayhint(err, result, ctx, _)
return
end
local bufstate = bufstates[bufnr]
+ if not bufstate or not bufstate.enabled then
+ return
+ end
if not (bufstate.client_hint and bufstate.version) then
bufstate.client_hint = vim.defaulttable()
bufstate.version = ctx.version
@@ -122,10 +125,9 @@ local function disable(bufnr)
bufnr = api.nvim_get_current_buf()
end
clear(bufnr)
- if bufstates[bufnr] and bufstates[bufnr].autocmd_id then
- api.nvim_del_autocmd(bufstates[bufnr].autocmd_id)
+ if bufstates[bufnr] then
+ bufstates[bufnr] = { enabled = false, applied = {} }
end
- bufstates[bufnr] = nil
end
--- Enable inlay hints for a buffer
@@ -136,24 +138,27 @@ local function enable(bufnr)
end
local bufstate = bufstates[bufnr]
if not bufstate then
- bufstates[bufnr] = { applied = {} }
- bufstates[bufnr].autocmd_id = api.nvim_create_autocmd('LspNotify', {
+ bufstates[bufnr] = { applied = {}, enabled = true }
+ api.nvim_create_autocmd('LspNotify', {
buffer = bufnr,
callback = function(opts)
- if opts.data.method ~= 'textDocument/didChange' then
+ if
+ opts.data.method ~= 'textDocument/didChange'
+ and opts.data.method ~= 'textDocument/didOpen'
+ then
return
end
- if bufstates[bufnr] then
+ if bufstates[bufnr] and bufstates[bufnr].enabled then
util._refresh('textDocument/inlayHint', { bufnr = bufnr })
end
end,
group = augroup,
})
util._refresh('textDocument/inlayHint', { bufnr = bufnr })
- api.nvim_buf_attach(bufnr, true, {
+ api.nvim_buf_attach(bufnr, false, {
on_reload = function(_, cb_bufnr)
clear(cb_bufnr)
- if bufstates[cb_bufnr] then
+ if bufstates[cb_bufnr] and bufstates[cb_bufnr].enabled then
bufstates[cb_bufnr].applied = {}
util._refresh('textDocument/inlayHint', { bufnr = cb_bufnr })
end
@@ -167,9 +172,11 @@ local function enable(bufnr)
callback = function()
disable(bufnr)
end,
- once = true,
group = augroup,
})
+ else
+ bufstate.enabled = true
+ util._refresh('textDocument/inlayHint', { bufnr = bufnr })
end
end
@@ -180,7 +187,7 @@ local function toggle(bufnr)
bufnr = api.nvim_get_current_buf()
end
local bufstate = bufstates[bufnr]
- if bufstate then
+ if bufstate and bufstate.enabled then
disable(bufnr)
else
enable(bufnr)
