nvim-tree / nvim-tree.lua

A file explorer tree for neovim written in lua
Other
7.22k stars 610 forks source link

Diagnostic Indicators Not Updating #2990

Open alex-courtis opened 2 days ago

alex-courtis commented 2 days ago

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

20241106_102409

:w on log.lua will update the status

alex-courtis commented 2 days ago

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

des-b commented 2 days ago

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: https://github.com/nvim-tree/nvim-tree.lua/blob/610a1c189bdb2b9b936169b2ea9d1838f971fa2b/lua/nvim-tree/diagnostics.lua#L175

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 commented 20 hours ago

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 commented 20 hours ago

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 commented 20 hours ago

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