zed-industries/zed
1
1
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2024-09-16 00:47:39 +03:00
Code Issues Packages Projects Releases Wiki Activity

Improve same line diagnostic rendering (#14741)

Browse Source 
Follow-up of https://github.com/zed-industries/zed/pull/14515

Fixed certain visual artifacts, related to multi line diagnostics and
block toggle rendering.
Also enabled diagnostics toolbar controls for the experimental view too.


Release Notes:

- N/A
This commit is contained in:
Kirill Bulatov 2024-07-18 17:22:38 +03:00 committed by GitHub
parent 24d9374744
commit ed3d3dc690
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 152 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
crates/diagnostics/src/diagnostics.rs
View File

@ -4,7 +4,7 @@ mod toolbar_controls;
#[cfg(test)] #[cfg(test)]
mod diagnostics_tests; mod diagnostics_tests;
mod grouped_diagnostics; pub(crate) mod grouped_diagnostics;
use anyhow::Result; use anyhow::Result;
use collections::{BTreeSet, HashSet}; use collections::{BTreeSet, HashSet};

123
crates/diagnostics/src/grouped_diagnostics.rs
View File

@ -51,18 +51,18 @@ pub fn init(cx: &mut AppContext) {
.detach(); .detach();
} }
struct GroupedDiagnosticsEditor { pub struct GroupedDiagnosticsEditor {
project: Model<Project>, pub project: Model<Project>,
workspace: WeakView<Workspace>, workspace: WeakView<Workspace>,
focus_handle: FocusHandle, focus_handle: FocusHandle,
editor: View<Editor>, editor: View<Editor>,
summary: DiagnosticSummary, summary: DiagnosticSummary,
excerpts: Model<MultiBuffer>, excerpts: Model<MultiBuffer>,
path_states: Vec<PathState>, path_states: Vec<PathState>,
paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>, pub paths_to_update: BTreeSet<(ProjectPath, LanguageServerId)>,
include_warnings: bool, pub include_warnings: bool,
context: u32, context: u32,
update_paths_tx: UnboundedSender<(ProjectPath, Option<LanguageServerId>)>, pub update_paths_tx: UnboundedSender<(ProjectPath, Option<LanguageServerId>)>,
_update_excerpts_task: Task<Result<()>>, _update_excerpts_task: Task<Result<()>>,
_subscription: Subscription, _subscription: Subscription,
} }
@ -260,7 +260,7 @@ impl GroupedDiagnosticsEditor {
} }
} }
fn toggle_warnings(&mut self, _: &ToggleWarnings, cx: &mut ViewContext<Self>) { pub fn toggle_warnings(&mut self, _: &ToggleWarnings, cx: &mut ViewContext<Self>) {
self.include_warnings = !self.include_warnings; self.include_warnings = !self.include_warnings;
self.enqueue_update_all_excerpts(cx); self.enqueue_update_all_excerpts(cx);
cx.notify(); cx.notify();
@ -297,7 +297,7 @@ impl GroupedDiagnosticsEditor {
/// to have changed. If a language server id is passed, then only the excerpts for /// to have changed. If a language server id is passed, then only the excerpts for
/// that language server's diagnostics will be updated. Otherwise, all stale excerpts /// that language server's diagnostics will be updated. Otherwise, all stale excerpts
/// will be refreshed. /// will be refreshed.
fn enqueue_update_stale_excerpts(&mut self, language_server_id: Option<LanguageServerId>) { pub fn enqueue_update_stale_excerpts(&mut self, language_server_id: Option<LanguageServerId>) {
for (path, server_id) in &self.paths_to_update { for (path, server_id) in &self.paths_to_update {
if language_server_id.map_or(true, |id| id == *server_id) { if language_server_id.map_or(true, |id| id == *server_id) {
self.update_paths_tx self.update_paths_tx
@ -320,7 +320,7 @@ impl GroupedDiagnosticsEditor {
}); });
// TODO kb change selections as in the old panel, to the next primary diagnostics // TODO kb change selections as in the old panel, to the next primary diagnostics
// TODO kb make [shift-]f8 to work, jump to the next block group // TODO make [shift-]f8 to work, jump to the next block group
let _was_empty = self.path_states.is_empty(); let _was_empty = self.path_states.is_empty();
let path_ix = match self.path_states.binary_search_by(|probe| { let path_ix = match self.path_states.binary_search_by(|probe| {
project::compare_paths((&probe.path.path, true), (&path_to_update.path, true)) project::compare_paths((&probe.path.path, true), (&path_to_update.path, true))
@ -340,7 +340,6 @@ impl GroupedDiagnosticsEditor {
} }
}; };
// TODO kb when warnings are turned off, there's a lot of refresh for many paths happening, why?
let max_severity = if self.include_warnings { let max_severity = if self.include_warnings {
DiagnosticSeverity::WARNING DiagnosticSeverity::WARNING
} else { } else {
@ -633,7 +632,7 @@ fn compare_diagnostic_ranges(
}) })
} }
// TODO kb wrong? What to do here instead? // TODO wrong? What to do here instead?
fn compare_diagnostic_range_edges( fn compare_diagnostic_range_edges(
old: &Range<language::Anchor>, old: &Range<language::Anchor>,
new: &Range<language::Anchor>, new: &Range<language::Anchor>,
@ -1316,59 +1315,67 @@ fn render_same_line_diagnostics(
.map(|diagnostic| diagnostic_text_lines(diagnostic)) .map(|diagnostic| diagnostic_text_lines(diagnostic))
.sum::<u8>(); .sum::<u8>();
let editor_handle = editor_handle.clone(); let editor_handle = editor_handle.clone();
let mut parent = v_flex(); let parent = h_flex()
let mut diagnostics_iter = diagnostics.iter().fuse(); .items_start()
if let Some(first_diagnostic) = diagnostics_iter.next() { .child(v_flex().size_full().when_some_else(
let mut renderer = diagnostic_block_renderer( toggle_expand_label,
first_diagnostic.clone(), |parent, label| {
Some(folded_block_height), parent.child(Button::new(cx.transform_block_id, label).on_click({
false, let diagnostics = Arc::clone(&diagnostics);
true, move |_, cx| {
); let new_expanded = !expanded;
parent = parent.child( button_expanded.store(new_expanded, atomic::Ordering::Release);
h_flex() let new_size = if new_expanded {
.when_some(toggle_expand_label, |parent, label| { expanded_block_height
parent.child(Button::new(cx.transform_block_id, label).on_click({ } else {
let diagnostics = Arc::clone(&diagnostics); folded_block_height
move |_, cx| { };
let new_expanded = !expanded; editor_handle.update(cx, |editor, cx| {
button_expanded.store(new_expanded, atomic::Ordering::Release); editor.replace_blocks(
let new_size = if new_expanded { HashMap::from_iter(Some((
expanded_block_height block_id,
} else { (
folded_block_height Some(new_size),
}; render_same_line_diagnostics(
editor_handle.update(cx, |editor, cx| { Arc::clone(&button_expanded),
editor.replace_blocks( Arc::clone(&diagnostics),
HashMap::from_iter(Some(( editor_handle.clone(),
block_id, folded_block_height,
(
Some(new_size),
render_same_line_diagnostics(
Arc::clone(&button_expanded),
Arc::clone(&diagnostics),
editor_handle.clone(),
folded_block_height,
),
), ),
))), ),
None, ))),
cx, None,
) cx,
}); )
} });
})) }
}) }))
.child(renderer(cx)), },
); |parent| {
} parent.child(
h_flex()
.size(IconSize::default().rems())
.invisible()
.flex_none(),
)
},
));
let max_message_rows = if expanded {
None
} else {
Some(folded_block_height)
};
let mut renderer =
diagnostic_block_renderer(first_diagnostic.clone(), max_message_rows, false, true);
let mut diagnostics_element = v_flex();
diagnostics_element = diagnostics_element.child(renderer(cx));
if expanded { if expanded {
for diagnostic in diagnostics_iter { for diagnostic in diagnostics.iter().skip(1) {
let mut renderer = diagnostic_block_renderer(diagnostic.clone(), None, false, true); let mut renderer = diagnostic_block_renderer(diagnostic.clone(), None, false, true);
parent = parent.child(renderer(cx)); diagnostics_element = diagnostics_element.child(renderer(cx));
} }
} }
parent.into_any_element() parent.child(diagnostics_element).into_any_element()
}) })
} }

98
crates/diagnostics/src/toolbar_controls.rs
View File

@ -1,11 +1,12 @@
use crate::ProjectDiagnosticsEditor; use crate::{grouped_diagnostics::GroupedDiagnosticsEditor, ProjectDiagnosticsEditor};
use gpui::{EventEmitter, ParentElement, Render, ViewContext, WeakView}; use futures::future::Either;
use gpui::{EventEmitter, ParentElement, Render, View, ViewContext, WeakView};
use ui::prelude::*; use ui::prelude::*;
use ui::{IconButton, IconName, Tooltip}; use ui::{IconButton, IconName, Tooltip};
use workspace::{item::ItemHandle, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView}; use workspace::{item::ItemHandle, ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView};
pub struct ToolbarControls { pub struct ToolbarControls {
editor: Option<WeakView<ProjectDiagnosticsEditor>>, editor: Option<Either<WeakView<ProjectDiagnosticsEditor>, WeakView<GroupedDiagnosticsEditor>>>,
} }
impl Render for ToolbarControls { impl Render for ToolbarControls {
@ -14,18 +15,33 @@ impl Render for ToolbarControls {
let mut has_stale_excerpts = false; let mut has_stale_excerpts = false;
let mut is_updating = false; let mut is_updating = false;
if let Some(editor) = self.editor.as_ref().and_then(|editor| editor.upgrade()) { if let Some(editor) = self.editor() {
let editor = editor.read(cx); match editor {
Either::Left(editor) => {
include_warnings = editor.include_warnings; let editor = editor.read(cx);
has_stale_excerpts = !editor.paths_to_update.is_empty(); include_warnings = editor.include_warnings;
is_updating = editor.update_paths_tx.len() > 0 has_stale_excerpts = !editor.paths_to_update.is_empty();
|| editor is_updating = editor.update_paths_tx.len() > 0
.project || editor
.read(cx) .project
.language_servers_running_disk_based_diagnostics() .read(cx)
.next() .language_servers_running_disk_based_diagnostics()
.is_some(); .next()
.is_some();
}
Either::Right(editor) => {
let editor = editor.read(cx);
include_warnings = editor.include_warnings;
has_stale_excerpts = !editor.paths_to_update.is_empty();
is_updating = editor.update_paths_tx.len() > 0
|| editor
.project
.read(cx)
.language_servers_running_disk_based_diagnostics()
.next()
.is_some();
}
}
} }
let tooltip = if include_warnings { let tooltip = if include_warnings {
@ -42,12 +58,19 @@ impl Render for ToolbarControls {
.disabled(is_updating) .disabled(is_updating)
.tooltip(move |cx| Tooltip::text("Update excerpts", cx)) .tooltip(move |cx| Tooltip::text("Update excerpts", cx))
.on_click(cx.listener(|this, _, cx| { .on_click(cx.listener(|this, _, cx| {
if let Some(editor) = if let Some(editor) = this.editor() {
this.editor.as_ref().and_then(|editor| editor.upgrade()) match editor {
{ Either::Left(editor) => {
editor.update(cx, |editor, _| { editor.update(cx, |editor, _| {
editor.enqueue_update_stale_excerpts(None); editor.enqueue_update_stale_excerpts(None);
}); });
}
Either::Right(editor) => {
editor.update(cx, |editor, _| {
editor.enqueue_update_stale_excerpts(None);
});
}
}
} }
})), })),
) )
@ -56,12 +79,19 @@ impl Render for ToolbarControls {
IconButton::new("toggle-warnings", IconName::ExclamationTriangle) IconButton::new("toggle-warnings", IconName::ExclamationTriangle)
.tooltip(move |cx| Tooltip::text(tooltip, cx)) .tooltip(move |cx| Tooltip::text(tooltip, cx))
.on_click(cx.listener(|this, _, cx| { .on_click(cx.listener(|this, _, cx| {
if let Some(editor) = if let Some(editor) = this.editor() {
this.editor.as_ref().and_then(|editor| editor.upgrade()) match editor {
{ Either::Left(editor) => {
editor.update(cx, |editor, cx| { editor.update(cx, |editor, cx| {
editor.toggle_warnings(&Default::default(), cx); editor.toggle_warnings(&Default::default(), cx);
}); });
}
Either::Right(editor) => {
editor.update(cx, |editor, cx| {
editor.toggle_warnings(&Default::default(), cx);
});
}
}
} }
})), })),
) )
@ -78,7 +108,10 @@ impl ToolbarItemView for ToolbarControls {
) -> ToolbarItemLocation { ) -> ToolbarItemLocation {
if let Some(pane_item) = active_pane_item.as_ref() { if let Some(pane_item) = active_pane_item.as_ref() {
if let Some(editor) = pane_item.downcast::<ProjectDiagnosticsEditor>() { if let Some(editor) = pane_item.downcast::<ProjectDiagnosticsEditor>() {
self.editor = Some(editor.downgrade()); self.editor = Some(Either::Left(editor.downgrade()));
ToolbarItemLocation::PrimaryRight
} else if let Some(editor) = pane_item.downcast::<GroupedDiagnosticsEditor>() {
self.editor = Some(Either::Right(editor.downgrade()));
ToolbarItemLocation::PrimaryRight ToolbarItemLocation::PrimaryRight
} else { } else {
ToolbarItemLocation::Hidden ToolbarItemLocation::Hidden
@ -93,4 +126,13 @@ impl ToolbarControls {
pub fn new() -> Self { pub fn new() -> Self {
ToolbarControls { editor: None } ToolbarControls { editor: None }
} }
fn editor(
&self,
) -> Option<Either<View<ProjectDiagnosticsEditor>, View<GroupedDiagnosticsEditor>>> {
Some(match self.editor.as_ref()? {
Either::Left(diagnostics) => Either::Left(diagnostics.upgrade()?),
Either::Right(grouped_diagnostics) => Either::Right(grouped_diagnostics.upgrade()?),
})
}
} }

24
crates/editor/src/editor.rs
View File

@ -12827,23 +12827,31 @@ pub fn highlight_diagnostic_message(
let mut prev_offset = 0; let mut prev_offset = 0;
let mut in_code_block = false; let mut in_code_block = false;
let has_row_limit = max_message_rows.is_some();
let mut newline_indices = diagnostic let mut newline_indices = diagnostic
.message .message
.match_indices('\n') .match_indices('\n')
.filter(|_| has_row_limit)
.map(|(ix, _)| ix) .map(|(ix, _)| ix)
.fuse() .fuse()
.peekable(); .peekable();
for (ix, _) in diagnostic
for (quote_ix, _) in diagnostic
.message .message
.match_indices('`') .match_indices('`')
.chain([(diagnostic.message.len(), "")]) .chain([(diagnostic.message.len(), "")])
{ {
let mut trimmed_ix = ix; let mut first_newline_ix = None;
while let Some(newline_index) = newline_indices.peek() { let mut last_newline_ix = None;
if *newline_index < ix { while let Some(newline_ix) = newline_indices.peek() {
if *newline_ix < quote_ix {
if first_newline_ix.is_none() {
first_newline_ix = Some(*newline_ix);
}
last_newline_ix = Some(*newline_ix);
if let Some(rows_left) = &mut max_message_rows { if let Some(rows_left) = &mut max_message_rows {
if *rows_left == 0 { if *rows_left == 0 {
trimmed_ix = newline_index.saturating_sub(1);
break; break;
} else { } else {
*rows_left -= 1; *rows_left -= 1;
@ -12855,14 +12863,14 @@ pub fn highlight_diagnostic_message(
} }
} }
let prev_len = text_without_backticks.len(); let prev_len = text_without_backticks.len();
let new_text = &diagnostic.message[prev_offset..trimmed_ix]; let new_text = &diagnostic.message[prev_offset..first_newline_ix.unwrap_or(quote_ix)];
text_without_backticks.push_str(new_text); text_without_backticks.push_str(new_text);
if in_code_block { if in_code_block {
code_ranges.push(prev_len..text_without_backticks.len()); code_ranges.push(prev_len..text_without_backticks.len());
} }
prev_offset = trimmed_ix + 1; prev_offset = last_newline_ix.unwrap_or(quote_ix) + 1;
in_code_block = !in_code_block; in_code_block = !in_code_block;
if trimmed_ix != ix { if first_newline_ix.map_or(false, |newline_ix| newline_ix < quote_ix) {
text_without_backticks.push_str("..."); text_without_backticks.push_str("...");
break; break;
} }