Group diagnostics by primary

Render primary message above the excerpt and supporting messages as block decorations with a `Below` disposition. This is still super rough.

Co-Authored-By: Max Brunsfeld <maxbrunsfeld@gmail.com>

crates/chat_panel/src/chat_panel.rs

@@ -13,7 +13,7 @@ use gpui::{
     ViewContext, ViewHandle,
 };
 use postage::{prelude::Stream, watch};
-use std::sync::Arc;
+use std::{rc::Rc, sync::Arc};
 use time::{OffsetDateTime, UtcOffset};
 use util::{ResultExt, TryFutureExt};
 use workspace::Settings;
@@ -56,14 +56,14 @@ impl ChatPanel {
                 4,
                 {
                     let settings = settings.clone();
-                    move |_| {
+                    Rc::new(move |_| {
                         let settings = settings.borrow();
                         EditorSettings {
                             tab_size: settings.tab_size,
                             style: settings.theme.chat_panel.input_editor.as_editor(),
                             soft_wrap: editor::SoftWrap::EditorWidth,
                         }
-                    }
+                    })
                 },
                 cx,
             )

crates/diagnostics/src/diagnostics.rs

@@ -1,7 +1,10 @@
-use std::sync::Arc;
+use std::{cmp, sync::Arc};
 
-use collections::HashMap;
+use editor::{
-use editor::{diagnostic_style, Editor, ExcerptProperties, MultiBuffer};
+    diagnostic_block_renderer, diagnostic_style,
+    display_map::{BlockDisposition, BlockProperties},
+    Anchor, Editor, ExcerptProperties, MultiBuffer,
+};
 use gpui::{
     action, elements::*, keymap::Binding, AppContext, Entity, ModelHandle, MutableAppContext,
     RenderContext, View, ViewContext, ViewHandle,
@@ -68,13 +71,9 @@ impl workspace::Item for ProjectDiagnostics {
     ) -> Self::View {
         let project = handle.read(cx).project.clone();
         let excerpts = cx.add_model(|cx| MultiBuffer::new(project.read(cx).replica_id(cx)));
-        let editor = cx.add_view(|cx| {
+        let build_settings = editor::settings_builder(excerpts.downgrade(), settings.clone());
-            Editor::for_buffer(
+        let editor =
-                excerpts.clone(),
+            cx.add_view(|cx| Editor::for_buffer(excerpts.clone(), build_settings.clone(), cx));
-                editor::settings_builder(excerpts.downgrade(), settings.clone()),
-                cx,
-            )
-        });
 
         let project_paths = project
             .read(cx)
@@ -91,59 +90,86 @@ impl workspace::Item for ProjectDiagnostics {
                         .await?;
                     let snapshot = buffer.read_with(&cx, |b, _| b.snapshot());
 
-                    let mut grouped_diagnostics = HashMap::default();
-                    for entry in snapshot.all_diagnostics() {
-                        let mut group = grouped_diagnostics
-                            .entry(entry.diagnostic.group_id)
-                            .or_insert((Point::zero(), Vec::new()));
-                        if entry.diagnostic.is_primary {
-                            group.0 = entry.range.start;
-                        }
-                        group.1.push(entry);
-                    }
-                    let mut sorted_diagnostic_groups =
-                        grouped_diagnostics.into_values().collect::<Vec<_>>();
-                    sorted_diagnostic_groups.sort_by_key(|group| group.0);
-
-                    for entry in snapshot.all_diagnostics::<Point>() {
                     this.update(&mut cx, |this, cx| {
-                            this.excerpts.update(cx, |excerpts, cx| {
+                        let mut blocks = Vec::new();
-                                excerpts.push_excerpt(
+                        this.excerpts.update(cx, |excerpts, excerpts_cx| {
+                            for group in snapshot.diagnostic_groups::<Point>() {
+                                let excerpt_start = cmp::min(
+                                    group.primary.range.start.row,
+                                    group
+                                        .supporting
+                                        .first()
+                                        .map_or(u32::MAX, |entry| entry.range.start.row),
+                                );
+                                let excerpt_end = cmp::max(
+                                    group.primary.range.end.row,
+                                    group
+                                        .supporting
+                                        .last()
+                                        .map_or(0, |entry| entry.range.end.row),
+                                );
+
+                                let primary_diagnostic = group.primary.diagnostic;
+                                let excerpt_id = excerpts.push_excerpt(
                                     ExcerptProperties {
                                         buffer: &buffer,
-                                        range: entry.range,
+                                        range: Point::new(excerpt_start, 0)
-                                        header_height: entry
+                                            ..Point::new(
-                                            .diagnostic
+                                                excerpt_end,
+                                                snapshot.line_len(excerpt_end),
+                                            ),
+                                        header_height: primary_diagnostic
                                             .message
                                             .matches('\n')
                                             .count()
                                             as u8
                                             + 1,
                                         render_header: Some(Arc::new({
-                                            let message = entry.diagnostic.message.clone();
                                             let settings = settings.clone();
 
                                             move |_| {
                                                 let editor_style = &settings.borrow().theme.editor;
                                                 let mut text_style = editor_style.text.clone();
                                                 text_style.color = diagnostic_style(
-                                                    entry.diagnostic.severity,
+                                                    primary_diagnostic.severity,
                                                     true,
                                                     &editor_style,
                                                 )
                                                 .text;
 
-                                                Text::new(message.clone(), text_style).boxed()
+                                                Text::new(
+                                                    primary_diagnostic.message.clone(),
+                                                    text_style,
+                                                )
+                                                .boxed()
                                             }
                                         })),
                                     },
-                                    cx,
+                                    excerpts_cx,
                                 );
-                                cx.notify();
+
+                                for entry in group.supporting {
+                                    let buffer_anchor = snapshot.anchor_before(entry.range.start);
+                                    blocks.push(BlockProperties {
+                                        position: Anchor::new(excerpt_id.clone(), buffer_anchor),
+                                        height: entry.diagnostic.message.matches('\n').count()
+                                            as u8
+                                            + 1,
+                                        render: diagnostic_block_renderer(
+                                            entry.diagnostic,
+                                            true,
+                                            build_settings.clone(),
+                                        ),
+                                        disposition: BlockDisposition::Below,
+                                    });
+                                }
+                            }
+                        });
+                        this.editor.update(cx, |editor, cx| {
+                            editor.insert_blocks(blocks, cx);
                         });
                     })
                 }
-                }
                 Result::Ok::<_, anyhow::Error>(())
             }
         })

crates/editor/src/display_map.rs

@@ -8,7 +8,7 @@ use crate::{
 };
 use block_map::{BlockMap, BlockPoint};
 use fold_map::{FoldMap, ToFoldPoint as _};
-use gpui::{fonts::FontId, ElementBox, Entity, ModelContext, ModelHandle};
+use gpui::{fonts::FontId, Entity, ModelContext, ModelHandle};
 use language::{Point, Subscription as BufferSubscription};
 use std::{
     collections::{HashMap, HashSet},
@@ -21,7 +21,7 @@ use wrap_map::WrapMap;
 
 pub use block_map::{
     AlignedBlock, BlockBufferRows as DisplayBufferRows, BlockChunks as DisplayChunks, BlockContext,
-    BlockDisposition, BlockId, BlockProperties,
+    BlockDisposition, BlockId, BlockProperties, RenderBlock,
 };
 
 pub trait ToDisplayPoint {
@@ -146,10 +146,7 @@ impl DisplayMap {
         block_map.insert(blocks)
     }
 
-    pub fn replace_blocks<F>(&mut self, styles: HashMap<BlockId, F>)
+    pub fn replace_blocks(&mut self, styles: HashMap<BlockId, RenderBlock>) {
-    where
-        F: 'static + Fn(&BlockContext) -> ElementBox,
-    {
         self.block_map.replace(styles);
     }
 

crates/editor/src/display_map/block_map.rs

@@ -45,11 +45,13 @@ struct BlockRow(u32);
 #[derive(Copy, Clone, Debug, Default, Eq, Ord, PartialOrd, PartialEq)]
 struct WrapRow(u32);
 
+pub type RenderBlock = Arc<dyn Fn(&BlockContext) -> ElementBox>;
+
 pub struct Block {
     id: BlockId,
     position: Anchor,
     height: u8,
-    render: Mutex<Arc<dyn Fn(&BlockContext) -> ElementBox>>,
+    render: Mutex<RenderBlock>,
     disposition: BlockDisposition,
 }
 
@@ -306,13 +308,10 @@ impl BlockMap {
         *transforms = new_transforms;
     }
 
-    pub fn replace<F>(&mut self, mut element_builders: HashMap<BlockId, F>)
+    pub fn replace(&mut self, mut renderers: HashMap<BlockId, RenderBlock>) {
-    where
-        F: 'static + Fn(&BlockContext) -> ElementBox,
-    {
         for block in &self.blocks {
-            if let Some(build_element) = element_builders.remove(&block.id) {
+            if let Some(render) = renderers.remove(&block.id) {
-                *block.render.lock() = Arc::new(build_element);
+                *block.render.lock() = render;
             }
         }
     }
@@ -832,6 +831,14 @@ impl Deref for AlignedBlock {
     }
 }
 
+impl<'a> Deref for BlockContext<'a> {
+    type Target = AppContext;
+
+    fn deref(&self) -> &Self::Target {
+        &self.cx
+    }
+}
+
 impl Debug for Block {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("Block")

crates/editor/src/editor.rs

@@ -27,16 +27,13 @@ use language::{
     BracketPair, Buffer, Diagnostic, DiagnosticSeverity, Language, Point, Selection, SelectionGoal,
     TransactionId,
 };
-use multi_buffer::{
+pub use multi_buffer::{Anchor, ExcerptProperties, MultiBuffer};
-    Anchor, AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, ToOffset, ToPoint,
+use multi_buffer::{AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, ToOffset, ToPoint};
-};
-pub use multi_buffer::{ExcerptProperties, MultiBuffer};
 use postage::watch;
 use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
 use smol::Timer;
 use std::{
-    cell::RefCell,
     cmp,
     collections::HashMap,
     iter::{self, FromIterator},
@@ -359,6 +356,8 @@ pub enum SoftWrap {
     Column(u32),
 }
 
+type BuildSettings = Rc<dyn Fn(&AppContext) -> EditorSettings>;
+
 pub struct Editor {
     handle: WeakViewHandle<Self>,
     buffer: ModelHandle<MultiBuffer>,
@@ -377,7 +376,7 @@ pub struct Editor {
     scroll_position: Vector2F,
     scroll_top_anchor: Anchor,
     autoscroll_request: Option<Autoscroll>,
-    build_settings: Rc<RefCell<dyn Fn(&AppContext) -> EditorSettings>>,
+    build_settings: BuildSettings,
     focused: bool,
     show_local_cursors: bool,
     blink_epoch: usize,
@@ -433,10 +432,7 @@ struct ClipboardSelection {
 }
 
 impl Editor {
-    pub fn single_line(
+    pub fn single_line(build_settings: BuildSettings, cx: &mut ViewContext<Self>) -> Self {
-        build_settings: impl 'static + Fn(&AppContext) -> EditorSettings,
-        cx: &mut ViewContext<Self>,
-    ) -> Self {
         let buffer = cx.add_model(|cx| Buffer::new(0, String::new(), cx));
         let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx));
         let mut view = Self::for_buffer(buffer, build_settings, cx);
@@ -446,7 +442,7 @@ impl Editor {
 
     pub fn auto_height(
         max_lines: usize,
-        build_settings: impl 'static + Fn(&AppContext) -> EditorSettings,
+        build_settings: BuildSettings,
         cx: &mut ViewContext<Self>,
     ) -> Self {
         let buffer = cx.add_model(|cx| Buffer::new(0, String::new(), cx));
@@ -458,10 +454,10 @@ impl Editor {
 
     pub fn for_buffer(
         buffer: ModelHandle<MultiBuffer>,
-        build_settings: impl 'static + Fn(&AppContext) -> EditorSettings,
+        build_settings: BuildSettings,
         cx: &mut ViewContext<Self>,
     ) -> Self {
-        Self::new(buffer, Rc::new(RefCell::new(build_settings)), cx)
+        Self::new(buffer, build_settings, cx)
     }
 
     pub fn clone(&self, cx: &mut ViewContext<Self>) -> Self {
@@ -473,10 +469,10 @@ impl Editor {
 
     pub fn new(
         buffer: ModelHandle<MultiBuffer>,
-        build_settings: Rc<RefCell<dyn Fn(&AppContext) -> EditorSettings>>,
+        build_settings: BuildSettings,
         cx: &mut ViewContext<Self>,
     ) -> Self {
-        let settings = build_settings.borrow_mut()(cx);
+        let settings = build_settings(cx);
         let display_map = cx.add_model(|cx| {
             DisplayMap::new(
                 buffer.clone(),
@@ -1440,7 +1436,7 @@ impl Editor {
 
     pub fn tab(&mut self, _: &Tab, cx: &mut ViewContext<Self>) {
         self.start_transaction(cx);
-        let tab_size = self.build_settings.borrow()(cx).tab_size;
+        let tab_size = (self.build_settings)(cx).tab_size;
         let mut selections = self.local_selections::<Point>(cx);
         let mut last_indent = None;
         self.buffer.update(cx, |buffer, cx| {
@@ -1512,7 +1508,7 @@ impl Editor {
 
     pub fn outdent(&mut self, _: &Outdent, cx: &mut ViewContext<Self>) {
         self.start_transaction(cx);
-        let tab_size = self.build_settings.borrow()(cx).tab_size;
+        let tab_size = (self.build_settings)(cx).tab_size;
         let selections = self.local_selections::<Point>(cx);
         let mut deletion_ranges = Vec::new();
         let mut last_outdent = None;
@@ -2900,13 +2896,14 @@ impl Editor {
                 active_diagnostics.is_valid = is_valid;
                 let mut new_styles = HashMap::new();
                 for (block_id, diagnostic) in &active_diagnostics.blocks {
-                    let build_settings = self.build_settings.clone();
+                    new_styles.insert(
-                    let diagnostic = diagnostic.clone();
+                        *block_id,
-                    new_styles.insert(*block_id, move |cx: &BlockContext| {
+                        diagnostic_block_renderer(
-                        let diagnostic = diagnostic.clone();
+                            diagnostic.clone(),
-                        let settings = build_settings.borrow()(cx.cx);
+                            is_valid,
-                        render_diagnostic(diagnostic, &settings.style, is_valid, cx.anchor_x)
+                            self.build_settings.clone(),
-                    });
+                        ),
+                    );
                 }
                 self.display_map
                     .update(cx, |display_map, _| display_map.replace_blocks(new_styles));
@@ -2950,11 +2947,7 @@ impl Editor {
                         BlockProperties {
                             position: entry.range.start,
                             height: message_height,
-                            render: Arc::new(move |cx| {
+                            render: diagnostic_block_renderer(diagnostic, true, build_settings),
-                                let settings = build_settings.borrow()(cx.cx);
-                                let diagnostic = diagnostic.clone();
-                                render_diagnostic(diagnostic, &settings.style, true, cx.anchor_x)
-                            }),
                             disposition: BlockDisposition::Below,
                         }
                     }),
@@ -3431,6 +3424,18 @@ impl Editor {
         }
     }
 
+    pub fn insert_blocks<P>(
+        &mut self,
+        blocks: impl IntoIterator<Item = BlockProperties<P>>,
+        cx: &mut ViewContext<Self>,
+    ) -> Vec<BlockId>
+    where
+        P: ToOffset + Clone,
+    {
+        self.display_map
+            .update(cx, |display_map, cx| display_map.insert_blocks(blocks, cx))
+    }
+
     pub fn longest_row(&self, cx: &mut MutableAppContext) -> u32 {
         self.display_map
             .update(cx, |map, cx| map.snapshot(cx))
@@ -3645,7 +3650,7 @@ impl Entity for Editor {
 
 impl View for Editor {
     fn render(&mut self, cx: &mut RenderContext<Self>) -> ElementBox {
-        let settings = self.build_settings.borrow_mut()(cx);
+        let settings = (self.build_settings)(cx);
         self.display_map.update(cx, |map, cx| {
             map.set_font(
                 settings.style.text.font_id,
@@ -3757,6 +3762,18 @@ impl<T: ToPoint + ToOffset> SelectionExt for Selection<T> {
     }
 }
 
+pub fn diagnostic_block_renderer(
+    diagnostic: Diagnostic,
+    is_valid: bool,
+    build_settings: BuildSettings,
+) -> RenderBlock {
+    Arc::new(move |cx: &BlockContext| {
+        let diagnostic = diagnostic.clone();
+        let settings = build_settings(cx);
+        render_diagnostic(diagnostic, &settings.style, is_valid, cx.anchor_x)
+    })
+}
+
 fn render_diagnostic(
     diagnostic: Diagnostic,
     style: &EditorStyle,
@@ -3792,8 +3809,8 @@ pub fn diagnostic_style(
 pub fn settings_builder(
     buffer: WeakModelHandle<MultiBuffer>,
     settings: watch::Receiver<workspace::Settings>,
-) -> impl Fn(&AppContext) -> EditorSettings {
+) -> BuildSettings {
-    move |cx| {
+    Rc::new(move |cx| {
         let settings = settings.borrow();
         let font_cache = cx.font_cache();
         let font_family_id = settings.buffer_font_family;
@@ -3828,7 +3845,7 @@ pub fn settings_builder(
             soft_wrap,
             style: theme,
         }
-    }
+    })
 }
 
 #[cfg(test)]
@@ -6086,7 +6103,7 @@ mod tests {
         settings: EditorSettings,
         cx: &mut ViewContext<Editor>,
     ) -> Editor {
-        Editor::for_buffer(buffer, move |_| settings.clone(), cx)
+        Editor::for_buffer(buffer, Rc::new(move |_| settings.clone()), cx)
     }
 }
 

crates/editor/src/element.rs

@@ -1179,6 +1179,7 @@ fn scale_horizontal_mouse_autoscroll_delta(delta: f32) -> f32 {
 mod tests {
     use super::*;
     use crate::{Editor, EditorSettings, MultiBuffer};
+    use std::rc::Rc;
     use util::test::sample_text;
 
     #[gpui::test]
@@ -1190,7 +1191,7 @@ mod tests {
                 buffer,
                 {
                     let settings = settings.clone();
-                    move |_| settings.clone()
+                    Rc::new(move |_| settings.clone())
                 },
                 cx,
             )

crates/editor/src/multi_buffer.rs

@@ -559,6 +559,8 @@ impl MultiBuffer {
 
         self.subscriptions.publish_mut([edit]);
 
+        cx.notify();
+
         id
     }
 

crates/editor/src/multi_buffer/anchor.rs

@@ -14,6 +14,13 @@ pub struct Anchor {
 }
 
 impl Anchor {
+    pub fn new(excerpt_id: ExcerptId, text_anchor: text::Anchor) -> Self {
+        Self {
+            excerpt_id,
+            text_anchor,
+        }
+    }
+
     pub fn min() -> Self {
         Self {
             excerpt_id: ExcerptId::min(),

crates/file_finder/src/file_finder.rs

@@ -16,6 +16,7 @@ use project::{Project, ProjectPath};
 use std::{
     cmp,
     path::Path,
+    rc::Rc,
     sync::{
         atomic::{self, AtomicBool},
         Arc,
@@ -270,14 +271,14 @@ impl FileFinder {
             Editor::single_line(
                 {
                     let settings = settings.clone();
-                    move |_| {
+                    Rc::new(move |_| {
                         let settings = settings.borrow();
                         EditorSettings {
                             style: settings.theme.selector.input_editor.as_editor(),
                             tab_size: settings.tab_size,
                             soft_wrap: editor::SoftWrap::None,
                         }
-                    }
+                    })
                 },
                 cx,
             )

crates/go_to_line/src/go_to_line.rs

@@ -1,3 +1,5 @@
+use std::rc::Rc;
+
 use editor::{display_map::ToDisplayPoint, Autoscroll, Editor, EditorSettings};
 use gpui::{
     action, elements::*, geometry::vector::Vector2F, keymap::Binding, Axis, Entity,
@@ -49,14 +51,14 @@ impl GoToLine {
             Editor::single_line(
                 {
                     let settings = settings.clone();
-                    move |_| {
+                    Rc::new(move |_| {
                         let settings = settings.borrow();
                         EditorSettings {
                             tab_size: settings.tab_size,
                             style: settings.theme.selector.input_editor.as_editor(),
                             soft_wrap: editor::SoftWrap::None,
                         }
-                    }
+                    })
                 },
                 cx,
             )

crates/language/src/buffer.rs

@@ -1,4 +1,4 @@
-use crate::diagnostic_set::DiagnosticEntry;
+use crate::diagnostic_set::{DiagnosticEntry, DiagnosticGroup};
 pub use crate::{
     diagnostic_set::DiagnosticSet,
     highlight_map::{HighlightId, HighlightMap},
@@ -1728,6 +1728,13 @@ impl BufferSnapshot {
         self.diagnostics.range(search_range, self, true)
     }
 
+    pub fn diagnostic_groups<O>(&self) -> Vec<DiagnosticGroup<O>>
+    where
+        O: FromAnchor + Ord + Copy,
+    {
+        self.diagnostics.groups(self)
+    }
+
     pub fn diagnostic_group<'a, O>(
         &'a self,
         group_id: usize,

crates/language/src/diagnostic_set.rs

@@ -1,4 +1,5 @@
 use crate::Diagnostic;
+use collections::HashMap;
 use std::{
     cmp::{Ordering, Reverse},
     iter,
@@ -18,6 +19,11 @@ pub struct DiagnosticEntry<T> {
     pub diagnostic: Diagnostic,
 }
 
+pub struct DiagnosticGroup<T> {
+    pub primary: DiagnosticEntry<T>,
+    pub supporting: Vec<DiagnosticEntry<T>>,
+}
+
 #[derive(Clone, Debug)]
 pub struct Summary {
     start: Anchor,
@@ -98,6 +104,40 @@ impl DiagnosticSet {
         })
     }
 
+    pub fn groups<O>(&self, buffer: &text::BufferSnapshot) -> Vec<DiagnosticGroup<O>>
+    where
+        O: FromAnchor + Ord + Copy,
+    {
+        let mut groups =
+            HashMap::<usize, (Option<DiagnosticEntry<O>>, Vec<DiagnosticEntry<O>>)>::default();
+
+        for entry in self.diagnostics.iter() {
+            let entry = entry.resolve(buffer);
+            let (ref mut primary, ref mut supporting) = groups
+                .entry(entry.diagnostic.group_id)
+                .or_insert((None, Vec::new()));
+            if entry.diagnostic.is_primary {
+                *primary = Some(entry);
+            } else {
+                supporting.push(entry);
+            }
+        }
+
+        let mut groups = groups
+            .into_values()
+            .map(|(primary, mut supporting)| {
+                supporting.sort_unstable_by_key(|entry| entry.range.start);
+                DiagnosticGroup {
+                    primary: primary.unwrap(),
+                    supporting,
+                }
+            })
+            .collect::<Vec<_>>();
+        groups.sort_unstable_by_key(|group| group.primary.range.start);
+
+        groups
+    }
+
     pub fn group<'a, O: FromAnchor>(
         &'a self,
         group_id: usize,

crates/theme_selector/src/theme_selector.rs

@@ -9,7 +9,7 @@ use gpui::{
 };
 use parking_lot::Mutex;
 use postage::watch;
-use std::{cmp, sync::Arc};
+use std::{cmp, rc::Rc, sync::Arc};
 use theme::ThemeRegistry;
 use workspace::{Settings, Workspace};
 
@@ -64,14 +64,14 @@ impl ThemeSelector {
             Editor::single_line(
                 {
                     let settings = settings.clone();
-                    move |_| {
+                    Rc::new(move |_| {
                         let settings = settings.borrow();
                         EditorSettings {
                             tab_size: settings.tab_size,
                             style: settings.theme.selector.input_editor.as_editor(),
                             soft_wrap: editor::SoftWrap::None,
                         }
-                    }
+                    })
                 },
                 cx,
             )