Diagnostic Indicators Not Updating

[alex-courtis]

Description

Diagnostic signs are eventually showing correctly however they take some time.

Regression since 82ab19e

Can I please leave this with you @des-b ?

Neovim version

: ; nvim --version
NVIM v0.10.2
Build type: RelWithDebInfo
LuaJIT 2.1.1727870382

Operating system and version

Linux 6.11.5-arch1-1

Windows variant

No response

nvim-tree version

82ab19e

Clean room replication

vim.g.loaded_netrw = 1
vim.g.loaded_netrwPlugin = 1

vim.cmd([[set runtimepath=$VIMRUNTIME]])
vim.cmd([[set packpath=/tmp/nvt-min/site]])
local package_root = "/tmp/nvt-min/site/pack"
local install_path = package_root .. "/packer/start/packer.nvim"
local function load_plugins()
  require("packer").startup({
    {
      "wbthomason/packer.nvim",
      "nvim-tree/nvim-tree.lua",
      "nvim-tree/nvim-web-devicons",
      -- ADD PLUGINS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
    },
    config = {
      package_root = package_root,
      compile_path = install_path .. "/plugin/packer_compiled.lua",
      display = { non_interactive = true },
    },
  })
end
if vim.fn.isdirectory(install_path) == 0 then
  print("Installing nvim-tree and dependencies.")
  vim.fn.system({ "git", "clone", "--depth=1", "https://github.com/wbthomason/packer.nvim", install_path })
end
load_plugins()
require("packer").sync()
vim.cmd([[autocmd User PackerComplete ++once echo "Ready!" | lua setup()]])
vim.opt.termguicolors = true
vim.opt.cursorline = true

-- MODIFY NVIM-TREE SETTINGS THAT ARE _NECESSARY_ FOR REPRODUCING THE ISSUE
_G.setup = function()
  require("nvim-tree").setup({
    diagnostics = {
      enable = true,
    }
  })
end

-- UNCOMMENT this block for diagnostics issues, substituting pattern and cmd as appropriate.
-- Requires diagnostics.enable = true in setup.
vim.api.nvim_create_autocmd("FileType", {
  pattern = "lua",
  callback = function()
    vim.lsp.start {
      name = "my-luals",
      cmd = { "lua-language-server" },
      root_dir = vim.loop.cwd(),
    }
  end,
})

Steps to reproduce

Change to nvim-tree source
echo "foo bar" > lua/nvim-tree/notify.lua
nvim -nu /tmp/nvt-min.lua lua/nvim-tree/log.lua
:NvimTreeFindFile

Expected behavior

notify.lua should be marked with a red diagnostic sign

Actual behavior

:w on log.lua will update the status

[alex-courtis]

For a dramatic example, set diagnostics.show_on_dirs and change "undefined-field": "None", to Any in .luarc.json

[des-b]

Sorry, I did not test/compare enough with the old behavior. The diagnostic will show up after saving for example. But this is clearly a regression.

The issue is this line:

nvim-tree.lua/lua/nvim-tree/diagnostics.lua

Line 175 in 610a1c1

and vim.api.nvim_get_option_value("buflisted", { buf = bufnr })

I am not sure if I fully understand bufnr in update(). I thought this must be the buffer which caused the diagnostic. But this is not the case. It seems to be the currently viewed buffer. (can be any listed file buffer or NvimTree-buffer itself) Not sure if checks (which where already there) actually make that much sense.

With the following debug output in update():

    if bufnr then
      print(vim.inspect(vim.api.nvim_buf_get_name(bufnr)))
    end

...you can see a lot of output with <cwd>/NvimTree_1 and some of actual buffers, e.g. after save. (after removing the mentioned line)

So it would help to remove the linked line above. The downside of this is that if some neovim user decides to attach an LSP to filetype=NvimTree (or filetype=* ) the flashing will appear again. (but at least this is now reproducible 🫣😂)

Not sure how complex it will become but it maybe possible to actually fix this by filtering on the DiagnosticChanged event argument object. This contains the path and buf of the nvim-tree buffer. I am not sure what the filtering will look like. And one would need to check if CocDiagnosticChange provides the same information.

[alex-courtis]

I am not sure if I fully understand bufnr in update(). I thought this must be the buffer which caused the diagnostic. But this is not the case.

That sounds right - I was similarly misdirected. bufnr is indeed the tree rather than the diagnostic buf.

should_draw is always evaluating false. Experiment never raises the error:

    local should_draw = bufnr
      and vim.api.nvim_buf_is_valid(bufnr)
      and vim.api.nvim_buf_is_loaded(bufnr)
      and vim.api.nvim_get_option_value("buflisted", { buf = bufnr })
    if should_draw then
      error("diagnostics calling explorer draw")
      local explorer = core.get_explorer()

Not sure how complex it will become but it maybe possible to actually fix this by filtering on the DiagnosticChanged event argument object.

I like that idea; all the information is present in the event, and only contains the buffers whose diagnostics have changed e.g.

  if opts.diagnostics.enable then
    create_nvim_tree_autocmd("DiagnosticChanged", {
      callback = function(data)
        log.line("diagnostics", "DiagnosticChanged %s", vim.inspect(data))
        require("nvim-tree.diagnostics").update()

One problem was present, added a second.
Both trailing spaces are identified for the buffer.
Other buffers with problems are not shown in the event.
buf appears to be the lsp buffer number however the documentation does not specify.

[2024-11-08 10:14:54] [diagnostics] DiagnosticChanged {
  buf = 3,
  data = {
    diagnostics = { {
        bufnr = 3,
        code = "trailing-space",
        col = 38,
        end_col = 39,
        end_lnum = 0,
        lnum = 0,
        message = "Line with trailing space.",
        namespace = 7,
        severity = 4,
        source = "Lua Diagnostics.",
        user_data = {
          lsp = {
            code = "trailing-space",
            message = "Line with trailing space.",
            range = {
              ["end"] = {
                character = 39,
                line = 0
              },
              start = {
                character = 38,
                line = 0
              }
            },
            severity = 4,
            source = "Lua Diagnostics."
          }
        }
      }, {
        bufnr = 3,
        code = "trailing-space",
        col = 38,
        end_col = 40,
        end_lnum = 1,
        lnum = 1,
        message = "Line with trailing space.",
        namespace = 7,
        severity = 4,
        source = "Lua Diagnostics.",
        user_data = {
          lsp = {
            code = "trailing-space",
            message = "Line with trailing space.",
            range = {
              ["end"] = {
                character = 40,
                line = 1
              },
              start = {
                character = 38,
                line = 1
              }
            },
            severity = 4,
            source = "Lua Diagnostics."
          }
        }
      } }
  },
  event = "DiagnosticChanged",
  file = "/home/alex/src/nvim-tree/master/lua/nvim-tree/api.lua",
  group = 10,
  id = 15,
  match = "/home/alex/src/nvim-tree/master/lua/nvim-tree/api.lua"
}

[alex-courtis]

And one would need to check if CocDiagnosticChange provides the same information.

Let's get a good solution for LSP first, as it's the primary use case. We can adapt for COC afterwards: it's much simpler with a shotgun approach.

I'd be most grateful for a PR!

[alex-courtis]

I've added you as a collaborator so that you can push branches and always run CI.

[des-b]

Okay, I will have a look into this. But this may take some time, since I am a few days before entering vacation. Should I create a PR that removes and vim.api.nvim_get_option_value(“buflisted”, { buf = bufnr }) to improve the situation for now?

[alex-courtis]

Should I create a PR that removes and vim.api.nvim_get_option_value(“buflisted”, { buf = bufnr }) to improve the situation for now?

Thank you, that small change sounds like the best thing to do.

[kah-ell]

That's quite a lot of duplicated data in that DiagnosticChanged event, considering how often those are fired.

[alex-courtis]

Yes, it needs a bit of a rethink. Perhaps looking at deltas between events. In the meantime you could increase the debounce time.

…

On Tue, 12 Nov 2024, 04:00 kah-ell, ***@***.***> wrote: That's quite a lot of duplicated data in that DiagnosticChanged event, considering how often those are fired. — Reply to this email directly, view it on GitHub <#2990 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AALPQYSLSBX3KQNUMSFI6CL2ADPB7AVCNFSM6AAAAABRHUCPX6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRYGY2DAMJZGA> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>