Invalidate active diagnostics when they are removed

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

crates/editor/src/display_map.rs           |  10 
crates/editor/src/display_map/block_map.rs |  19 +
crates/editor/src/element.rs               |   2 
crates/editor/src/lib.rs                   | 229 +++++++++++++----------
crates/lsp/src/lib.rs                      |   1 
5 files changed, 148 insertions(+), 113 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -8,7 +8,10 @@ pub use block_map::{BlockDisposition, BlockId, BlockProperties, BufferRows, Chun
 use block_map::{BlockMap, BlockPoint};
 use buffer::Rope;
 use fold_map::{FoldMap, ToFoldPoint as _};
-use gpui::{fonts::FontId, AppContext, Entity, ModelContext, ModelHandle};
+use gpui::{
+    fonts::{FontId, HighlightStyle},
+    AppContext, Entity, ModelContext, ModelHandle,
+};
 use language::{Anchor, Buffer, Point, ToOffset, ToPoint};
 use std::{
     collections::{HashMap, HashSet},
@@ -131,9 +134,10 @@ impl DisplayMap {
         block_map.insert(blocks, cx)
     }
 
-    pub fn restyle_blocks<F>(&mut self, styles: HashMap<BlockId, Option<F>>)
+    pub fn restyle_blocks<F1, F2>(&mut self, styles: HashMap<BlockId, (Option<F1>, Option<F2>)>)
     where
-        F: 'static + Fn(&AppContext) -> BlockStyle,
+        F1: 'static + Fn(&AppContext) -> Vec<(usize, HighlightStyle)>,
+        F2: 'static + Fn(&AppContext) -> BlockStyle,
     {
         self.block_map.restyle(styles);
     }

crates/editor/src/display_map/block_map.rs 🔗

@@ -52,7 +52,7 @@ pub struct Block {
     id: BlockId,
     position: Anchor,
     text: Rope,
-    build_runs: Option<Arc<dyn Fn(&AppContext) -> Vec<(usize, HighlightStyle)>>>,
+    build_runs: Mutex<Option<Arc<dyn Fn(&AppContext) -> Vec<(usize, HighlightStyle)>>>>,
     build_style: Mutex<Option<Arc<dyn Fn(&AppContext) -> BlockStyle>>>,
     disposition: BlockDisposition,
 }
@@ -333,12 +333,16 @@ impl BlockMap {
         *transforms = new_transforms;
     }
 
-    pub fn restyle<F>(&mut self, mut styles: HashMap<BlockId, Option<F>>)
+    pub fn restyle<F1, F2>(&mut self, mut styles: HashMap<BlockId, (Option<F1>, Option<F2>)>)
     where
-        F: 'static + Fn(&AppContext) -> BlockStyle,
+        F1: 'static + Fn(&AppContext) -> Vec<(usize, HighlightStyle)>,
+        F2: 'static + Fn(&AppContext) -> BlockStyle,
     {
         for block in &self.blocks {
-            if let Some(build_style) = styles.remove(&block.id) {
+            if let Some((build_runs, build_style)) = styles.remove(&block.id) {
+                *block.build_runs.lock() = build_runs.map(|build_runs| {
+                    Arc::new(build_runs) as Arc<dyn Fn(&AppContext) -> Vec<(usize, HighlightStyle)>>
+                });
                 *block.build_style.lock() = build_style.map(|build_style| {
                     Arc::new(build_style) as Arc<dyn Fn(&AppContext) -> BlockStyle>
                 });
@@ -433,7 +437,7 @@ impl<'a> BlockMapWriter<'a> {
                     id,
                     position,
                     text: block.text.into(),
-                    build_runs: block.build_runs,
+                    build_runs: Mutex::new(block.build_runs),
                     build_style: Mutex::new(block.build_style),
                     disposition: block.disposition,
                 }),
@@ -800,6 +804,7 @@ impl<'a> BlockChunks<'a> {
 
         let mut runs = block
             .build_runs
+            .lock()
             .as_ref()
             .zip(cx)
             .map(|(build_runs, cx)| build_runs(cx))
@@ -1031,9 +1036,9 @@ mod tests {
                 position: Anchor::min(),
                 text: "one!\ntwo three\nfour".into(),
                 build_style: Mutex::new(None),
-                build_runs: Some(Arc::new(move |_| {
+                build_runs: Mutex::new(Some(Arc::new(move |_| {
                     vec![(3, red.into()), (6, Default::default()), (5, blue.into())]
-                })),
+                }))),
                 disposition: BlockDisposition::Above,
             }),
         };

crates/editor/src/element.rs 🔗

@@ -613,7 +613,7 @@ impl EditorElement {
                     }
 
                     let underline = if let Some(severity) = chunk.diagnostic {
-                        Some(super::diagnostic_style(severity, false, style).text)
+                        Some(super::diagnostic_style(severity, true, style).text)
                     } else {
                         highlight_style.underline
                     };

crates/editor/src/lib.rs 🔗

@@ -345,6 +345,7 @@ struct ActiveDiagnosticGroup {
     blocks: HashMap<BlockId, Diagnostic>,
     group_range: Range<Anchor>,
     is_valid: bool,
+    update_count: usize,
 }
 
 #[derive(Serialize, Deserialize)]
@@ -2251,80 +2252,7 @@ impl Editor {
                 });
 
             if let Some((primary_range, group_id)) = next_group {
-                self.dismiss_diagnostics(cx);
-                self.active_diagnostics = self.display_map.update(cx, |display_map, cx| {
-                    let buffer = self.buffer.read(cx);
-
-                    let mut group_end = Point::zero();
-                    let diagnostic_group = buffer
-                        .diagnostic_group::<Point>(group_id)
-                        .map(|(range, diagnostic)| {
-                            if range.end > group_end {
-                                group_end = range.end;
-                            }
-                            (range, diagnostic.clone())
-                        })
-                        .collect::<Vec<_>>();
-
-                    let group_range = buffer.anchor_after(diagnostic_group[0].0.start)
-                        ..buffer.anchor_before(group_end);
-                    let primary_range = buffer.anchor_after(primary_range.start)
-                        ..buffer.anchor_before(primary_range.end);
-                    let mut primary_message = None;
-
-                    let blocks = display_map
-                        .insert_blocks(
-                            diagnostic_group.iter().map(|(range, diagnostic)| {
-                                let build_settings = self.build_settings.clone();
-                                let message_len = diagnostic.message.len();
-                                let severity = diagnostic.severity;
-                                if diagnostic.is_primary {
-                                    primary_message = Some(diagnostic.message.clone());
-                                }
-                                BlockProperties {
-                                    position: range.start,
-                                    text: diagnostic.message.as_str(),
-                                    build_runs: Some(Arc::new({
-                                        let build_settings = build_settings.clone();
-                                        move |cx| {
-                                            let settings = build_settings.borrow()(cx);
-                                            vec![(
-                                                message_len,
-                                                diagnostic_style(severity, false, &settings.style)
-                                                    .text
-                                                    .into(),
-                                            )]
-                                        }
-                                    })),
-                                    build_style: Some(Arc::new({
-                                        let build_settings = build_settings.clone();
-                                        move |cx| {
-                                            let settings = build_settings.borrow()(cx);
-                                            diagnostic_style(severity, false, &settings.style).block
-                                        }
-                                    })),
-                                    disposition: BlockDisposition::Below,
-                                }
-                            }),
-                            cx,
-                        )
-                        .into_iter()
-                        .zip(
-                            diagnostic_group
-                                .into_iter()
-                                .map(|(_, diagnostic)| diagnostic),
-                        )
-                        .collect();
-
-                    Some(ActiveDiagnosticGroup {
-                        primary_range,
-                        primary_message: primary_message.unwrap(),
-                        group_range,
-                        blocks,
-                        is_valid: true,
-                    })
-                });
-
+                self.activate_diagnostics(group_id, cx);
                 self.update_selections(
                     vec![Selection {
                         id: selection.id,
@@ -2349,39 +2277,138 @@ impl Editor {
     fn refresh_active_diagnostics(&mut self, cx: &mut ViewContext<Editor>) {
         if let Some(active_diagnostics) = self.active_diagnostics.as_mut() {
             let buffer = self.buffer.read(cx);
-            let primary_range_start = active_diagnostics.primary_range.start.to_offset(buffer);
-            let matching_diagnostic = buffer
-                .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone())
-                .find_map(|(range, diagnostic)| {
-                    if diagnostic.is_primary
-                        && range.start == primary_range_start
-                        && diagnostic.message == active_diagnostics.primary_message
-                    {
-                        Some(diagnostic.group_id)
-                    } else {
-                        None
+            let update_count = buffer.diagnostics_update_count();
+            if update_count > active_diagnostics.update_count {
+                active_diagnostics.update_count = update_count;
+                let primary_range_start = active_diagnostics.primary_range.start.to_offset(buffer);
+                let is_valid = buffer
+                    .diagnostics_in_range::<_, usize>(active_diagnostics.primary_range.clone())
+                    .any(|(range, diagnostic)| {
+                        diagnostic.is_primary
+                            && range.start == primary_range_start
+                            && diagnostic.message == active_diagnostics.primary_message
+                    });
+
+                if is_valid != active_diagnostics.is_valid {
+                    active_diagnostics.is_valid = is_valid;
+                    let mut new_styles = HashMap::new();
+                    for (block_id, diagnostic) in &active_diagnostics.blocks {
+                        let severity = diagnostic.severity;
+                        let message_len = diagnostic.message.len();
+                        new_styles.insert(
+                            *block_id,
+                            (
+                                Some({
+                                    let build_settings = self.build_settings.clone();
+                                    move |cx: &AppContext| {
+                                        let settings = build_settings.borrow()(cx);
+                                        vec![(
+                                            message_len,
+                                            diagnostic_style(severity, is_valid, &settings.style)
+                                                .text
+                                                .into(),
+                                        )]
+                                    }
+                                }),
+                                Some({
+                                    let build_settings = self.build_settings.clone();
+                                    move |cx: &AppContext| {
+                                        let settings = build_settings.borrow()(cx);
+                                        diagnostic_style(severity, is_valid, &settings.style).block
+                                    }
+                                }),
+                            ),
+                        );
                     }
-                });
-            if let Some(matching_diagnostic) = matching_diagnostic {
-            } else if active_diagnostics.is_valid {
-                let mut new_styles = HashMap::new();
-                for (block_id, diagnostic) in &active_diagnostics.blocks {
-                    let build_settings = self.build_settings.clone();
-                    let severity = diagnostic.severity;
-                    new_styles.insert(
-                        *block_id,
-                        Some(move |cx: &AppContext| {
-                            let settings = build_settings.borrow()(cx);
-                            diagnostic_style(severity, false, &settings.style).block
-                        }),
-                    );
+                    self.display_map
+                        .update(cx, |display_map, _| display_map.restyle_blocks(new_styles));
                 }
-                self.display_map
-                    .update(cx, |display_map, _| display_map.restyle_blocks(new_styles));
             }
         }
     }
 
+    fn activate_diagnostics(&mut self, group_id: usize, cx: &mut ViewContext<Self>) {
+        self.dismiss_diagnostics(cx);
+        self.active_diagnostics = self.display_map.update(cx, |display_map, cx| {
+            let buffer = self.buffer.read(cx);
+
+            let update_count = buffer.diagnostics_update_count();
+            let mut primary_range = None;
+            let mut primary_message = None;
+            let mut group_end = Point::zero();
+            let diagnostic_group = buffer
+                .diagnostic_group::<Point>(group_id)
+                .map(|(range, diagnostic)| {
+                    if range.end > group_end {
+                        group_end = range.end;
+                    }
+                    if diagnostic.is_primary {
+                        primary_range = Some(range.clone());
+                        primary_message = Some(diagnostic.message.clone());
+                    }
+                    (range, diagnostic.clone())
+                })
+                .collect::<Vec<_>>();
+            let primary_range = primary_range.unwrap();
+            let primary_message = primary_message.unwrap();
+
+            let group_range =
+                buffer.anchor_after(diagnostic_group[0].0.start)..buffer.anchor_before(group_end);
+            let primary_range =
+                buffer.anchor_after(primary_range.start)..buffer.anchor_before(primary_range.end);
+
+            let blocks = display_map
+                .insert_blocks(
+                    diagnostic_group.iter().map(|(range, diagnostic)| {
+                        let build_settings = self.build_settings.clone();
+                        let message_len = diagnostic.message.len();
+                        let severity = diagnostic.severity;
+                        BlockProperties {
+                            position: range.start,
+                            text: diagnostic.message.as_str(),
+                            build_runs: Some(Arc::new({
+                                let build_settings = build_settings.clone();
+                                move |cx| {
+                                    let settings = build_settings.borrow()(cx);
+                                    vec![(
+                                        message_len,
+                                        diagnostic_style(severity, true, &settings.style)
+                                            .text
+                                            .into(),
+                                    )]
+                                }
+                            })),
+                            build_style: Some(Arc::new({
+                                let build_settings = build_settings.clone();
+                                move |cx| {
+                                    let settings = build_settings.borrow()(cx);
+                                    diagnostic_style(severity, true, &settings.style).block
+                                }
+                            })),
+                            disposition: BlockDisposition::Below,
+                        }
+                    }),
+                    cx,
+                )
+                .into_iter()
+                .zip(
+                    diagnostic_group
+                        .into_iter()
+                        .map(|(_, diagnostic)| diagnostic),
+                )
+                .collect();
+
+            Some(ActiveDiagnosticGroup {
+                primary_range,
+                primary_message,
+                group_range,
+                blocks,
+                is_valid: true,
+                update_count,
+            })
+        });
+    }
+
     fn dismiss_diagnostics(&mut self, cx: &mut ViewContext<Self>) {
         if let Some(active_diagnostic_group) = self.active_diagnostics.take() {
             self.display_map.update(cx, |display_map, cx| {

crates/lsp/src/lib.rs 🔗

@@ -136,7 +136,6 @@ impl LanguageServer {
                         buffer.resize(message_len, 0);
                         stdout.read_exact(&mut buffer).await?;
 
-                        println!("{}", std::str::from_utf8(&buffer).unwrap());
                         if let Ok(AnyNotification { method, params }) =
                             serde_json::from_slice(&buffer)
                         {