Skip to content

fix(diagnostics): throttle sequential CocDiagnostics updates, make debouncer safe and sequential #1430

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

Merged
merged 9 commits into from
Jul 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/nvim-tree-lua.txt
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ Subsequent calls to setup will replace the previous configuration.
diagnostics = {
enable = false,
show_on_dirs = false,
debounce_delay = 50,
icons = {
hint = "",
info = "",
Expand Down Expand Up @@ -328,6 +329,7 @@ Subsequent calls to setup will replace the previous configuration.
all = false,
config = false,
copy_paste = false,
dev = false,
diagnostics = false,
git = false,
profile = false,
Expand Down Expand Up @@ -471,6 +473,10 @@ Show LSP and COC diagnostics in the signcolumn
Enable/disable the feature.
Type: `boolean`, Default: `false`

*nvim-tree.diagnostics.debounce_delay*
Idle milliseconds between diagnostic event and update.
Type: `number`, Default: `50` (ms)

*nvim-tree.diagnostics.show_on_dirs*
Show diagnostic icons on parent directories.
Type: `boolean`, Default: `false`
Expand Down Expand Up @@ -888,6 +894,10 @@ Configuration for diagnostic logging.
File copy and paste actions.
Type: `boolean`, Default: `false`

*nvim-tree.log.types.dev*
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I keep needing a "clean" log context that doesn't get removed by validate_options. Feel free to remove.

Used for local development only. Not useful for users.
Type: `boolean`, Default: `false`

*nvim-tree.log.types.diagnostics*
LSP and COC processing, verbose.
Type: `boolean`, Default: `false`
Expand Down
12 changes: 10 additions & 2 deletions lua/nvim-tree.lua
Original file line number Diff line number Diff line change
Expand Up @@ -395,11 +395,17 @@ local function setup_autocommands(opts)

if opts.diagnostics.enable then
create_nvim_tree_autocmd("DiagnosticChanged", {
callback = require("nvim-tree.diagnostics").update,
callback = function()
log.line("diagnostics", "DiagnosticChanged")
require("nvim-tree.diagnostics").update()
end,
})
create_nvim_tree_autocmd("User", {
pattern = "CocDiagnosticChange",
callback = require("nvim-tree.diagnostics").update,
callback = function()
log.line("diagnostics", "CocDiagnosticChange")
require("nvim-tree.diagnostics").update()
end,
})
end
end
Expand Down Expand Up @@ -511,6 +517,7 @@ local DEFAULT_OPTS = { -- BEGIN_DEFAULT_OPTS
diagnostics = {
enable = false,
show_on_dirs = false,
debounce_delay = 50,
icons = {
hint = "",
info = "",
Expand Down Expand Up @@ -576,6 +583,7 @@ local DEFAULT_OPTS = { -- BEGIN_DEFAULT_OPTS
all = false,
config = false,
copy_paste = false,
dev = false,
diagnostics = false,
git = false,
profile = false,
Expand Down
2 changes: 1 addition & 1 deletion lua/nvim-tree/core.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ local first_init_done = false

function M.init(foldername)
if TreeExplorer then
TreeExplorer:_clear_watchers()
TreeExplorer:destroy()
end
TreeExplorer = explorer.Explorer.new(foldername)
if not first_init_done then
Expand Down
65 changes: 34 additions & 31 deletions lua/nvim-tree/diagnostics.lua
Original file line number Diff line number Diff line change
Expand Up @@ -90,43 +90,45 @@ function M.update()
if not M.enable or not core.get_explorer() or not view.is_buf_valid(view.get_bufnr()) then
return
end
local ps = log.profile_start "diagnostics update"
log.line("diagnostics", "update")

local buffer_severity
if is_using_coc() then
buffer_severity = from_coc()
else
buffer_severity = from_nvim_lsp()
end
utils.debounce("diagnostics", M.debounce_delay, function()
local ps = log.profile_start "diagnostics update"
log.line("diagnostics", "update")

local buffer_severity
if is_using_coc() then
buffer_severity = from_coc()
else
buffer_severity = from_nvim_lsp()
end

M.clear()
M.clear()

local nodes_by_line = utils.get_nodes_by_line(core.get_explorer().nodes, core.get_nodes_starting_line())
for _, node in pairs(nodes_by_line) do
node.diag_status = nil
end
local nodes_by_line = utils.get_nodes_by_line(core.get_explorer().nodes, core.get_nodes_starting_line())
for _, node in pairs(nodes_by_line) do
node.diag_status = nil
end

for bufname, severity in pairs(buffer_severity) do
local bufpath = utils.canonical_path(bufname)
log.line("diagnostics", " bufpath '%s' severity %d", bufpath, severity)
if 0 < severity and severity < 5 then
for line, node in pairs(nodes_by_line) do
local nodepath = utils.canonical_path(node.absolute_path)
log.line("diagnostics", " %d checking nodepath '%s'", line, nodepath)
if M.show_on_dirs and vim.startswith(bufpath, nodepath) then
log.line("diagnostics", " matched fold node '%s'", node.absolute_path)
node.diag_status = severity
add_sign(line, severity)
elseif nodepath == bufpath then
log.line("diagnostics", " matched file node '%s'", node.absolute_path)
node.diag_status = severity
add_sign(line, severity)
for bufname, severity in pairs(buffer_severity) do
local bufpath = utils.canonical_path(bufname)
log.line("diagnostics", " bufpath '%s' severity %d", bufpath, severity)
if 0 < severity and severity < 5 then
for line, node in pairs(nodes_by_line) do
local nodepath = utils.canonical_path(node.absolute_path)
log.line("diagnostics", " %d checking nodepath '%s'", line, nodepath)
if M.show_on_dirs and vim.startswith(bufpath, nodepath) then
log.line("diagnostics", " matched fold node '%s'", node.absolute_path)
node.diag_status = severity
add_sign(line, severity)
elseif nodepath == bufpath then
log.line("diagnostics", " matched file node '%s'", node.absolute_path)
node.diag_status = severity
add_sign(line, severity)
end
end
end
end
end
log.profile_end(ps, "diagnostics update")
log.profile_end(ps, "diagnostics update")
end)
end

local links = {
Expand All @@ -138,6 +140,7 @@ local links = {

function M.setup(opts)
M.enable = opts.diagnostics.enable
M.debounce_delay = opts.diagnostics.debounce_delay

if M.enable then
log.line("diagnostics", "setup")
Expand Down
10 changes: 10 additions & 0 deletions lua/nvim-tree/explorer/common.lua
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ function M.update_git_status(node, parent_ignored, status)
end
end

function M.node_destroy(node)
if not node then
return
end

if node.watcher then
node.watcher:destroy()
end
end

function M.setup(opts)
M.config = {
git = opts.git,
Expand Down
17 changes: 6 additions & 11 deletions lua/nvim-tree/explorer/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ local uv = vim.loop

local git = require "nvim-tree.git"
local watch = require "nvim-tree.explorer.watch"
local common = require "nvim-tree.explorer.common"

local M = {}

Expand Down Expand Up @@ -33,22 +34,16 @@ function Explorer:expand(node)
self:_load(node)
end

function Explorer.clear_watchers_for(root_node)
function Explorer:destroy()
local function iterate(node)
if node.watcher then
node.watcher:stop()
common.node_destroy(node)
if node.nodes then
for _, child in pairs(node.nodes) do
if child.watcher then
iterate(child)
end
iterate(child)
end
end
end
iterate(root_node)
end

function Explorer:_clear_watchers()
Explorer.clear_watchers_for(self)
iterate(self)
end

function M.setup(opts)
Expand Down
7 changes: 6 additions & 1 deletion lua/nvim-tree/explorer/reload.lua
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,12 @@ function M.reload(node, status)
node.nodes = vim.tbl_map(
update_status(nodes_by_path, node_ignored, status),
vim.tbl_filter(function(n)
return child_names[n.absolute_path]
if child_names[n.absolute_path] then
return child_names[n.absolute_path]
else
common.node_destroy(n)
return nil
end
end, node.nodes)
)

Expand Down
2 changes: 1 addition & 1 deletion lua/nvim-tree/explorer/watch.lua
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ function M.create_watcher(absolute_path)
end

log.line("watcher", "node start '%s'", absolute_path)
Watcher.new {
return Watcher.new {
absolute_path = absolute_path,
interval = M.interval,
on_event = function(opts)
Expand Down
10 changes: 3 additions & 7 deletions lua/nvim-tree/git/init.lua
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ function M.reload_project(project_root, path)
return
end

if path and not path:match("^" .. project_root) then
path = nil
if path and path:find(project_root, 1, true) ~= 1 then
return
end

local git_status = Runner.run {
Expand All @@ -43,7 +43,7 @@ function M.reload_project(project_root, path)

if path then
for p in pairs(project.files) do
if p:match("^" .. path) then
if p:find(path, 1, true) == 1 then
project.files[p] = nil
end
end
Expand Down Expand Up @@ -138,10 +138,6 @@ function M.load_project_status(cwd)
reload_tree_at(opts.project_root)
end)
end,
on_event0 = function()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I must have left this in when testing.

log.line("watcher", "git event")
M.reload_tree_at(project_root)
end,
}
end

Expand Down
6 changes: 3 additions & 3 deletions lua/nvim-tree/git/runner.lua
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,11 @@ function Runner.run(opts)
log.profile_end(ps, "git job %s %s", opts.project_root, opts.path)

if self.rc == -1 then
log.line("git", "job timed out")
log.line("git", "job timed out %s %s", opts.project_root, opts.path)
elseif self.rc ~= 0 then
log.line("git", "job failed with return code %d", self.rc)
log.line("git", "job fail rc %d %s %s", self.rc, opts.project_root, opts.path)
else
log.line("git", "job success")
log.line("git", "job success %s %s", opts.project_root, opts.path)
end

return self.output
Expand Down
54 changes: 43 additions & 11 deletions lua/nvim-tree/utils.lua
Original file line number Diff line number Diff line change
Expand Up @@ -307,25 +307,57 @@ function M.key_by(tbl, key)
return keyed
end

---Execute callback timeout ms after the lastest invocation with context. Waiting invocations for that context will be discarded. Caller should this ensure that callback performs the same or functionally equivalent actions.
local function timer_stop_close(timer)
if timer:is_active() then
timer:stop()
end
if not timer:is_closing() then
timer:close()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual reference count handling was not necessary; libuv will clear the last reference on handle close.

end
end

---Execute callback timeout ms after the lastest invocation with context.
---Waiting invocations for that context will be discarded.
---Invocation will be rescheduled while a callback is being executed.
---Caller must ensure that callback performs the same or functionally equivalent actions.
---
---@param context string identifies the callback to debounce
---@param timeout number ms to wait
---@param callback function to execute on completion
function M.debounce(context, timeout, callback)
if M.debouncers[context] then
pcall(uv.close, M.debouncers[context])
-- all execution here is done in a synchronous context; no thread safety required
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested exhaustively using vim.is_thread()


M.debouncers[context] = M.debouncers[context] or {}
local debouncer = M.debouncers[context]

-- cancel waiting or executing timer
if debouncer.timer then
timer_stop_close(debouncer.timer)
end

M.debouncers[context] = uv.new_timer()
M.debouncers[context]:start(
timeout,
0,
vim.schedule_wrap(function()
M.debouncers[context]:close()
M.debouncers[context] = nil
local timer = uv.new_timer()
debouncer.timer = timer
timer:start(timeout, 0, function()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change: perform all our control operations in the direct callback instead of during scheduled execution.

timer_stop_close(timer)

-- reschedule when callback is running
if debouncer.executing then
M.debounce(context, timeout, callback)
return
end

-- call back at a safe time
debouncer.executing = true
vim.schedule(function()
callback()
debouncer.executing = false

-- no other timer waiting
if debouncer.timer == timer then
M.debouncers[context] = nil
end
end)
)
end)
end

function M.focus_file(path)
Expand Down
Loading