Skip to content
This repository has been archived by the owner on Jul 6, 2021. It is now read-only.

Virtual text not always displaying #40

Closed
Iron-E opened this issue Jul 6, 2020 · 20 comments
Closed

Virtual text not always displaying #40

Iron-E opened this issue Jul 6, 2020 · 20 comments

Comments

@Iron-E
Copy link
Contributor

Iron-E commented Jul 6, 2020

From #36:

Hmm… one thing I am noticing is that g:diagnostic_enable_virtual_text isn't doing anything for me right now (no virtual text is shown, only things in the sign column. Is this happening for you too?

Seems to be happening only on certain language servers. sumneko_lua and bashls are two of them, but interestingly enough omnisharp and gopls work but the diagnostics stay on screen after they should disappear.

I'll add that currently omnisharp is one of the more troubled language servers with nvim_lsp, so it's odd that it's working there.

Popups work on all servers I've tried, and so do items in the sign column.

@aktau
Copy link

aktau commented Jul 6, 2020

For me, gopls has the behaviour where the diagnostics stay on screen even when they should disappear.

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 6, 2020

I can confirm that behavior as well. Are you able to test bashls or sumneko_lua to see if you get the other behaviors from the OP there too?

@aktau
Copy link

aktau commented Jul 7, 2020

I think I can confirm that this bug is not related to virtual text, but to the diagnostics itself. I've switched from using virtual texts to popups on CursorHold (#29) and I see the same phenomenon:

  • I see (two) diagnostics (which are accurate) when opening the file.
  • After removing the line which provoked one diagnostic (and saving), the diagnostic still exists, now on an empty line.

Before:

Screenshot 2020-07-07 at 11 51 12

After:

Screenshot 2020-07-07 at 11 51 32

I can confirm this happens with efm and gopls langservers. I don't have the two others requested by @Iron-E above.

@aktau
Copy link

aktau commented Jul 7, 2020

I tried disabling diagnostic-nvim (commenting out Plug and on_attach) and the situation persists. So this may not be due to diagnostic-nvim.

At this point I'm thinking something may be wrong with util.buf_clear_diagnostics(bufnr) and/or util.buf_diagnostics_save_positions(bufnr, result.diagnostics), though I don't know this code (at all).

Also. I did not know that vanilla nvim-lsp had signs and virtual text: https://github.com/neovim/neovim/blob/91572ddad185c2c4f6d266543d66919543c5bb2a/runtime/lua/vim/lsp/callbacks.lua#L100. As a side note it may be good to specify in the README how this plugin differs/enhances the default experience.

@aktau
Copy link

aktau commented Jul 7, 2020

So, the only artifact that (seems to) remain after calling vim.lsp.util.buf_clear_diagnostics is the stuff fetched by vim.lsp.util.show_line_diagnostic (and Prev/NextDiagnostic which uses everything from the location list without filtering if I'm reading this right).

The former gets its values from vim.lsp.util.get_line_diagnostics, which sources them from M.diagnostics_by_buf[bufnr]. Which is itself filled by vim.lsp.util.buf_diagnostics_save_positions.

Here's M.buf_clear_diagnostics:

  function M.buf_clear_diagnostics(bufnr)
    validate { bufnr = {bufnr, 'n', true} }
    bufnr = bufnr == 0 and api.nvim_get_current_buf() or bufnr

    -- clear sign group
    vim.fn.sign_unplace(sign_ns, {buffer=bufnr})

    -- clear virtual text namespace
    api.nvim_buf_clear_namespace(bufnr, diagnostic_ns, 0, -1)
  end

Which indeed doesn't remove anything from M.diagnostics_by_buf. Perhaps it should...

Adding M.buf_clear_diagnostics[bufnr] = nil at the end of this function fixes the behaviour such that it properly wipes everything, but it doesn't fix everything. Normal plugin behaviour is still messy. Unsure why yet.

@megalithic
Copy link

megalithic commented Jul 7, 2020

confirmed failing for me too.. if you don't include require'diagnostic'.on_attach() as part of your on_attach function that you bind to your lsps, then it works (you still get the virtual text, but of course, other settings don't work at that point).

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 7, 2020

It seems that the virtual text function (lua/diagnostic/util.lua @ line 72) is roughly based on vim.lsp.util.buf_diagnostics_virtual_text(). I expected that the error was somewhere in the difference, although the :Gblame shows me that this repo's version hasn't been touched in several months which means the error must be elsewhere.

@megalithic
Copy link

I'll say that I noticed the "break" when i switched to the new github org of nvim-lua from @haorenW1025 's own account.

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 7, 2020

Do you remember around what commit that was? I noticed a lot of these bugs popped up about that time. I could git bisect if I knew.


Edit: I believe I've figured out where the function is failing:

buffer_line_diagnostics = all_buffer_diagnostics[bufnr]
if not buffer_line_diagnostics then
  return
end
for line, line_diags in pairs(buffer_line_diagnostics) do
   --
end

Here, buffer_line_diagnostics is always an empty table. Because of this, the for loop does not do anything and no virtual text is added to the buffer. This must be related to a change in all_buffer_diagnostics or M.buf_diagnostics_save_positions().

On that note, I see that vim.lsp.util.buf_diagnostics_save_positions() is a function that is currently exposed by the vim.lsp table. Is that something that #18 might be referencing? I also noticed that there was a TODO on all_line_diagnostics that suggested there was a variable in the vim.lsp table which already covered the same purposes. I couldn't find such a variable in the documentation so hopefully someone else can shed some light on this.

@aktau
Copy link

aktau commented Jul 7, 2020

Sorry, after thinking about it for a bit more it may be that my issue is not the same as the issue in this post. I never had the problem that virtual text wasn't always displayed. My problem is that it's sometimes displayed when I think it shouldn't. But after investigating I've found out that:

  1. It has nothing to do with virtual text. The diagnostics just don't go away while they should.
  2. After fixing (what I think is) a bug in buf_clear_diagnostics (see Virtual text not always displaying #40 (comment)). I noticed that now gopls was fixed (diagnostics dissapear correctly) but efm-langserver wasn't.
  3. Investigating efm-langserver shows that it tells vim about these diagnostics, but they are of zero width (see below). This is either a bug in (a) the efm-langserver, (b) how nvim-lsp sends textDocument/didChange to efm-langserver [1] or (c) how nvim-lsp interprets zero-width diagnostics (perhaps they should be ignored?).

So, sorry for the noise. I'll create issues elsewhere once I've dug some more.

For later reference, the full nvim-lsp debugging output.

# Before deleting line 5 (it' indeed has a flaw warranting a diagnostic):
range = {
  end = {          character = 8,          line = 4        },
  start = {          character = 5,          line = 4        }
},

# After deleting line 5, making it now an empty line. This is especially strange because there is no "character = 5" on an empty line.
range = {
  end = {          character = 5,          line = 4        },
  start = {          character = 5,          line = 4        }
},

[1]: Unlikely, checking the debugging output shows that nvim-lsp takes the simple approach of just including the entire file, which makes sense and is allowed. It's hard to mess that up.

@megalithic
Copy link

@haorenW1025 any thoughts around this?

@haorenW1025
Copy link
Collaborator

Sorry for the responding... I was too busy this couple days. However I haven't noticed any issue on my side(mainly using sumneko_lua and clangd). I made a possible fix and please try if it solves your issues.

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 9, 2020

If I understood @aktau's findings correctly, I think that latest commit may fix his issue. Although the things I noticed here are still happening (and that would confirm what he suspected about these being two different issues appearing as one).

@aktau
Copy link

aktau commented Jul 9, 2020

I think d7734f1 is just a partial fix, vim.lsp.util.buf_clear_diagnostics itself also needs to clear the Lua table that contains the diagnostics (what I mentioned in #40 (comment)). However, a fix for that would need to be submitted to the main neovim repository. I'll see if I can make a PR.

@haorenW1025
Copy link
Collaborator

@aktau Yeah the fix should be in the neovim repo, please tag me if you open a PR, thanks:)

@aktau
Copy link

aktau commented Jul 9, 2020

Done. Also, once again I got confused as this issue (as described by @Iron-E) is likely not exactly the same thing as what I'm experiencing. Of course, fixing my bug will mean less potential confusion/causes. From the original report, I only experienced the following (highlighted):

Hmm… one thing I am noticing is that g:diagnostic_enable_virtual_text isn't doing anything for me right now (no virtual text is shown, only things in the sign column. Is this happening for you too?

Seems to be happening only on certain language servers. sumneko_lua and bashls are two of them, but interestingly enough omnisharp and gopls work but the diagnostics stay on screen after they should disappear.

Let's see if the fixes make the second issue go away, and then we could deal with the first (if it can be reproduced).

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 13, 2020

Okay, so I went ahead and performed a local merge of your @aktau's using neovim/neovim as a base (I don't use the appimage anyway). This issue has been fixed:

interestingly enough omnisharp and gopls work but the diagnostics stay on screen after they should disappear.

However this issue still persists for me:

no virtual text is shown, only things in the sign column

and

Seems to be happening only on certain language servers. sumneko_lua and bashls are two of them.

Here are some screenshots to demonstrate the issue.


Edit: I recompiled today and both issues are occurring. I will make note on the PR.

@megalithic
Copy link

confirmed, same behaviour; add elixirls to that list of ones not showing virtualtext, but shows the sign in signcolumn.

@el-iot
Copy link
Contributor

el-iot commented Jul 23, 2020

@haorenW1025 I've made a pull-request that fixes this issue for me, check out #50.

@Iron-E
Copy link
Contributor Author

Iron-E commented Jul 25, 2020

I can confirm that #50 has fixed the issue. I will close it. Thank you!

@Iron-E Iron-E closed this as completed Jul 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants