wyj/neovim
1
0
Fork 0
mirror of https://github.com/neovim/neovim.git synced 2024-09-17 20:58:20 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix(lsp): inlay hints: "Failed to delete autocmd" when closing buffer #24469

Browse Source 
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>
This commit is contained in:
Chris AtLee 2023-08-01 08:13:52 -04:00 committed by GitHub
parent 20bfdbe832
commit e55e80d51c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 67 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

85
runtime/lua/vim/lsp/diagnostic.lua
View File

@ -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

35
runtime/lua/vim/lsp/inlay_hint.lua
View File

@ -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)