Add option to show diagnostics as annotations

[rchl]

Add an show_diagnostics_annotations_severity_level option to show diagnostics as annotations.

[rchl]

• It can be noisy
• Diagnostic annotations have priority over code action annotations so code actions are hidden behind "hover" action. Or possibly it will even be random which ones are prioritized.
• Ideally the most severe diagnostics would be prioritized (showed without hovering) but it's the opposite way right now since annotations and regions are added by the same add_regions call, and regions need to be added in the opposite order to annotations

[daggy1234]

Hey super excited for this feature. Just a few caveats:

1. It would be nice to make the colors less dark, perhaps more transparency
2. Some more padding on the surrounding boxes (just for readability)
3. Perhaps like errorlens color the text as well? (white doesn't go well on all colors) . By which I mean a much lighter background accent color on which darker colored text that matches goes.

These would be my choices however, feel free to interject!

[rchl]

• It would be nice to make the colors less dark, perhaps more transparency

I don't think it's possible to apply transparency to the background of those annotations.

2. Some more padding on the surrounding boxes (just for readability)

Where exactly? Top and bottom padding is probably not a good idea because then it could get taller than line height.

3. Perhaps like errorlens color the text as well? (white doesn't go well on all colors) . By which I mean a much lighter background accent color on which darker colored text that matches goes.

Can experiment with that.

[rchl]

This is with the text color defined and no background. Compare with the first screenshot.

[Zomatree]

When hovering over an annotation it seems all other annotation's text color changed:

[rchl]

When hovering over an annotation it seems all other annotation's text color changed:

Yes. Looks like annotations don't play that well with custom styling.
It works better when just overriding text color. Just pushed that change.

[Rapptz]

Thanks for taking the time to implement this! At first glance, I like it. Though I imagine this would probably require new settings for people who don't prefer this view. Error lens lets you configure the range of severity that you care to show. Obviously there aren't any settings since this is just a draft PR.

Personally I couldn't get it to show all diagnostics, just the error ones. Not sure why that is.

[daggy1234]

This is with the text color defined and no background. Compare with the first screenshot.

Looks fantastic!

[eproxus]

How would this look if you actually had a more "normal" line width where errors overlap the code? (I realize some people might have very wide editor panes, but I certainly don't).

[rchl]

The errors would never overlap the code. Those are always shortened to fit the available space.

[rchl]

Still experimenting.

Brought back the code from the original @rwols' branch that showed annotation only for the diagnostic under the cursor.

I think the behavior of this should be customisable. So at least have those options:

• show annotations for all active diagnostics
• show annotation only for the diagnostic under the cursor
• ...?

The fact that we can't control what has priority in the annotation when there are multiple in the same region is a bit of a problem because what can happen is that on moving the cursor to a diagnostic, the diagnostic annotation will show up for a moment only to be replaced by code actions a moment later.

I feel like to solve that properly we would need either some support for prioritizing in the ST API or have a proper region/diagnostic manager class that would control the order and "drawing" of all those things from one central place. That would be major undertaking but I think that would also be needed if we would want to have some proper "problems panel".

[rchl]

I feel like to solve that properly [...] have a proper region/diagnostic manager class that would control the order and "drawing" of all those things from one central place. That would be major undertaking but I think that would also be needed if we would want to have some proper "problems panel".

One more advantage - should allow implementing proper support for navigating to next/previous diagnostic instead of relying on native next/previous_result commands which don't always work as expected.

[dmilith]

As an alternative, the Rust-Enhanced package does a similar thing already (just not inline): https://github.com/rust-lang/rust-enhanced

[rchl]

Specific to Rust so not much of an alternative for many, is it? :)

[TerminalFi]

Have we played with showing diagnostics as phantoms below the issue?

[rchl]

Have we played with showing diagnostics as phantoms below the issue?

I would consider this to be probably the worst option since it would make the content jump up and down constantly and create gaps in the code that would make it less readable.

[TerminalFi]

Have we played with showing diagnostics as phantoms below the issue?

I would consider this to be probably the worst option since it would make the content jump up and down constantly and create gaps in the code that would make it less readable.

I might work on an implementation for POC. I personal would prefer it based on my hacked together implementation. I don't image code jumping a ton unless you have a ton of diagnostics.

[TerminalFi]

Just for reference

[rchl]

I think I'm ready with this feature (diagnostic annotations).

It will conflict with code action annotations but documentation states that those should not be used together.

Otherwise its perhaps a bit flickery on editing the view but I think that's acceptable. And it's not enabled by default.

[predragnikolic]

Q1: When navigating with the lsp_next_diagnostics and when show_diagnostics is set to annotation,
Would it be better to not show the hover popup that displays only the diagnostics in that case? (to avoid showing duplicate information in the hover popup and annotation)

LSP/plugin/core/registry.py

Line 311 in 84a4c2e

sublime.set_timeout_async(lambda: view.run_command('lsp_hover', {'only_diagnostics': True, 'point': diag_pos}), 200)

---

My initial answer to that question would be to hide the popup,
but than I changed my mind.

Annotation are not always visible, if the lines are wide...
See the video below:
Screencast from 2023-06-11 14-27-41.webm

As you can see, the annotation is not visible,
and showing the popup helps see what the problem is immediately.

I could use the mouse,
and hover over the squiggly underline in the view (or the small annotation that is peaking from the right side)
to trigger the annotation to show the text. (but that behavior could sometime have a buggy behavior to show a annotation on the wrong line ... see the video below)
Screencast from 2023-06-11 14-35-28.webm

[predragnikolic]

All in all, i haven't noticed any real issues, while testing this today.

I will test this PR in practice this week,
and let you know how it goes. :)

[rchl]

My initial answer to that question would be to hide the popup,
but than I changed my mind.

Looks like you've answered yourself. :)
I agree that the popup should still show as annotation can only show limited number of characters so it might or might not show the whole info.

[jwortmann]

I think it was mentioned before, but wouldn't it be better to only show the annotation for the diagnostic under the cursor? Or maybe for all diagnostics on the line of the cursor (i.e. even if the cursor is not directly on a diagnostic, but somewhere on the same line).

So this would be an alternative to showing the active diagnostic in the status bar.

Ideally it should be configurable, so perhaps a setting with string value would be better, like

  // Allowed options:
  // - "all"
  // - "active_line"
  // - "under_cursor"  (maybe)
  // - ""
  "show_diagnostics_annotations": "",

[rchl]

I think it was mentioned before, but wouldn't it be better to only show the annotation for the diagnostic under the cursor? Or maybe for all diagnostics on the line of the cursor (i.e. even if the cursor is not directly on a diagnostic, but somewhere on the same line).

Yeah, that's a valid feature request and I had code for that on this branch (not sure if fully finished) but I think that showing all diagnostics is the more basic feature and generally more useful since editing the code can often result in diagnostics appearing on other lines so it's useful to see them without having to move to that line.

[predrag-codetribe]

Usability feedback,
after using it for three days.

The good.
It is ok if there are not a lot of errors in the view.

The awkward.

If each line has errors,
the annotations can be really distracting:

Yesterday, I opened a file... I could not focus on the code, because the red annotations on each line, spanning long red text from the right of the screen...
My whole screen was filled with text, (code coming from left[which is ok], and annotations coming from the right, basically there was no blank space in the view :D )

My subjective conclusion.
I would not use this feature,
although I find it cool.

Keep in mind, this feedback comes from a Person who likes to see less stuff on the screen. (example. I like to hide the line numbers)

---

I approve it.
Although If the LSP enabled a way for plugins to hook into it,
I would delegate this feature to the a plugin, than inside LSP. (but that is a different topic, and requires a lot of thinking)

[rchl]

I suppose that proper code in the projects we are working on should not have files that have errors on each line. If they do, we would probably fix them. So it would be more of an issue for some random third party or minimized files that happen to show diagnostics? In that case normal diagnostic regions would also be distracting (maybe not as much) and maybe the solution to that is to be able to disable diagnostics temporarily or per file?

[jwortmann]

I agree that the annotations can be quite distracting. That's why I thought that only showing it on the active line might be a better default implementation. Even then, I would personally probably not use this feature, because I find it too intrusive and when you are currently typing there is most of the time an error diagnostic for incomplete statement, unused variable, or similar. This situation might improve if we would figure out a solution to #2117 (i.e. don't show new diagnostics immediately), but I don't think there is really a way to solve that problem - the set of diagnostics would need to be compared to the previous ones, and therefore I think each individual diagnostic would need to have an "id" given by the language server, because we can't use the (possibly changing) region to compare them.

I suppose that proper code in the projects we are working on should not have files that have errors on each line.

Maybe not errors, but there can be many "info" or "hint" diagnostics that you can safely ignore, like in the screenshot above for alleged spelling errors or for example for "unused variable or function argument". So maybe another possible alternative for the setting value would be to use an integer:

  // Show diagnostics as annotation with level equal to or less than:
  // none: 0 (never show)
  // error: 1
  // warning: 2
  // info: 3
  // hint: 4
  "show_diagnostics_annotations_severity_level": 0,

Then you could at least restrict it to show only errors for example.

I'm fine with merging this PR since it's not enabled by default (I haven't reviewed the code though), but I think defining the setting in the most useful way is important, because it's annoying to change it afterwards. So just something to consider and discuss at least.

[rchl]

Switched to show_diagnostics_annotations_severity_level and made it so that regions are created per-session which mainly makes a difference when running multiple servers in a single file.

[FichteFoll]

Just a quick question here, now that it is going to be shipped in the next update, is it or will it also be possible to configure the annotations in a way that they only show an error code for example? I configured that for SublimeLinter and it makes the annotations much less noisy. Example below:

when hovered:

Of course the full message also still shows in a popup when hovering over the annotated region.

Relevant configuration.

[rchl]

I suppose you could crate a feature request for it but my initial thought is that this wouldn't work well as a global LSP setting as with many servers the error code won't be present or won't mean much to most. So this would probably need to be a per-server configuration.

[FichteFoll]

my initial thought is that this wouldn't work well as a global LSP setting as with many servers the error code won't be present

Yeah, this was also my suspicion. Thanks for clarifying. I'll think of a proper feature request once I've had the opportunity to test it.