Merge pull request #337 from zed-industries/project-diagnostics-styling

Max Brunsfeld created

Restructure the project diagnostics view to match some aspects of current designs

Change summary

crates/diagnostics/src/diagnostics.rs      | 416 ++++++++++++++++++-----
crates/editor/src/display_map/block_map.rs |  49 ++
crates/editor/src/editor.rs                |  42 --
crates/editor/src/element.rs               |  20 
crates/gpui/src/app.rs                     |  22 +
crates/theme/src/theme.rs                  |   2 
crates/workspace/src/workspace.rs          |   6 
crates/zed/assets/themes/_base.toml        |  15 
8 files changed, 403 insertions(+), 169 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -1,10 +1,10 @@
 pub mod items;
 
 use anyhow::Result;
-use collections::{HashMap, HashSet, BTreeSet};
+use collections::{BTreeSet, HashMap, HashSet};
 use editor::{
-    context_header_renderer, diagnostic_block_renderer, diagnostic_header_renderer,
-    display_map::{BlockDisposition, BlockId, BlockProperties},
+    diagnostic_block_renderer, diagnostic_style,
+    display_map::{BlockDisposition, BlockId, BlockProperties, RenderBlock},
     items::BufferItemHandle,
     Autoscroll, BuildSettings, Editor, ExcerptId, ExcerptProperties, MultiBuffer, ToOffset,
 };
@@ -12,10 +12,10 @@ use gpui::{
     action, elements::*, keymap::Binding, AppContext, Entity, ModelHandle, MutableAppContext,
     RenderContext, Task, View, ViewContext, ViewHandle, WeakViewHandle,
 };
-use language::{Bias, Buffer, DiagnosticEntry, Point, Selection, SelectionGoal};
+use language::{Bias, Buffer, Diagnostic, DiagnosticEntry, Point, Selection, SelectionGoal};
 use postage::watch;
 use project::{Project, ProjectPath, WorktreeId};
-use std::{cmp::Ordering, mem, ops::Range};
+use std::{cmp::Ordering, mem, ops::Range, sync::Arc};
 use util::TryFutureExt;
 use workspace::Workspace;
 
@@ -48,12 +48,18 @@ struct ProjectDiagnosticsEditor {
     workspace: WeakViewHandle<Workspace>,
     editor: ViewHandle<Editor>,
     excerpts: ModelHandle<MultiBuffer>,
-    path_states: Vec<(ProjectPath, Vec<DiagnosticGroupState>)>,
+    path_states: Vec<PathState>,
     paths_to_update: HashMap<WorktreeId, BTreeSet<ProjectPath>>,
     build_settings: BuildSettings,
     settings: watch::Receiver<workspace::Settings>,
 }
 
+struct PathState {
+    path: ProjectPath,
+    header: Option<BlockId>,
+    diagnostic_groups: Vec<DiagnosticGroupState>,
+}
+
 struct DiagnosticGroupState {
     primary_diagnostic: DiagnosticEntry<language::Anchor>,
     primary_excerpt_ix: usize,
@@ -154,11 +160,6 @@ impl ProjectDiagnosticsEditor {
         this
     }
 
-    #[cfg(test)]
-    fn text(&self, cx: &AppContext) -> String {
-        self.editor.read(cx).text(cx)
-    }
-
     fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext<Workspace>) {
         if let Some(existing) = workspace.item_of_type::<ProjectDiagnostics>(cx) {
             workspace.activate_item(&existing, cx);
@@ -208,11 +209,7 @@ impl ProjectDiagnosticsEditor {
         }
     }
 
-    fn update_excerpts(
-        &self,
-        paths: BTreeSet<ProjectPath>,
-        cx: &mut ViewContext<Self>,
-    ) {
+    fn update_excerpts(&self, paths: BTreeSet<ProjectPath>, cx: &mut ViewContext<Self>) {
         let project = self.model.read(cx).project.clone();
         cx.spawn(|this, mut cx| {
             async move {
@@ -220,9 +217,7 @@ impl ProjectDiagnosticsEditor {
                     let buffer = project
                         .update(&mut cx, |project, cx| project.open_buffer(path.clone(), cx))
                         .await?;
-                    this.update(&mut cx, |view, cx| {
-                        view.populate_excerpts(path, buffer, cx)
-                    })
+                    this.update(&mut cx, |view, cx| view.populate_excerpts(path, buffer, cx))
                 }
                 Result::<_, anyhow::Error>::Ok(())
             }
@@ -239,32 +234,39 @@ impl ProjectDiagnosticsEditor {
     ) {
         let was_empty = self.path_states.is_empty();
         let snapshot = buffer.read(cx).snapshot();
-        let path_ix = match self
-            .path_states
-            .binary_search_by_key(&&path, |e| &e.0)
-        {
+        let path_ix = match self.path_states.binary_search_by_key(&&path, |e| &e.path) {
             Ok(ix) => ix,
             Err(ix) => {
-                self.path_states
-                    .insert(ix, (path.clone(), Default::default()));
+                self.path_states.insert(
+                    ix,
+                    PathState {
+                        path: path.clone(),
+                        header: None,
+                        diagnostic_groups: Default::default(),
+                    },
+                );
                 ix
             }
         };
 
         let mut prev_excerpt_id = if path_ix > 0 {
-            let prev_path_last_group = &self.path_states[path_ix - 1].1.last().unwrap();
+            let prev_path_last_group = &self.path_states[path_ix - 1]
+                .diagnostic_groups
+                .last()
+                .unwrap();
             prev_path_last_group.excerpts.last().unwrap().clone()
         } else {
             ExcerptId::min()
         };
 
-        let groups = &mut self.path_states[path_ix].1;
+        let path_state = &mut self.path_states[path_ix];
         let mut groups_to_add = Vec::new();
         let mut group_ixs_to_remove = Vec::new();
         let mut blocks_to_add = Vec::new();
         let mut blocks_to_remove = HashSet::default();
+        let mut first_excerpt_id = None;
         let excerpts_snapshot = self.excerpts.update(cx, |excerpts, excerpts_cx| {
-            let mut old_groups = groups.iter().enumerate().peekable();
+            let mut old_groups = path_state.diagnostic_groups.iter().enumerate().peekable();
             let mut new_groups = snapshot
                 .diagnostic_groups()
                 .into_iter()
@@ -273,17 +275,17 @@ impl ProjectDiagnosticsEditor {
 
             loop {
                 let mut to_insert = None;
-                let mut to_invalidate = None;
+                let mut to_remove = None;
                 let mut to_keep = None;
                 match (old_groups.peek(), new_groups.peek()) {
                     (None, None) => break,
                     (None, Some(_)) => to_insert = new_groups.next(),
-                    (Some(_), None) => to_invalidate = old_groups.next(),
+                    (Some(_), None) => to_remove = old_groups.next(),
                     (Some((_, old_group)), Some(new_group)) => {
                         let old_primary = &old_group.primary_diagnostic;
                         let new_primary = &new_group.entries[new_group.primary_ix];
                         match compare_diagnostics(old_primary, new_primary, &snapshot) {
-                            Ordering::Less => to_invalidate = old_groups.next(),
+                            Ordering::Less => to_remove = old_groups.next(),
                             Ordering::Equal => {
                                 to_keep = old_groups.next();
                                 new_groups.next();
@@ -331,6 +333,7 @@ impl ProjectDiagnosticsEditor {
                             );
 
                             prev_excerpt_id = excerpt_id.clone();
+                            first_excerpt_id.get_or_insert_with(|| prev_excerpt_id.clone());
                             group_state.excerpts.push(excerpt_id.clone());
                             let header_position = (excerpt_id.clone(), language::Anchor::min());
 
@@ -343,9 +346,8 @@ impl ProjectDiagnosticsEditor {
                                 group_state.block_count += 1;
                                 blocks_to_add.push(BlockProperties {
                                     position: header_position,
-                                    height: 3,
+                                    height: 2,
                                     render: diagnostic_header_renderer(
-                                        buffer.clone(),
                                         header,
                                         true,
                                         self.build_settings.clone(),
@@ -394,12 +396,13 @@ impl ProjectDiagnosticsEditor {
                     }
 
                     groups_to_add.push(group_state);
-                } else if let Some((group_ix, group_state)) = to_invalidate {
+                } else if let Some((group_ix, group_state)) = to_remove {
                     excerpts.remove_excerpts(group_state.excerpts.iter(), excerpts_cx);
                     group_ixs_to_remove.push(group_ix);
                     blocks_to_remove.extend(group_state.blocks.iter().copied());
                 } else if let Some((_, group)) = to_keep {
                     prev_excerpt_id = group.excerpts.last().unwrap().clone();
+                    first_excerpt_id.get_or_insert_with(|| prev_excerpt_id.clone());
                 }
             }
 
@@ -407,10 +410,18 @@ impl ProjectDiagnosticsEditor {
         });
 
         self.editor.update(cx, |editor, cx| {
+            blocks_to_remove.extend(path_state.header);
             editor.remove_blocks(blocks_to_remove, cx);
-            let mut block_ids = editor
-                .insert_blocks(
-                    blocks_to_add.into_iter().map(|block| {
+            let header_block = first_excerpt_id.map(|excerpt_id| BlockProperties {
+                position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, language::Anchor::min()),
+                height: 2,
+                render: path_header_renderer(buffer, self.build_settings.clone()),
+                disposition: BlockDisposition::Above,
+            });
+            let block_ids = editor.insert_blocks(
+                blocks_to_add
+                    .into_iter()
+                    .map(|block| {
                         let (excerpt_id, text_anchor) = block.position;
                         BlockProperties {
                             position: excerpts_snapshot.anchor_in_excerpt(excerpt_id, text_anchor),
@@ -418,21 +429,23 @@ impl ProjectDiagnosticsEditor {
                             render: block.render,
                             disposition: block.disposition,
                         }
-                    }),
-                    cx,
-                )
-                .into_iter();
+                    })
+                    .chain(header_block.into_iter()),
+                cx,
+            );
 
+            let mut block_ids = block_ids.into_iter();
             for group_state in &mut groups_to_add {
                 group_state.blocks = block_ids.by_ref().take(group_state.block_count).collect();
             }
+            path_state.header = block_ids.next();
         });
 
         for ix in group_ixs_to_remove.into_iter().rev() {
-            groups.remove(ix);
+            path_state.diagnostic_groups.remove(ix);
         }
-        groups.extend(groups_to_add);
-        groups.sort_unstable_by(|a, b| {
+        path_state.diagnostic_groups.extend(groups_to_add);
+        path_state.diagnostic_groups.sort_unstable_by(|a, b| {
             let range_a = &a.primary_diagnostic.range;
             let range_b = &b.primary_diagnostic.range;
             range_a
@@ -442,7 +455,7 @@ impl ProjectDiagnosticsEditor {
                 .then_with(|| range_a.end.cmp(&range_b.end, &snapshot).unwrap())
         });
 
-        if groups.is_empty() {
+        if path_state.diagnostic_groups.is_empty() {
             self.path_states.remove(path_ix);
         }
 
@@ -451,7 +464,7 @@ impl ProjectDiagnosticsEditor {
             let mut selections;
             let new_excerpt_ids_by_selection_id;
             if was_empty {
-                groups = self.path_states.first()?.1.as_slice();
+                groups = self.path_states.first()?.diagnostic_groups.as_slice();
                 new_excerpt_ids_by_selection_id = [(0, ExcerptId::min())].into_iter().collect();
                 selections = vec![Selection {
                     id: 0,
@@ -461,7 +474,7 @@ impl ProjectDiagnosticsEditor {
                     goal: SelectionGoal::None,
                 }];
             } else {
-                groups = self.path_states.get(path_ix)?.1.as_slice();
+                groups = self.path_states.get(path_ix)?.diagnostic_groups.as_slice();
                 new_excerpt_ids_by_selection_id = editor.refresh_selections(cx);
                 selections = editor.local_selections::<usize>(cx);
             }
@@ -575,6 +588,61 @@ impl workspace::ItemView for ProjectDiagnosticsEditor {
     }
 }
 
+fn path_header_renderer(buffer: ModelHandle<Buffer>, build_settings: BuildSettings) -> RenderBlock {
+    Arc::new(move |cx| {
+        let settings = build_settings(cx);
+        let file_path = if let Some(file) = buffer.read(&**cx).file() {
+            file.path().to_string_lossy().to_string()
+        } else {
+            "untitled".to_string()
+        };
+        let mut text_style = settings.style.text.clone();
+        let style = settings.style.diagnostic_path_header;
+        text_style.color = style.text;
+        Label::new(file_path, text_style)
+            .aligned()
+            .left()
+            .contained()
+            .with_style(style.header)
+            .with_padding_left(cx.line_number_x)
+            .expanded()
+            .named("path header block")
+    })
+}
+
+fn diagnostic_header_renderer(
+    diagnostic: Diagnostic,
+    is_valid: bool,
+    build_settings: BuildSettings,
+) -> RenderBlock {
+    Arc::new(move |cx| {
+        let settings = build_settings(cx);
+        let mut text_style = settings.style.text.clone();
+        let diagnostic_style = diagnostic_style(diagnostic.severity, is_valid, &settings.style);
+        text_style.color = diagnostic_style.text;
+        Text::new(diagnostic.message.clone(), text_style)
+            .with_soft_wrap(false)
+            .aligned()
+            .left()
+            .contained()
+            .with_style(diagnostic_style.header)
+            .with_padding_left(cx.line_number_x)
+            .expanded()
+            .named("diagnostic header")
+    })
+}
+
+fn context_header_renderer(build_settings: BuildSettings) -> RenderBlock {
+    Arc::new(move |cx| {
+        let settings = build_settings(cx);
+        let text_style = settings.style.text.clone();
+        Label::new("…".to_string(), text_style)
+            .contained()
+            .with_padding_left(cx.line_number_x)
+            .named("collapsed context")
+    })
+}
+
 fn compare_diagnostics<L: language::ToOffset, R: language::ToOffset>(
     lhs: &DiagnosticEntry<L>,
     rhs: &DiagnosticEntry<R>,
@@ -596,11 +664,10 @@ fn compare_diagnostics<L: language::ToOffset, R: language::ToOffset>(
 #[cfg(test)]
 mod tests {
     use super::*;
-    use client::{http::ServerResponse, test::FakeHttpClient, Client, UserStore};
-    use editor::DisplayPoint;
+    use editor::{display_map::BlockContext, DisplayPoint, EditorSnapshot};
     use gpui::TestAppContext;
-    use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, LanguageRegistry, PointUtf16};
-    use project::{worktree, FakeFs};
+    use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, PointUtf16};
+    use project::worktree;
     use serde_json::json;
     use std::sync::Arc;
     use unindent::Unindent as _;
@@ -608,31 +675,23 @@ mod tests {
 
     #[gpui::test]
     async fn test_diagnostics(mut cx: TestAppContext) {
-        let workspace_params = cx.update(WorkspaceParams::test);
-        let settings = workspace_params.settings.clone();
-        let http_client = FakeHttpClient::new(|_| async move { Ok(ServerResponse::new(404)) });
-        let client = Client::new(http_client.clone());
-        let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx));
-        let fs = Arc::new(FakeFs::new());
-
-        let project = cx.update(|cx| {
-            Project::local(
-                client.clone(),
-                user_store,
-                Arc::new(LanguageRegistry::new()),
-                fs.clone(),
-                cx,
-            )
-        });
-
-        fs.insert_tree(
-            "/test",
-            json!({
-                "a.rs": "
+        let params = cx.update(WorkspaceParams::test);
+        let project = params.project.clone();
+        let workspace = cx.add_view(0, |cx| Workspace::new(&params, cx));
+
+        params
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/test",
+                json!({
+                    "consts.rs": "
                     const a: i32 = 'a';
-                ".unindent(),
+                    const b: i32 = c;
+                "
+                    .unindent(),
 
-                "main.rs": "
+                    "main.rs": "
                     fn main() {
                         let x = vec![];
                         let y = vec![];
@@ -644,10 +703,10 @@ mod tests {
                         d(x);
                     }
                 "
-                .unindent(),
-            }),
-        )
-        .await;
+                    .unindent(),
+                }),
+            )
+            .await;
 
         let worktree = project
             .update(&mut cx, |project, cx| {
@@ -656,6 +715,7 @@ mod tests {
             .await
             .unwrap();
 
+        // Create some diagnostics
         worktree.update(&mut cx, |worktree, cx| {
             worktree
                 .update_diagnostic_entries(
@@ -738,28 +798,36 @@ mod tests {
                 .unwrap();
         });
 
+        // Open the project diagnostics view while there are already diagnostics.
         let model = cx.add_model(|_| ProjectDiagnostics::new(project.clone()));
-        let workspace = cx.add_view(0, |cx| Workspace::new(&workspace_params, cx));
-
         let view = cx.add_view(0, |cx| {
-            ProjectDiagnosticsEditor::new(model, workspace.downgrade(), settings, cx)
+            ProjectDiagnosticsEditor::new(model, workspace.downgrade(), params.settings, cx)
         });
 
-        view.condition(&mut cx, |view, cx| view.text(cx).contains("fn main()"))
-            .await;
-
+        view.next_notification(&cx).await;
         view.update(&mut cx, |view, cx| {
             let editor = view.editor.update(cx, |editor, cx| editor.snapshot(cx));
 
+            assert_eq!(
+                editor_blocks(&editor, cx),
+                [
+                    (0, "path header block".into()),
+                    (2, "diagnostic header".into()),
+                    (15, "diagnostic header".into()),
+                    (24, "collapsed context".into()),
+                ]
+            );
             assert_eq!(
                 editor.text(),
                 concat!(
                     //
-                    // main.rs, diagnostic group 1
+                    // main.rs
                     //
+                    "\n", // filename
                     "\n", // padding
+                    // diagnostic group 1
                     "\n", // primary message
-                    "\n", // filename
+                    "\n", // padding
                     "    let x = vec![];\n",
                     "    let y = vec![];\n",
                     "\n", // supporting diagnostic
@@ -771,12 +839,9 @@ mod tests {
                     "    c(y);\n",
                     "\n", // supporting diagnostic
                     "    d(x);\n",
-                    //
-                    // main.rs, diagnostic group 2
-                    //
-                    "\n", // padding
+                    // diagnostic group 2
                     "\n", // primary message
-                    "\n", // filename
+                    "\n", // padding
                     "fn main() {\n",
                     "    let x = vec![];\n",
                     "\n", // supporting diagnostic
@@ -792,18 +857,20 @@ mod tests {
                 )
             );
 
+            // Cursor is at the first diagnostic
             view.editor.update(cx, |editor, cx| {
                 assert_eq!(
                     editor.selected_display_ranges(cx),
-                    [DisplayPoint::new(11, 6)..DisplayPoint::new(11, 6)]
+                    [DisplayPoint::new(12, 6)..DisplayPoint::new(12, 6)]
                 );
             });
         });
 
+        // Diagnostics are added for another earlier path.
         worktree.update(&mut cx, |worktree, cx| {
             worktree
                 .update_diagnostic_entries(
-                    Arc::from("/test/a.rs".as_ref()),
+                    Arc::from("/test/consts.rs".as_ref()),
                     None,
                     vec![DiagnosticEntry {
                         range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15),
@@ -822,30 +889,43 @@ mod tests {
             cx.emit(worktree::Event::DiskBasedDiagnosticsUpdated);
         });
 
-        view.condition(&mut cx, |view, cx| view.text(cx).contains("const a"))
-            .await;
-
+        view.next_notification(&cx).await;
         view.update(&mut cx, |view, cx| {
             let editor = view.editor.update(cx, |editor, cx| editor.snapshot(cx));
 
+            assert_eq!(
+                editor_blocks(&editor, cx),
+                [
+                    (0, "path header block".into()),
+                    (2, "diagnostic header".into()),
+                    (7, "path header block".into()),
+                    (9, "diagnostic header".into()),
+                    (22, "diagnostic header".into()),
+                    (31, "collapsed context".into()),
+                ]
+            );
             assert_eq!(
                 editor.text(),
                 concat!(
                     //
-                    // a.rs
+                    // consts.rs
                     //
+                    "\n", // filename
                     "\n", // padding
+                    // diagnostic group 1
                     "\n", // primary message
-                    "\n", // filename
+                    "\n", // padding
                     "const a: i32 = 'a';\n",
                     "\n", // supporting diagnostic
-                    "\n", // context line
+                    "const b: i32 = c;\n",
                     //
-                    // main.rs, diagnostic group 1
+                    // main.rs
                     //
+                    "\n", // filename
                     "\n", // padding
+                    // diagnostic group 1
                     "\n", // primary message
-                    "\n", // filename
+                    "\n", // padding
                     "    let x = vec![];\n",
                     "    let y = vec![];\n",
                     "\n", // supporting diagnostic
@@ -857,10 +937,126 @@ mod tests {
                     "    c(y);\n",
                     "\n", // supporting diagnostic
                     "    d(x);\n",
+                    // diagnostic group 2
+                    "\n", // primary message
+                    "\n", // filename
+                    "fn main() {\n",
+                    "    let x = vec![];\n",
+                    "\n", // supporting diagnostic
+                    "    let y = vec![];\n",
+                    "    a(x);\n",
+                    "\n", // supporting diagnostic
+                    "    b(y);\n",
+                    "\n", // context ellipsis
+                    "    c(y);\n",
+                    "    d(x);\n",
+                    "\n", // supporting diagnostic
+                    "}"
+                )
+            );
+
+            // Cursor keeps its position.
+            view.editor.update(cx, |editor, cx| {
+                assert_eq!(
+                    editor.selected_display_ranges(cx),
+                    [DisplayPoint::new(19, 6)..DisplayPoint::new(19, 6)]
+                );
+            });
+        });
+
+        // Diagnostics are added to the first path
+        worktree.update(&mut cx, |worktree, cx| {
+            worktree
+                .update_diagnostic_entries(
+                    Arc::from("/test/consts.rs".as_ref()),
+                    None,
+                    vec![
+                        DiagnosticEntry {
+                            range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15),
+                            diagnostic: Diagnostic {
+                                message: "mismatched types\nexpected `usize`, found `char`"
+                                    .to_string(),
+                                severity: DiagnosticSeverity::ERROR,
+                                is_primary: true,
+                                is_disk_based: true,
+                                group_id: 0,
+                                ..Default::default()
+                            },
+                        },
+                        DiagnosticEntry {
+                            range: PointUtf16::new(1, 15)..PointUtf16::new(1, 15),
+                            diagnostic: Diagnostic {
+                                message: "unresolved name `c`".to_string(),
+                                severity: DiagnosticSeverity::ERROR,
+                                is_primary: true,
+                                is_disk_based: true,
+                                group_id: 1,
+                                ..Default::default()
+                            },
+                        },
+                    ],
+                    cx,
+                )
+                .unwrap();
+            cx.emit(worktree::Event::DiskBasedDiagnosticsUpdated);
+        });
+
+        view.next_notification(&cx).await;
+        view.update(&mut cx, |view, cx| {
+            let editor = view.editor.update(cx, |editor, cx| editor.snapshot(cx));
+
+            assert_eq!(
+                editor_blocks(&editor, cx),
+                [
+                    (0, "path header block".into()),
+                    (2, "diagnostic header".into()),
+                    (7, "diagnostic header".into()),
+                    (12, "path header block".into()),
+                    (14, "diagnostic header".into()),
+                    (27, "diagnostic header".into()),
+                    (36, "collapsed context".into()),
+                ]
+            );
+            assert_eq!(
+                editor.text(),
+                concat!(
+                    //
+                    // consts.rs
+                    //
+                    "\n", // filename
+                    "\n", // padding
+                    // diagnostic group 1
+                    "\n", // primary message
+                    "\n", // padding
+                    "const a: i32 = 'a';\n",
+                    "\n", // supporting diagnostic
+                    "const b: i32 = c;\n",
+                    // diagnostic group 2
+                    "\n", // primary message
+                    "\n", // padding
+                    "const a: i32 = 'a';\n",
+                    "const b: i32 = c;\n",
+                    "\n", // supporting diagnostic
                     //
-                    // main.rs, diagnostic group 2
+                    // main.rs
                     //
+                    "\n", // filename
                     "\n", // padding
+                    // diagnostic group 1
+                    "\n", // primary message
+                    "\n", // padding
+                    "    let x = vec![];\n",
+                    "    let y = vec![];\n",
+                    "\n", // supporting diagnostic
+                    "    a(x);\n",
+                    "    b(y);\n",
+                    "\n", // supporting diagnostic
+                    "    // comment 1\n",
+                    "    // comment 2\n",
+                    "    c(y);\n",
+                    "\n", // supporting diagnostic
+                    "    d(x);\n",
+                    // diagnostic group 2
                     "\n", // primary message
                     "\n", // filename
                     "fn main() {\n",
@@ -879,4 +1075,20 @@ mod tests {
             );
         });
     }
+
+    fn editor_blocks(editor: &EditorSnapshot, cx: &AppContext) -> Vec<(u32, String)> {
+        editor
+            .blocks_in_range(0..editor.max_point().row())
+            .filter_map(|(row, block)| {
+                block
+                    .render(&BlockContext {
+                        cx,
+                        anchor_x: 0.,
+                        line_number_x: 0.,
+                    })
+                    .name()
+                    .map(|s| (row, s.to_string()))
+            })
+            .collect()
+    }
 }

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

@@ -5,7 +5,7 @@ use gpui::{AppContext, ElementBox};
 use language::Chunk;
 use parking_lot::Mutex;
 use std::{
-    cmp::{self, Ordering},
+    cmp::{self, Ordering, Reverse},
     fmt::Debug,
     ops::{Deref, Range},
     sync::{
@@ -69,6 +69,7 @@ where
 pub struct BlockContext<'a> {
     pub cx: &'a AppContext,
     pub anchor_x: f32,
+    pub line_number_x: f32,
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
@@ -275,7 +276,11 @@ impl BlockMap {
                         (position.row(), column, block.clone())
                     }),
             );
-            blocks_in_edit.sort_by_key(|(row, _, block)| (*row, block.disposition, block.id));
+
+            // When multiple blocks are on the same row, newer blocks appear above older
+            // blocks. This is arbitrary, but we currently rely on it in ProjectDiagnosticsEditor.
+            blocks_in_edit
+                .sort_by_key(|(row, _, block)| (*row, block.disposition, Reverse(block.id)));
 
             // For each of these blocks, insert a new isomorphic transform preceding the block,
             // and then insert the block itself.
@@ -939,18 +944,24 @@ mod tests {
                     start_row..start_row + block.height(),
                     block.column(),
                     block
-                        .render(&BlockContext { cx, anchor_x: 0. })
+                        .render(&BlockContext {
+                            cx,
+                            anchor_x: 0.,
+                            line_number_x: 0.,
+                        })
                         .name()
                         .unwrap()
                         .to_string(),
                 )
             })
             .collect::<Vec<_>>();
+
+        // When multiple blocks are on the same line, the newer blocks appear first.
         assert_eq!(
             blocks,
             &[
-                (1..2, 0, "block 1".to_string()),
-                (2..4, 2, "block 2".to_string()),
+                (1..3, 2, "block 2".to_string()),
+                (3..4, 0, "block 1".to_string()),
                 (7..10, 3, "block 3".to_string()),
             ]
         );
@@ -1261,13 +1272,15 @@ mod tests {
                     )
                 })
                 .collect::<Vec<_>>();
-            sorted_blocks
-                .sort_unstable_by_key(|(id, block)| (block.position.row, block.disposition, *id));
-            let mut sorted_blocks = sorted_blocks.into_iter().peekable();
+            sorted_blocks.sort_unstable_by_key(|(id, block)| {
+                (block.position.row, block.disposition, Reverse(*id))
+            });
+            let mut sorted_blocks_iter = sorted_blocks.iter().peekable();
 
             let input_buffer_rows = buffer_snapshot.buffer_rows(0).collect::<Vec<_>>();
             let mut expected_buffer_rows = Vec::new();
             let mut expected_text = String::new();
+            let mut expected_block_positions = Vec::new();
             let input_text = wraps_snapshot.text();
             for (row, input_line) in input_text.split('\n').enumerate() {
                 let row = row as u32;
@@ -1279,14 +1292,16 @@ mod tests {
                     .to_point(WrapPoint::new(row, 0), Bias::Left)
                     .row as usize];
 
-                while let Some((_, block)) = sorted_blocks.peek() {
+                while let Some((block_id, block)) = sorted_blocks_iter.peek() {
                     if block.position.row == row && block.disposition == BlockDisposition::Above {
+                        expected_block_positions
+                            .push((expected_text.matches('\n').count() as u32, *block_id));
                         let text = "\n".repeat(block.height as usize);
                         expected_text.push_str(&text);
                         for _ in 0..block.height {
                             expected_buffer_rows.push(None);
                         }
-                        sorted_blocks.next();
+                        sorted_blocks_iter.next();
                     } else {
                         break;
                     }
@@ -1296,14 +1311,16 @@ mod tests {
                 expected_buffer_rows.push(if soft_wrapped { None } else { buffer_row });
                 expected_text.push_str(input_line);
 
-                while let Some((_, block)) = sorted_blocks.peek() {
+                while let Some((block_id, block)) = sorted_blocks_iter.peek() {
                     if block.position.row == row && block.disposition == BlockDisposition::Below {
+                        expected_block_positions
+                            .push((expected_text.matches('\n').count() as u32 + 1, *block_id));
                         let text = "\n".repeat(block.height as usize);
                         expected_text.push_str(&text);
                         for _ in 0..block.height {
                             expected_buffer_rows.push(None);
                         }
-                        sorted_blocks.next();
+                        sorted_blocks_iter.next();
                     } else {
                         break;
                     }
@@ -1331,6 +1348,14 @@ mod tests {
                 );
             }
 
+            assert_eq!(
+                blocks_snapshot
+                    .blocks_in_range(0..(expected_row_count as u32))
+                    .map(|(row, block)| (row, block.id))
+                    .collect::<Vec<_>>(),
+                expected_block_positions
+            );
+
             let mut expected_longest_rows = Vec::new();
             let mut longest_line_len = -1_isize;
             for (row, line) in expected_lines.iter().enumerate() {

crates/editor/src/editor.rs 🔗

@@ -3691,6 +3691,7 @@ impl EditorSettings {
                     selection: Default::default(),
                     guest_selections: Default::default(),
                     syntax: Default::default(),
+                    diagnostic_path_header: Default::default(),
                     error_diagnostic: Default::default(),
                     invalid_error_diagnostic: Default::default(),
                     warning_diagnostic: Default::default(),
@@ -3851,47 +3852,6 @@ pub fn diagnostic_block_renderer(
     })
 }
 
-pub fn diagnostic_header_renderer(
-    buffer: ModelHandle<Buffer>,
-    diagnostic: Diagnostic,
-    is_valid: bool,
-    build_settings: BuildSettings,
-) -> RenderBlock {
-    Arc::new(move |cx| {
-        let settings = build_settings(cx);
-        let mut text_style = settings.style.text.clone();
-        let diagnostic_style = diagnostic_style(diagnostic.severity, is_valid, &settings.style);
-        text_style.color = diagnostic_style.text;
-        let file_path = if let Some(file) = buffer.read(&**cx).file() {
-            file.path().to_string_lossy().to_string()
-        } else {
-            "untitled".to_string()
-        };
-
-        Flex::column()
-            .with_child(
-                Text::new(diagnostic.message.clone(), text_style)
-                    .with_soft_wrap(false)
-                    .boxed(),
-            )
-            .with_child(Label::new(file_path, settings.style.text.clone()).boxed())
-            .aligned()
-            .left()
-            .contained()
-            .with_style(diagnostic_style.header)
-            .expanded()
-            .boxed()
-    })
-}
-
-pub fn context_header_renderer(build_settings: BuildSettings) -> RenderBlock {
-    Arc::new(move |cx| {
-        let settings = build_settings(cx);
-        let text_style = settings.style.text.clone();
-        Label::new("...".to_string(), text_style).boxed()
-    })
-}
-
 pub fn diagnostic_style(
     severity: DiagnosticSeverity,
     valid: bool,

crates/editor/src/element.rs 🔗

@@ -417,7 +417,7 @@ impl EditorElement {
 
     fn paint_blocks(
         &mut self,
-        text_bounds: RectF,
+        bounds: RectF,
         visible_bounds: RectF,
         layout: &mut LayoutState,
         cx: &mut PaintContext,
@@ -427,7 +427,7 @@ impl EditorElement {
         let scroll_top = scroll_position.y() * layout.line_height;
 
         for (row, element) in &mut layout.blocks {
-            let origin = text_bounds.origin()
+            let origin = bounds.origin()
                 + vec2f(-scroll_left, *row as f32 * layout.line_height - scroll_top);
             element.paint(origin, visible_bounds, cx);
         }
@@ -623,7 +623,9 @@ impl EditorElement {
         &mut self,
         rows: Range<u32>,
         snapshot: &EditorSnapshot,
-        text_width: f32,
+        width: f32,
+        line_number_x: f32,
+        text_x: f32,
         line_height: f32,
         style: &EditorStyle,
         line_layouts: &[text_layout::Line],
@@ -638,7 +640,7 @@ impl EditorElement {
                     .to_display_point(snapshot)
                     .row();
 
-                let anchor_x = if rows.contains(&anchor_row) {
+                let anchor_x = text_x + if rows.contains(&anchor_row) {
                     line_layouts[(anchor_row - rows.start) as usize]
                         .x_for_index(block.column() as usize)
                 } else {
@@ -646,11 +648,11 @@ impl EditorElement {
                         .x_for_index(block.column() as usize)
                 };
 
-                let mut element = block.render(&BlockContext { cx, anchor_x });
+                let mut element = block.render(&BlockContext { cx, anchor_x, line_number_x, });
                 element.layout(
                     SizeConstraint {
                         min: Vector2F::zero(),
-                        max: vec2f(text_width, block.height() as f32 * line_height),
+                        max: vec2f(width, block.height() as f32 * line_height),
                     },
                     cx,
                 );
@@ -810,7 +812,9 @@ impl Element for EditorElement {
         let blocks = self.layout_blocks(
             start_row..end_row,
             &snapshot,
-            text_size.x(),
+            size.x(),
+            gutter_padding,
+            gutter_width + text_offset.x(),
             line_height,
             &style,
             &line_layouts,
@@ -886,7 +890,7 @@ impl Element for EditorElement {
                 self.paint_gutter(gutter_bounds, visible_bounds, layout, cx);
             }
             self.paint_text(text_bounds, visible_bounds, layout, cx);
-            self.paint_blocks(text_bounds, visible_bounds, layout, cx);
+            self.paint_blocks(bounds, visible_bounds, layout, cx);
 
             cx.scene.pop_layer();
 

crates/gpui/src/app.rs 🔗

@@ -2870,6 +2870,28 @@ impl<T: View> ViewHandle<T> {
             .map_or(false, |focused_id| focused_id == self.view_id)
     }
 
+    pub fn next_notification(&self, cx: &TestAppContext) -> impl Future<Output = ()> {
+        let (mut tx, mut rx) = mpsc::channel(1);
+        let mut cx = cx.cx.borrow_mut();
+        let subscription = cx.observe(self, move |_, _| {
+            tx.blocking_send(()).ok();
+        });
+
+        let duration = if std::env::var("CI").is_ok() {
+            Duration::from_secs(5)
+        } else {
+            Duration::from_secs(1)
+        };
+
+        async move {
+            let notification = timeout(duration, rx.recv())
+                .await
+                .expect("next notification timed out");
+            drop(subscription);
+            notification.expect("model dropped while test was waiting for its next notification")
+        }
+    }
+
     pub fn condition(
         &self,
         cx: &TestAppContext,

crates/theme/src/theme.rs 🔗

@@ -251,6 +251,7 @@ pub struct EditorStyle {
     pub line_number_active: Color,
     pub guest_selections: Vec<SelectionStyle>,
     pub syntax: Arc<SyntaxTheme>,
+    pub diagnostic_path_header: DiagnosticStyle,
     pub error_diagnostic: DiagnosticStyle,
     pub invalid_error_diagnostic: DiagnosticStyle,
     pub warning_diagnostic: DiagnosticStyle,
@@ -316,6 +317,7 @@ impl InputEditorStyle {
             line_number_active: Default::default(),
             guest_selections: Default::default(),
             syntax: Default::default(),
+            diagnostic_path_header: Default::default(),
             error_diagnostic: Default::default(),
             invalid_error_diagnostic: Default::default(),
             warning_diagnostic: Default::default(),

crates/workspace/src/workspace.rs 🔗

@@ -980,7 +980,11 @@ impl Workspace {
     }
 
     pub fn activate_next_pane(&mut self, cx: &mut ViewContext<Self>) {
-        let ix = self.panes.iter().position(|pane| pane == &self.active_pane).unwrap();
+        let ix = self
+            .panes
+            .iter()
+            .position(|pane| pane == &self.active_pane)
+            .unwrap();
         let next_ix = (ix + 1) % self.panes.len();
         self.activate_pane(self.panes[next_ix].clone(), cx);
     }

crates/zed/assets/themes/_base.toml 🔗

@@ -187,7 +187,7 @@ corner_radius = 6
 
 [project_panel]
 extends = "$panel"
-padding.top = 6 # ($workspace.tab.height - $project_panel.entry.height) / 2
+padding.top = 6    # ($workspace.tab.height - $project_panel.entry.height) / 2
 
 [project_panel.entry]
 text = "$text.1"
@@ -256,21 +256,26 @@ invalid_warning_diagnostic = { text = "$text.3.color" }
 invalid_information_diagnostic = { text = "$text.3.color" }
 invalid_hint_diagnostic = { text = "$text.3.color" }
 
+[editor.diagnostic_path_header]
+text = "$text.0.color"
+header.background = "#ffffff08"
+header.border = { width = 1, top = true, color = "$border.0" }
+
 [editor.error_diagnostic]
 text = "$status.bad"
-header = { padding = { left = 10 }, background = "#ffffff08" }
+header.border = { width = 1, top = true, color = "$border.0" }
 
 [editor.warning_diagnostic]
 text = "$status.warn"
-header = { padding = { left = 10 }, background = "#ffffff08" }
+header.border = { width = 1, top = true, color = "$border.0" }
 
 [editor.information_diagnostic]
 text = "$status.info"
-header = { padding = { left = 10 }, background = "#ffffff08" }
+border = { width = 1, top = true, color = "$border.0" }
 
 [editor.hint_diagnostic]
 text = "$status.info"
-header = { padding = { left = 10 }, background = "#ffffff08" }
+border = { width = 1, top = true, color = "$border.0" }
 
 [project_diagnostics]
 background = "$surface.1"