-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
fix(diagnostic): allow virtual_{lines,text} cursor exclusivity #33517
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
base: master
Are you sure you want to change the base?
Conversation
Problem: virtual_text diagnostics are great when skimming a file, and virtual_lines are great when "zooming in" on a particular problem. Having both enabled results in duplicate diagnostics on-screen. Solution: This PR expands the behavior of `current_line` for virtual_text and virtual_lines by making `virtual_text.current_line = false` distinct from `nil`. If you set: vim.diagnostic.config({ virtual_text = { current_line = false }, virtual_lines = { current_line = true }, }) With this configuration, virtual_text will be used to display diagnostics until the cursor reaches the same line, at which point they will be hidden and virtual_lines will take its place.
This comment was marked as resolved.
This comment was marked as resolved.
I think a simpler way to implement this would be to check for This will need a docs update too to indicate that
Or as @justinmk suggested, using a string enum instead of a nullable boolean (if we do that we'd also want to change the option for |
This concise framing makes this PR much easier to understand. So I'm in favor of the |
api.nvim_buf_clear_namespace(bufnr, namespace, 0, -1) | ||
|
||
for line, line_diagnostics in pairs(diagnostics) do | ||
local virt_texts = M._get_virt_text_chunks(line_diagnostics, opts) | ||
local should_render_current_line = (opts.current_line == true and line == lnum) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If opts.current_line
is nil
this should also be true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That case is handled by the last part of the condition: or (opts == true or type(opts) == 'table')
Thanks, that does sound a lot simpler. I reimplemented it there and it's a lot cleaner. If you're happy with the code, I'll update the docs. Do you think this needs tests? |
could be a 2-line change in an existing test. docs neovim/runtime/lua/vim/diagnostic.lua Lines 193 to 195 in 99e754a
|
Problem:
virtual_text diagnostics are great when skimming a file, and virtual_lines are great when "zooming in" on a particular problem. Having both enabled results in duplicate diagnostics on-screen.
First reported in #33092, with this image as an example:

Solution:
This PR expands the behavior of
current_line
for virtual_text and virtual_lines by makingvirtual_text.current_line = false
distinct fromnil
. If you set:With this configuration, virtual_text will be used to display diagnostics until the cursor reaches the same line, at which point they will be hidden and virtual_lines will take its place.
The situation in the image above now behaves like this:
Note to maintainers: this is my first PR to neovim and I haven't looked into writing tests for this yet but I will look into it if desired.