From a9937ee8be6f414b2eef5ce86ac9f500320e5da0 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Jan 2022 13:57:44 -0800 Subject: [PATCH 1/7] Expand block decorations' bounds to include the gutter --- crates/editor/src/element.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 9f80eed2fb..31c2e4e661 100644 --- a/crates/editor/src/element.rs +++ b/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,8 @@ impl EditorElement { &mut self, rows: Range, snapshot: &EditorSnapshot, - text_width: f32, + width: f32, + text_x: f32, line_height: f32, style: &EditorStyle, line_layouts: &[text_layout::Line], @@ -638,7 +639,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 { @@ -650,7 +651,7 @@ impl EditorElement { 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 +811,8 @@ impl Element for EditorElement { let blocks = self.layout_blocks( start_row..end_row, &snapshot, - text_size.x(), + size.x(), + gutter_width + text_offset.x(), line_height, &style, &line_layouts, @@ -886,7 +888,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(); From b5ee095da9cd76d4c2156e8923cf73cbf9724c15 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Jan 2022 14:56:54 -0800 Subject: [PATCH 2/7] Deduplicate path names in the project diagnostics view --- crates/diagnostics/src/diagnostics.rs | 158 +++++++++++++++------ crates/editor/src/display_map/block_map.rs | 3 +- crates/editor/src/editor.rs | 41 ------ crates/editor/src/element.rs | 4 +- crates/zed/assets/themes/_base.toml | 2 +- 5 files changed, 120 insertions(+), 88 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index f58fab4dc0..adfc82105c 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/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, editor: ViewHandle, excerpts: ModelHandle, - path_states: Vec<(ProjectPath, Vec)>, + path_states: Vec, paths_to_update: HashMap>, build_settings: BuildSettings, settings: watch::Receiver, } +struct PathState { + path: ProjectPath, + header: Option, + diagnostic_groups: Vec, +} + struct DiagnosticGroupState { primary_diagnostic: DiagnosticEntry, primary_excerpt_ix: usize, @@ -208,11 +214,7 @@ impl ProjectDiagnosticsEditor { } } - fn update_excerpts( - &self, - paths: BTreeSet, - cx: &mut ViewContext, - ) { + fn update_excerpts(&self, paths: BTreeSet, cx: &mut ViewContext) { let project = self.model.read(cx).project.clone(); cx.spawn(|this, mut cx| { async move { @@ -220,9 +222,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 +239,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 +280,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 +338,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 +351,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 +401,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,32 +415,43 @@ impl ProjectDiagnosticsEditor { }); self.editor.update(cx, |editor, cx| { + blocks_to_remove.extend(path_state.header); editor.remove_blocks(blocks_to_remove, cx); + 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 mut 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), - height: block.height, - render: block.render, - disposition: block.disposition, - } - }), + header_block + .into_iter() + .chain(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), + height: block.height, + render: block.render, + disposition: block.disposition, + } + })), cx, ) .into_iter(); + path_state.header = block_ids.next(); for group_state in &mut groups_to_add { group_state.blocks = block_ids.by_ref().take(group_state.block_count).collect(); } }); 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 +461,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 +470,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 +480,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::(cx); } @@ -575,6 +594,57 @@ impl workspace::ItemView for ProjectDiagnosticsEditor { } } +fn path_header_renderer(buffer: ModelHandle, 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() + }; + Label::new(file_path, settings.style.text.clone()) + .aligned() + .left() + .contained() + .with_padding_left(cx.line_number_x) + .expanded() + .boxed() + }) +} + +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() + .boxed() + }) +} + +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) + .boxed() + }) +} + fn compare_diagnostics( lhs: &DiagnosticEntry, rhs: &DiagnosticEntry, diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 2d61a123b7..c542906776 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -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)] @@ -939,7 +940,7 @@ 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(), diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a7cf2365a9..55affde95d 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3851,47 +3851,6 @@ pub fn diagnostic_block_renderer( }) } -pub fn diagnostic_header_renderer( - buffer: ModelHandle, - 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, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 31c2e4e661..5180314c2a 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -624,6 +624,7 @@ impl EditorElement { rows: Range, snapshot: &EditorSnapshot, width: f32, + line_number_x: f32, text_x: f32, line_height: f32, style: &EditorStyle, @@ -647,7 +648,7 @@ 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(), @@ -812,6 +813,7 @@ impl Element for EditorElement { start_row..end_row, &snapshot, size.x(), + gutter_padding, gutter_width + text_offset.x(), line_height, &style, diff --git a/crates/zed/assets/themes/_base.toml b/crates/zed/assets/themes/_base.toml index f24a6997ba..e85638c225 100644 --- a/crates/zed/assets/themes/_base.toml +++ b/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" From 9ccf2f3f58d06fe2f18418949db6751b129c43a3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Jan 2022 15:17:18 -0800 Subject: [PATCH 3/7] Tweak theming of project diagnostics --- crates/diagnostics/src/diagnostics.rs | 6 +++++- crates/editor/src/editor.rs | 1 + crates/theme/src/theme.rs | 2 ++ crates/zed/assets/themes/_base.toml | 13 +++++++++---- 4 files changed, 17 insertions(+), 5 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index adfc82105c..905cc39347 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -602,10 +602,14 @@ fn path_header_renderer(buffer: ModelHandle, build_settings: BuildSettin } else { "untitled".to_string() }; - Label::new(file_path, settings.style.text.clone()) + 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() .boxed() diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 55affde95d..eb412f3dcb 100644 --- a/crates/editor/src/editor.rs +++ b/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(), diff --git a/crates/theme/src/theme.rs b/crates/theme/src/theme.rs index dcd378dcc5..3bd78f2634 100644 --- a/crates/theme/src/theme.rs +++ b/crates/theme/src/theme.rs @@ -251,6 +251,7 @@ pub struct EditorStyle { pub line_number_active: Color, pub guest_selections: Vec, pub syntax: Arc, + 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(), diff --git a/crates/zed/assets/themes/_base.toml b/crates/zed/assets/themes/_base.toml index e85638c225..95dc7eee60 100644 --- a/crates/zed/assets/themes/_base.toml +++ b/crates/zed/assets/themes/_base.toml @@ -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" From ac0d55222fa83a3ea03017e1d1dab8b1c1a73ea3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Jan 2022 15:40:21 -0800 Subject: [PATCH 4/7] Adjust project diagnostics test to reflect new block structure --- crates/diagnostics/src/diagnostics.rs | 36 +++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 905cc39347..d5dce3e075 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -829,11 +829,13 @@ mod tests { editor.text(), concat!( // - // main.rs, diagnostic group 1 + // main.rs // - "\n", // padding - "\n", // primary message "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding " let x = vec![];\n", " let y = vec![];\n", "\n", // supporting diagnostic @@ -845,12 +847,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 @@ -869,7 +868,7 @@ mod tests { 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)] ); }); }); @@ -908,18 +907,22 @@ mod tests { // // a.rs // - "\n", // padding - "\n", // primary message "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding "const a: i32 = 'a';\n", "\n", // supporting diagnostic "\n", // context line // - // main.rs, diagnostic group 1 + // main.rs // - "\n", // padding - "\n", // primary message "\n", // filename + "\n", // padding + // diagnostic group 1 + "\n", // primary message + "\n", // padding " let x = vec![];\n", " let y = vec![];\n", "\n", // supporting diagnostic @@ -931,10 +934,7 @@ 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 "fn main() {\n", From 6ad9ff10c1a1bae442222ab553d630b60236e1be Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Tue, 11 Jan 2022 18:17:42 -0800 Subject: [PATCH 5/7] Ensure path headers appear before first diagnostic header Co-Authored-By: Nathan Sobo --- crates/diagnostics/src/diagnostics.rs | 35 +++++++++++----------- crates/editor/src/display_map/block_map.rs | 14 +++++++-- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index d5dce3e075..7fc94286ff 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -423,28 +423,27 @@ impl ProjectDiagnosticsEditor { render: path_header_renderer(buffer, self.build_settings.clone()), disposition: BlockDisposition::Above, }); - let mut block_ids = editor - .insert_blocks( - header_block - .into_iter() - .chain(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), - height: block.height, - render: block.render, - disposition: block.disposition, - } - })), - cx, - ) - .into_iter(); + 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), + height: block.height, + render: block.render, + disposition: block.disposition, + } + }) + .chain(header_block.into_iter()), + cx, + ); - path_state.header = block_ids.next(); + 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() { diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index c542906776..e9c0bceeda 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/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::{ @@ -276,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. @@ -940,7 +944,11 @@ mod tests { start_row..start_row + block.height(), block.column(), block - .render(&BlockContext { cx, anchor_x: 0., line_number_x: 0., }) + .render(&BlockContext { + cx, + anchor_x: 0., + line_number_x: 0., + }) .name() .unwrap() .to_string(), From ed88fdcea26081cd43beb02272359548fd142abf Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 12 Jan 2022 11:34:57 -0800 Subject: [PATCH 6/7] Add unit test for diagnostic + path header ordering --- crates/diagnostics/src/diagnostics.rs | 239 ++++++++++++++++++++------ crates/gpui/src/app.rs | 22 +++ crates/workspace/src/workspace.rs | 6 +- 3 files changed, 216 insertions(+), 51 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 7fc94286ff..098e554574 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -160,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) { if let Some(existing) = workspace.item_of_type::(cx) { workspace.activate_item(&existing, cx); @@ -611,7 +606,7 @@ fn path_header_renderer(buffer: ModelHandle, build_settings: BuildSettin .with_style(style.header) .with_padding_left(cx.line_number_x) .expanded() - .boxed() + .named("path header block") }) } @@ -633,7 +628,7 @@ fn diagnostic_header_renderer( .with_style(diagnostic_style.header) .with_padding_left(cx.line_number_x) .expanded() - .boxed() + .named("diagnostic header") }) } @@ -644,7 +639,7 @@ fn context_header_renderer(build_settings: BuildSettings) -> RenderBlock { Label::new("…".to_string(), text_style) .contained() .with_padding_left(cx.line_number_x) - .boxed() + .named("collapsed context") }) } @@ -669,11 +664,10 @@ fn compare_diagnostics( #[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 _; @@ -681,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 params = cx.update(WorkspaceParams::test); + let project = params.project.clone(); + let workspace = cx.add_view(0, |cx| Workspace::new(¶ms, cx)); - 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": " + 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![]; @@ -717,10 +703,10 @@ mod tests { d(x); } " - .unindent(), - }), - ) - .await; + .unindent(), + }), + ) + .await; let worktree = project .update(&mut cx, |project, cx| { @@ -729,6 +715,7 @@ mod tests { .await .unwrap(); + // Create some diagnostics worktree.update(&mut cx, |worktree, cx| { worktree .update_diagnostic_entries( @@ -811,19 +798,25 @@ 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!( @@ -864,6 +857,7 @@ mod tests { ) ); + // Cursor is at the first diagnostic view.editor.update(cx, |editor, cx| { assert_eq!( editor.selected_display_ranges(cx), @@ -872,10 +866,11 @@ mod tests { }); }); + // 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), @@ -894,17 +889,26 @@ 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 @@ -913,7 +917,126 @@ mod tests { "\n", // padding "const a: i32 = 'a';\n", "\n", // supporting diagnostic - "\n", // context line + "const b: i32 = c;\n", + // + // 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", + " 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 // @@ -952,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() + } } diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 5b70981ba2..892c129547 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -2870,6 +2870,28 @@ impl ViewHandle { .map_or(false, |focused_id| focused_id == self.view_id) } + pub fn next_notification(&self, cx: &TestAppContext) -> impl Future { + 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, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index eec0a1d446..7faef17c1b 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -980,7 +980,11 @@ impl Workspace { } pub fn activate_next_pane(&mut self, cx: &mut ViewContext) { - 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); } From 1a672929e049a9ce21306663d7da513ffc30759e Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Wed, 12 Jan 2022 12:11:55 -0800 Subject: [PATCH 7/7] Adjust BlockMap tests to reflect new tiebreaking behavior Co-Authored-By: Nathan Sobo --- crates/editor/src/display_map/block_map.rs | 34 ++++++++++++++++------ 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index e9c0bceeda..d660307eb1 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -955,11 +955,13 @@ mod tests { ) }) .collect::>(); + + // 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()), ] ); @@ -1270,13 +1272,15 @@ mod tests { ) }) .collect::>(); - 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::>(); 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; @@ -1288,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; } @@ -1305,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; } @@ -1340,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::>(), + 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() {