editor: Add inline diagnostics feature

[davisp]

Closes #4901

This adds the ability to configure the display of diagnostic messages inline with code similar to how the inline git blame feature works. I'm fairly confident in the implementation in terms of performance/efficiency. I've not noticed any obvious slowdowns due to it even while scrolling quickly through large source files where I've introduced diagnostics to be rendered.

However, I'll be the first to admit that my UI design chops are slim to none. Which is to say I'm expecting to have requests for tweaking the actual UI which I'd be more than happy to integrate.

I'd also like to thank @nilskch for the initial UI mockup code. Having that was rather invaluable in getting this to an actual PR.

A few examples of the feature in use (the scrolling is much smoother than shown, the stutter is an effect of the GIF conversion):

Release Notes:

• Added configurable inline diagnostic support.

[nervenes]

Awesome work! Love this 👍

I'll just leave the link of my discord message for suggested opinions on some of the UI/UX concerns: https://discord.com/channels/869392257814519848/1106226198494859355/1320019106539241543

[ConradIrwin]

Assigning to @SomeoneToIgnore who knows most about Inlays.

Also needs @danilo-leal's input on aesthetics.

[SomeoneToIgnore]

Thank you for another stab at the topic.

I've tinkered with it for some time and can tell that this definitely feels more stable than the previous attempt, I guess due to more quick updates of the diagnostics, so we're on the right track.

The inline tooltip placement feels also better, as does not disrupt the line order, so I feel if we polish its logic enough and make the visuals better, we can merge this as a first iteration.

I have two groups of notes on top of this PR:

• "arhitectural" ones, related to how data for diagnostics rendering is created and stored, and various "when to update this" questions I've posted in the review comments below.

I think we can work on this right away without waiting for Danilo.

• presentation-wise, I'm not a good expert and leave to @danilo-leal to comment more here, but two important things I wanted to note:

1. There's no way to use this feature via the keyboard, which seems odd.

2. I've found the square and + icons more flashy and confusing than helpful.

I would propose to check out the solution Helix did:

https://helix-editor.com/news/release-25-01-highlights/#diagnostics
(notice the settings)

helix.mov

(single errors are not expanded, multiple errors are expanded on selections and have ... trimming overly long lines, no extra icons near the text, almost no flashing and very fast)

In case you're interested to have the file I've used for testing, to repro certain flashing issues:

src/main.rs

// #![allow(unused)];
use std::collections::{HashMap, HashSet};

fn main() {
  if true {
      loop {
          //
      }
      return;
  }

  // let foo = multiply()

  let long_params = LongParams;
  #[rustfmt::skip]
  {
      long_params.layout_visible_cursors(snapshot, selections, visible_display_row_range, line_layouts, text_hitbox, content_origin, scroll_position, scroll_pixel_position, line_height, em_width, autoscroll_containing_element, cx);
  }
  long_params.layout_visible_cursors(
      snapshot,
      selections,
      visible_display_row_range,
      line_layouts,
      text_hitbox,
      content_origin,
      scroll_position,
      scroll_pixel_position,
      line_height,
      em_width,
      autoscroll_containing_element,
      cx,
  );

  let aa = vec![String::new()];
  // testdasds_something();

  // let aa = Test();
  //� let aa = Test();

  let two = "string w";
  // let two = 2;
  // let two = 2;
  // let two = 2;
  // let two = 2;
  // let two = 2;
  // //√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // new
  // new
  // new
  // new
  // new
  // new
  // new
  // new
  // new
  // new
  // //√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
  // //√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√

  // let path = Path::new("/one/two");
  // dbg!(path.strip_prefix(path));

  let st = StructWithDeliberatelyLongNameToBreakThings;
  let mut test_map_identifier_that_is_also_long_to_try_things_more_and_more_and_more_again_because_we_need_a_wrap_agaaaaain
      // this goes after the hint
      = HashMap::new();
  test_map_identifier_that_is_also_long_to_try_things_more_and_more_and_more_again_because_we_need_a_wrap_agaaaaain.insert(st, "test");

  let mut my_random_integers = vec![42, 87, 13, 76, 29, 8, 97, 63, 52, 31];
  my_random_integers.sort_by(|a, b| a.cmp(b));
  dbg!(&my_random_integers);
  my_random_integers.sort_by(|a, b| b.cmp(a));
  dbg!(&my_random_integers);

  let mut my_random_chars = vec!['a', 'x', 'e', 'q', 'z', 'j', 'u', 'l', 'h', 'c'];
  my_random_chars.sort_by(|a, b| a.cmp(b));
  dbg!(&my_random_chars);
  my_random_chars.sort_by(|a, b| b.cmp(a));
  dbg!(&my_random_chars);

  if true {
      // panic!("((((");
      // loop {}
  }
}

fn multiply(one: i32, two: i32) -> i32 {
  one * two
}

///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
///√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√|√√√√√√√√√√√√√√√√
pub fn test_something() {
  //√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√√
}

#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)]
struct StructWithDeliberatelyLongNameToBreakThings;

#[cfg(test)]
mod tests {
  use super::*;

  #[test]
  fn test_name_1() {}

  #[test]
  fn test_name2() {}

  #[test]
  fn test_name4() {
      //
  }
}

trait DebugT {}

// impl<
// impl<T> DebugT for T {}

fn foo2<T>() {
  // let a = 1 ;
}

/// some

fn foo3<T>() {
  // let a = 1 ;
}

fn foo4<T>() {
  // let a = 1 ;
}

fn foo5<T>() {
  // let a = 1 ;
}

fn foo6<T>() {
  // let a = 1 ;
}

fn foo7<T>() {
  // let a = 1 ;
}

fn foo8<T>() {
  // let a = 1 ;
}

fn foo9<T>() {
  // let a = 1 ;
}

fn foo10<T>() {
  // let a = 1 ;
}

fn foo11<T>() {
  // let a = 1 ;
}

fn foo12<T>() {
  // let a = 1 ;
}

fn foo13<T>() {
  // let a = 1 ;
}

fn foo14<T>() {
  // let a = 1 ;
}

fn foo15<T>() {
  // let a = 1 ;
}

fn foo16<T>() {
  // let a = 1 ;
}

fn foo17<T>() {
  // let a = 1 ;
}

fn foo18<T>() {
  // let a = 1 ;
}

fn foo19<T>() {
  // let a = 1 ;
}

fn foo20<T>() {
  // let a = 1 ;
}

fn foo21<T>() {
  // let a = 1 ;
}

fn foo22<T>() {
  // let a = 1 ;
}

fn foo23<T>() {
  // let a = 1 ;
}

fn foo24<T>() {
  // let a = 1 ;
}

fn foo25<T>() {
  // let a = 1 ;
}

// test something too

#[test]
fn feature_1() {
  dbg!("111111111111");
}

#[test]
fn feature_2() {
  dbg!("222222222222");
}

struct LongParams;
impl LongParams {
  fn layout_visible_cursors(
      &self,
      _snapshot: &(),
      _selections: &[((), Vec<()>)],
      _visible_display_row_range: Range<()>,
      _line_layouts: &[()],
      _text_hitbox: &(),
      _content_origin: (),
      _scroll_position: (),
      _scroll_pixel_position: (),
      _line_height: (),
      _em_width: (),
      _autoscroll_containing_element: bool,
      _cx: &mut (),
  ) -> Vec<()> {
      Vec::new()
  }
}

fn long_params(
  _snapshot: &(),
  _selections: &[((), Vec<()>)],
  _visible_display_row_range: Range<()>,
  _line_layouts: &[()],
  _text_hitbox: &(),
  _content_origin: (),
  _scroll_position: (),
  _scroll_pixel_position: (),
  _line_height: (),
  _em_width: (),
  _autoscroll_containing_element: bool,
  _cx: &mut (),
) -> Vec<()> {
  Vec::new()
}

[davisp]

@SomeoneToIgnore Thanks for the review!

I won't be able to get to all of your points until tomorrow or Sunday, but its definitely on the agenda for this weekend.

I did spend some time last night reading through the Helix implementation and also played a bit with it to see how it'd behave in various situations. I'd say my two main take aways from this are that we should absolutely steal their UI mechanics for interacting with the diagnostics and that I'm absolutely not going to invest the time into reimplementing their diagnostic renderer.

I've been using this branch full time for over a week now and have to agree that the hover UI and the square/+ indicators are rather useless. In day to day work I just haven't found any use from the indicators. Just knowing there's an diagnostic to look at is more than enough. Though I do think we should probably add an option for highlighting the entire line to make the diagnostics easier to spot while skimming code during refactoring as its easy to miss things on lines nearly as wide as the editor (or soft-wrapped lines as you spotted, oops).

For the actual diagnostic rendering, I'm not about to follow the Helix path to implementing a custom algorithm. I was hoping they'd just figured out how to get rust-analyzer to given diagnostics with more/better structure/mark up, but instead they went all Sisyphus and wrote their own from scratch (which is fairly impressive, not gonna lie). For instance, here's one random quirk that I found playing with your test file:

Notice that they've rendered right-to-left as top-to-bottom with overlapping lines so its confusing what goes where. Making that work reliably in Rust would be hard enough. Making it work reliably for every language/LSP pair is my personal brand of nightmare fuel.

Covering a few other issues at a high level, there's definitely some funkiness with diagnostics that change or don't on user input. Some diagnostics are instantaneous (i.e., syntax errors) while other things like unused imports only update when clippy runs which obviously depends on the project size. I definitely think there's room for improvement here and you made a number of suggestions that I think will help quite nicely on that front. I'll have to contemplate the ideas on storing more state between frames, I certainly agree that there's some work that needs to be done there, I'm not entirely sure I see how that would work (though if you have a pointer on somewhere in the code base I can reference that'd be helpful).

One last thing was that bit in the flickering example with diagnostics changing on every keypress. What I think you're calling flickering is the fact that the diagnostics are rendered ~instantly, but with differing content so they're changing oddly. I can completely understand why we'd want to avoid that in a UI, but I just wanted to make sure we're on the same page as to the underlying behavior. As near as I can tell, the diagnostics are being rendered ~instantly, its just that the what is rendered is changing so they're going back and forth. I couldn't immediately reproduce this (I'll try again when I dig further) which makes me worry that its a rust-analyzer issue that we'll have to code around. Not the end of the world but not great either.

Also on the topic of performance, you mentioned Helix's speed of rendering. In my (very light) poking, its significantly slower than this PR. I was getting roughly 100ms delays (very unscientifically measured) for the inline diagnostics. No idea if that's just my machine or something with your All The Diagnostics test file.

[davisp]

That's probably it for me for today. I've made some progress and I think responded to all/most comments with hopefully something at least semi rational. Please, do let me know if this comment is a correct summary of what you're suggesting for the architectural changes. I'm pretty sure that I'm reasonably close after reading the entire review a couple times now. And it certainly makes sense.

I didn't quite get to the cursor selection logic so will try and get that knocked out tomorrow and have this PR updated with at least what the final version will look like rendering even if we do go back and update the implementation to be more efficient.

Also, I mentioned it a couple times, but the "select nearest diagnostic" logic I was referring to is this bit that I saw in the hover_popover code. I'm mostly linking that for my own benefit so apologies if that was already obvious.

[SomeoneToIgnore]

Thank you, I've restrained from commenting or resolving things too eagerly as there was no new code pushed, so even the things obsoleted are still hanging in the review.
Feel free to ping me more explicitly if I've missed something important, otherwise I'll try to do minimal activity here until the next code batch arrives.

To me, makes total sense to reuse the existing infra and avoid chasing Helix approach to every detail, that example served as an illustration to two crucial things to me:

• less visual noise
• better keyboard flow

Seems that we agree on both, so it's great for the first iteration.

[davisp]

@SomeoneToIgnore The updates from today include the new UI approach for using the active diagnostics to display what ever is under the cursor or optionally using an action to toggle the active diagnostics. This feels a lot more useful than the hover approach and has the nice benefit of being significantly more modular.

A short probably incomplete list of changes include:

1. The "are inline diagnostics enabled" is now a single boolean that is updated when the settings.json is edited
2. Allow users the ability to activate a diagnostic block based on the cursor movement (does not require inline diagnostics)
3. Allow users to bind a key to toggle the active diagnostic under the cursor (does not require inline diagnostics)
4. Allow users to the ability to show the rendered diagnostic, if it exists (does not require inline diagnostics)
5. Allow users to disable non-primary diagnostic activation (does not require inline diagnostics, non-primary aren't super useful in conjunction with using the rendered diagnostic).
6. Removed all hover behavior and indicator icons

This update does not include changes for the optimized architecture we've discussed, but I'll be working on that either in the evenings this week or perhaps next weekend depending on how real life shakes out.

[SomeoneToIgnore]

Thank you, I really like the presentation of the current hints — zero mouse involvement is so natural, and selection-based reveal seems great, especially the fact that it's so close to existing f8/shift-f8 that even ESC-to-hide and copying works with it.

Besides the recompute-during-render story, a long line bug and new, selection-related issues that I've commented, it seems quite pleasant to work with.

Thank you for keeping with this — there's quite a chunk of work to do still.

[davisp]

@SomeoneToIgnore Thanks for the in-progress re-review on this! Reading through I think we're mostly in agreement on most everything with what feels like fewer open questions. I'll probably not have time to get to more in-depth work on this until Sunday/Monday when I've not got $real_job commitments but so far I think we're coming to a consensus on behaviors so that seems like good progress.

I'm gonna respond to most/all of your comments just to make sure that we're all on the same page for when I get a chunk of time to focus on this. So apologies for the incoming email spam.

[davisp]

I have no idea why GitHub isn't giving me a reply box to this comment about the relative performance traces in Instruments. However, given that we're already planning on optimizing things I'm not sure how much to focus on it.

[SomeoneToIgnore]

I'm not sure how much to focus on it.

I'm almost sure that if we follow the "update the rendering data after debouncing, in the editor, on the DiagnosticsUpdated/other specific event", we're good without any specific optimizations.

[davisp]

@SomeoneToIgnore New update. Highlights include:

1. Remove everything that's not purely just showing diagnostics inline
2. Move all coordinate transformations out of the Element::prepaint method
3. Fix the wrapped line display bug

I've been using this as a daily driver and the automatic diagnostic jumping turned out to be terrible. I've written Rust, Java, Python, and Go and the only place it was remotely useful was in simplistic Rust cases. All the other languages I wrote in would just show the same simple message which just led to a lot of code bouncing around.

I'm pretty sure I've got the heavy parts you were worried about moved out of the painting code paths. The only math that happens while painting is some simple arithmetic to account for element layouts calculated earlier in the painting pipeline.

And lastly, I've got the long wrapped line logic figured out as best as I can. There's still the issue where the line just barely fits in the window without wrapping, but it matches the behavior of the inline git blame so I figure its good enough for now.

[nilskch]

Suggested change

	.bg(sev_to_color(&diagnostic.severity).color(cx).opacity(0.07))
	.bg(sev_to_color(&diagnostic.severity).color(cx).opacity(0.07))
	.px_1()

I really like how this feels now! Can we add a small padding here? I think that makes it a little bit nicer.

Before	After

[RafayAhmad7548]

i am waiting really hard for this to be merged, aaa

[odicho]

Sorry for the unrelated comment, but this PR is amazing. Great work from everyone involved 👏🚀

[SomeoneToIgnore]

Thank you, by stripping most of the topics that caused discussions, we've got a very solid first step, I expect this approach, modified slightly, will land as the first step for the diagnostics.

There's some work to do as, despite getting smaller, it's still a very dynamic hence complex part.
My feedback comes in these main topics:

• NITs about various ways to structure the code
• coordinates (Anchor <-> DisplayRow caveats)
• interaction with settings
• layouting with other elements

Last two items could be most interesting to fix, if you have time this week + a few days after, but hopefully starting next week, I'll be able to help to move this PR forward and push whatever ceremonies around the coordinates and related fixes along the way.

I think we're past the point of design concerns and this indeed looks quite close to be merged, time to think about the design, so we also plan to pair with @danilo-leal on a design part next week, welcome to work on this together, if you're interested.

[SomeoneToIgnore]

A few notes, at least the first one is important to fix in this PR.

• When this TODO is implemented, editor from crates/diagnostics/src/diagnostics.rs where we did editor.set_show_inline_diagnostics(false);, may accidentally get the diagnostics enabled after setting value update.

To avoid this, we need to decouple self.show_inline_diagnostics from settings updates, and keep it enabled programmatically only.
To keep the similar logic in settings_changed, we can add self.latest_settings: Option<ProjectSettings> or similar field, and compare values between settings.
The entire settings_changed value is applicable only for self.mode == EditorMode::Full case, so we can move it above all.

• Another great idea would be to add a button in this menu, for discoverability:

• Overall, we can implement this TODO by rewriting on_diagnostics_updated a bit: we react to multi_buffer::Event::DiagnosticsUpdated in the
  

  zed/crates/editor/src/editor.rs

  Line 13553 in 8d37279

  multi_buffer::Event::DiagnosticsUpdated => {

  so if we do self.refresh_inline_diagnostics(), we get both this place, the reaction to event, and, potentially, an editor action logic covered with one shared method.

I also suspect we might want to consider which active diagnostics are displayed now, to avoid overlap, so another reason to do things this way is that we need to be called after refresh_active_diagnostics.

The editor action method could be similar to what inlay hint toggling is doing in the same menu from above: same idea of showing/not showing inlays/diagnostics in the editor after a single keypress (which is faster than fiddling with the settings).

[SomeoneToIgnore]

Our regular pattern for None as a debounce is "zero debounce" whilst here it's "zero action".

Let's allow updating the diagnostics immediately, as some people might prefer flickering over latency(?).
The regular parttern looks like

zed/crates/assistant_context_editor/src/context.rs

Lines 2947 to 2951 in 8d37279

	self.pending_save = cx.spawn(|this, mut cx| async move {
	if let Some(debounce) = debounce {
	cx.background_executor().timer(debounce).await;
	}

and there, the initial value is Task::ready(()) (or whatever other inner value instead of ()) and the task assignent always happens if the if let Some(debounce) in it.

Then, we can remove if !matches!(event, multi_buffer and whatever unused function parameters, to have a pub fn refresh_inline_diagnostics that can be called from anywhere: settings enabled event, diagnostics updated event, whatever menu click, etc.

Last but not least, merge it with update_inline_diagnostics and you've got a single function doing all that's needed.

[SomeoneToIgnore]

I think we now have a very good set of settings and descriptions inside the default.json above, and we need to update this section respectfully.

I agree with the workflow, where all toggling is made by existing f8/shift-f8 , to potentially jumpy autotoggling or special actions are needed.

One wish I used to voice in another related discussion here: we can add "minimum_level" knob "inline" section, to allow users to show only errors, only errors + warnings, etc.
Similar to people not fond of flickering, there is a notable group of people who are not fond of many popovers/colors around their code and it seems simple to add another .filter inside update_inline_diagnostics to support that + new diagnostics re-queried after level change.

[SomeoneToIgnore]

Sorry for nitpicking, but it's another thing that is done a bit differently in this repo: Rust guidelines do not enforce setter/getter usage around, so many things are accessing fields, as they are pub anyway.

The defaults are set in default.json and sometimes also via #[serde(default = "false_value")] to avoid certain footguns.
Both ways are done on json -> Rust struct conversion, while here we start to add Rust -> Rust logic for defaults, which is unusual in this repo.

Let's uncomment related strings in assets/settings/default.json where defaults are, remove all these getters and use the fields directly.

default.json is baked into the binary with

zed/crates/settings/src/settings.rs

Line 72 in 8d37279

asset_str::<SettingsAssets>("settings/default.json")

and cannot be changed in release builds, so it's safe to consider them as a source of truth for defaults.

[SomeoneToIgnore]

Note to self to help with implementing:

We have to store it in multi_buffer::Anchor as a key, same way as inlay hints are stored, so in a sorted Vec<(Anchor, InlineDiagnostic)> which is extended with either binary_search_by + insert or util::extend_sorted, as Anchor needs MultiBufferSnapshot for comparison.

Editor is a storage, that keeps the data between text transformations, but DisplayRow are more about ephemeral pointer to "what displayed in the editor element now".

Using DisplayRow for rendering is great, as we create a throwaway editor element to render, every frame.
But here, we might keep a state which DisplayRows are not making much sense for the new text inside the buffer.

This outdated state leads to bugs like

undo-redo.mov

(undo-redo leaves jumping empty spaces between diagnostics)

laggy.updates.mov

(with large latency of updates, diagnostics start to leak into the positions they do not belong).

[SomeoneToIgnore]

This is the core place, combining multiple new feedback from this review.

undo-redo.mov

Interesting, I've intuitively expected that clear will cause a lot more issues with flickering, if the diagnostics retrieval is slow due to the amount of the diagnostics, but even on the large example it's not that bad.

I think we're saved here by the fact that Zed already has issues with large amounts of diagnostics, hence we're not seeing the new ones.
I would propose still, to rework this a slightly:

• accumulate new state (inline_diagnostics) first, and mutate the state it in the very end of the task, once
• pass everything that does not depend on self and cx through cx.background_spawn(async move { ... }).await

Combined, something like

let new_inlay_hints = cx.background_spawn(async move {
    let mut new_inlined_diagnostics: Vec<(Anchor, InlineDiagnostic)> = Vec::new();
    let mut prev_diagnostic_line = None;
    for diagnostic in diagnostics {
        //........
        new_inlined_diagnostics..binary_search_by(|probe| {
            diagnostic_anchor.cmp(&probe.0, &buffer)
        });
        //........
    }
}).await;
//........
self.inline_diagnostics = new_inline_diagnostics;

This way, we'll be usually on a background thread, debounced or computing, cancelled on concequent requests.
Old state will be kept, and Anchor (from Editor) -> DisplayPoint (when laying out) conversion will keep the diagnostics placed on the right lines when rendering, even after adding newlines or undoing.

Large diagnostics sets to process will cause more stale diagnostics text during fast editing, but as a somewhat inevitable trade-off, which does not flicker at least.

[SomeoneToIgnore]

I think this boolean check should be kept at an editor level — it can react to settings change fast and clear/fire update task, controlling its state that we query later (editor.inline_diagnostics).

[SomeoneToIgnore]

Here, we need a bit stricter approach to diagnostics deduplication: we need to consider the active diagnostics groups (appearing after f8/shift-f8, disappearing after ESC) and prefer showing in the other lines diagnostics from these groups.

No active diagnostics, last diagnostics on the expected place:

rust-analyzer's group active, last diagnostics is shifted:

rustc's group active, last diagnostics is shifted again:

I'd expect inline diagnostics to be different for different groups, specifically the last one not shown for rust-analyzer's group.
As we "flicker" during expanding active group anyway, changing the inline diagnostics seems not that bad.

Also, this latest diagnostics is different on each screenshot, as it's on outdated DisplayRow pointers are stored in the Editor for the diagnostics.
Anchor would have bound us to "logical place in the text" instead, and, when converted to DisplayRow, would point to the right place always.

[SomeoneToIgnore]

Suggested change

	.id(SharedString::from(format!("diagnostic-{}", row.0)))
	.id(("diagnostic", row.0))

Zero allocation compared to previous version, in the rendering code, so nice to have more of these.

Overall, nice that we've arrived to "just text" and it works just fine as there's f8/shift-f8 to cycle rich diagnostics views right from the cursor.

[SomeoneToIgnore]

There are cases, when new pop-ups are unaware of

• each other (?)

• expanded diagnostics

• other elements that's rendered such as inline completion pop-ups

• In addition to this, I've played with inline git blame and inline diagnostics enabled, and so far I feel the diagnostics are less volatile among everything else: you have to edit the text, disrupting the state around, to get them updated (or toggle them off, which is now possible only through settings json edits)

Overall, feels that we need to not render any inline diagnostics, if anything else is displayed instead of it for this line.
As most of other pop-ups we should remove with ESC or placing the caret elsewhere, but this one is not possible to remove without editing the text — as there's no way to know who authored a particular line via inline blame that has an inline diagnostics always shown instead.