From 6c5b27af1d197dd4e32afe8da9010be3879749ec Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Tue, 14 Dec 2021 18:26:42 -0700 Subject: [PATCH] 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 --- crates/chat_panel/src/chat_panel.rs | 6 +- crates/diagnostics/src/diagnostics.rs | 102 ++++++++++++-------- crates/editor/src/display_map.rs | 9 +- crates/editor/src/display_map/block_map.rs | 21 ++-- crates/editor/src/editor.rs | 85 +++++++++------- crates/editor/src/element.rs | 3 +- crates/editor/src/multi_buffer.rs | 2 + crates/editor/src/multi_buffer/anchor.rs | 7 ++ crates/file_finder/src/file_finder.rs | 5 +- crates/go_to_line/src/go_to_line.rs | 6 +- crates/language/src/buffer.rs | 9 +- crates/language/src/diagnostic_set.rs | 40 ++++++++ crates/theme_selector/src/theme_selector.rs | 6 +- 13 files changed, 204 insertions(+), 97 deletions(-) diff --git a/crates/chat_panel/src/chat_panel.rs b/crates/chat_panel/src/chat_panel.rs index 44c4bd6295..7aca6daa95 100644 --- a/crates/chat_panel/src/chat_panel.rs +++ b/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, ) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 59f3acd368..4067bb5894 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -1,7 +1,10 @@ -use std::sync::Arc; +use std::{cmp, sync::Arc}; -use collections::HashMap; -use editor::{diagnostic_style, Editor, ExcerptProperties, MultiBuffer}; +use editor::{ + 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| { - Editor::for_buffer( - excerpts.clone(), - editor::settings_builder(excerpts.downgrade(), settings.clone()), - cx, - ) - }); + let build_settings = editor::settings_builder(excerpts.downgrade(), settings.clone()); + let editor = + cx.add_view(|cx| Editor::for_buffer(excerpts.clone(), build_settings.clone(), cx)); let project_paths = project .read(cx) @@ -91,58 +90,85 @@ 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::>(); - sorted_diagnostic_groups.sort_by_key(|group| group.0); + this.update(&mut cx, |this, cx| { + let mut blocks = Vec::new(); + this.excerpts.update(cx, |excerpts, excerpts_cx| { + for group in snapshot.diagnostic_groups::() { + 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), + ); - for entry in snapshot.all_diagnostics::() { - this.update(&mut cx, |this, cx| { - this.excerpts.update(cx, |excerpts, cx| { - excerpts.push_excerpt( + let primary_diagnostic = group.primary.diagnostic; + let excerpt_id = excerpts.push_excerpt( ExcerptProperties { buffer: &buffer, - range: entry.range, - header_height: entry - .diagnostic + range: Point::new(excerpt_start, 0) + ..Point::new( + 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>(()) } diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 838b136f75..2c55f0f988 100644 --- a/crates/editor/src/display_map.rs +++ b/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(&mut self, styles: HashMap) - where - F: 'static + Fn(&BlockContext) -> ElementBox, - { + pub fn replace_blocks(&mut self, styles: HashMap) { self.block_map.replace(styles); } diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index cd25e58899..44d6d95e0f 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/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 ElementBox>; + pub struct Block { id: BlockId, position: Anchor, height: u8, - render: Mutex ElementBox>>, + render: Mutex, disposition: BlockDisposition, } @@ -306,13 +308,10 @@ impl BlockMap { *transforms = new_transforms; } - pub fn replace(&mut self, mut element_builders: HashMap) - where - F: 'static + Fn(&BlockContext) -> ElementBox, - { + pub fn replace(&mut self, mut renderers: HashMap) { for block in &self.blocks { - if let Some(build_element) = element_builders.remove(&block.id) { - *block.render.lock() = Arc::new(build_element); + if let Some(render) = renderers.remove(&block.id) { + *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") diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index a82b02435d..70c81d84a3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -27,16 +27,13 @@ use language::{ BracketPair, Buffer, Diagnostic, DiagnosticSeverity, Language, Point, Selection, SelectionGoal, TransactionId, }; -use multi_buffer::{ - Anchor, AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, ToOffset, ToPoint, -}; -pub use multi_buffer::{ExcerptProperties, MultiBuffer}; +pub use multi_buffer::{Anchor, ExcerptProperties, MultiBuffer}; +use multi_buffer::{AnchorRangeExt, MultiBufferChunks, MultiBufferSnapshot, ToOffset, ToPoint}; 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 EditorSettings>; + pub struct Editor { handle: WeakViewHandle, buffer: ModelHandle, @@ -377,7 +376,7 @@ pub struct Editor { scroll_position: Vector2F, scroll_top_anchor: Anchor, autoscroll_request: Option, - build_settings: Rc 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( - build_settings: impl 'static + Fn(&AppContext) -> EditorSettings, - cx: &mut ViewContext, - ) -> Self { + pub fn single_line(build_settings: BuildSettings, cx: &mut ViewContext) -> 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 { let buffer = cx.add_model(|cx| Buffer::new(0, String::new(), cx)); @@ -458,10 +454,10 @@ impl Editor { pub fn for_buffer( buffer: ModelHandle, - build_settings: impl 'static + Fn(&AppContext) -> EditorSettings, + build_settings: BuildSettings, cx: &mut ViewContext, ) -> 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 { @@ -473,10 +469,10 @@ impl Editor { pub fn new( buffer: ModelHandle, - build_settings: Rc EditorSettings>>, + build_settings: BuildSettings, cx: &mut ViewContext, ) -> 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.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::(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.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::(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(); - let diagnostic = diagnostic.clone(); - new_styles.insert(*block_id, move |cx: &BlockContext| { - let diagnostic = diagnostic.clone(); - let settings = build_settings.borrow()(cx.cx); - render_diagnostic(diagnostic, &settings.style, is_valid, cx.anchor_x) - }); + new_styles.insert( + *block_id, + diagnostic_block_renderer( + diagnostic.clone(), + is_valid, + 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| { - let settings = build_settings.borrow()(cx.cx); - let diagnostic = diagnostic.clone(); - render_diagnostic(diagnostic, &settings.style, true, cx.anchor_x) - }), + render: diagnostic_block_renderer(diagnostic, true, build_settings), disposition: BlockDisposition::Below, } }), @@ -3431,6 +3424,18 @@ impl Editor { } } + pub fn insert_blocks

( + &mut self, + blocks: impl IntoIterator>, + cx: &mut ViewContext, + ) -> Vec + 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) -> 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 SelectionExt for Selection { } } +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, settings: watch::Receiver, -) -> impl Fn(&AppContext) -> EditorSettings { - move |cx| { +) -> BuildSettings { + 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::for_buffer(buffer, move |_| settings.clone(), cx) + Editor::for_buffer(buffer, Rc::new(move |_| settings.clone()), cx) } } diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 768b591003..f0f07426fd 100644 --- a/crates/editor/src/element.rs +++ b/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, ) diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index dace217839..96e65ca484 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -559,6 +559,8 @@ impl MultiBuffer { self.subscriptions.publish_mut([edit]); + cx.notify(); + id } diff --git a/crates/editor/src/multi_buffer/anchor.rs b/crates/editor/src/multi_buffer/anchor.rs index 2cc4817a92..8fea4799e8 100644 --- a/crates/editor/src/multi_buffer/anchor.rs +++ b/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(), diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 8fef0b6bdf..fd1a60041f 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/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, ) diff --git a/crates/go_to_line/src/go_to_line.rs b/crates/go_to_line/src/go_to_line.rs index 85c5a6439b..a037b78d92 100644 --- a/crates/go_to_line/src/go_to_line.rs +++ b/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, ) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index d1683241b3..f2964374b6 100644 --- a/crates/language/src/buffer.rs +++ b/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(&self) -> Vec> + where + O: FromAnchor + Ord + Copy, + { + self.diagnostics.groups(self) + } + pub fn diagnostic_group<'a, O>( &'a self, group_id: usize, diff --git a/crates/language/src/diagnostic_set.rs b/crates/language/src/diagnostic_set.rs index caef7569c5..58ef94a0d5 100644 --- a/crates/language/src/diagnostic_set.rs +++ b/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 { pub diagnostic: Diagnostic, } +pub struct DiagnosticGroup { + pub primary: DiagnosticEntry, + pub supporting: Vec>, +} + #[derive(Clone, Debug)] pub struct Summary { start: Anchor, @@ -98,6 +104,40 @@ impl DiagnosticSet { }) } + pub fn groups(&self, buffer: &text::BufferSnapshot) -> Vec> + where + O: FromAnchor + Ord + Copy, + { + let mut groups = + HashMap::>, Vec>)>::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::>(); + groups.sort_unstable_by_key(|group| group.primary.range.start); + + groups + } + pub fn group<'a, O: FromAnchor>( &'a self, group_id: usize, diff --git a/crates/theme_selector/src/theme_selector.rs b/crates/theme_selector/src/theme_selector.rs index 545b512a8b..b611330afd 100644 --- a/crates/theme_selector/src/theme_selector.rs +++ b/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, )