From 600358609be3e7afb22bd131b65c0920b4c9dcca Mon Sep 17 00:00:00 2001 From: dinocosta Date: Wed, 9 Jul 2025 21:03:04 +0100 Subject: [PATCH 1/4] feat(diagnostics): add buffer diagnostics editor Introduce a new diagnostics view, `BufferDiagnosticsEditor`, which allows users to view the diagnostics for the currently open file. A new action has also been introduced so users can deploy this view from the command palette, `diagnostics: deploy current file`. This view is very similar in behaviour to `ProjectDiagnosticsEditor`, but it was worth noting that, while `ProjectDiagnosticsEditor` reads and writes to the global `IncludeWarnings` setting, `BufferDiagnosticsEditor` only reads its value when first being created, otherwise it keeps its own `include_warnings` state so as to not affect the `ProjectDiagnosticsEditor` or other `BufferDiagnosticsEditor` entities. A new method was introduced to make it easier to retrieve the diagnostic summary for a specific path rather for all paths, `project::lsp_store::LspStore.diagnostic_summary_for_path`, seeing as `BufferDiagnosticsEditor` only needs to display the summary for a single path. --- crates/diagnostics/src/buffer_diagnostics.rs | 894 ++++++++++++++++++ crates/diagnostics/src/diagnostic_renderer.rs | 62 +- crates/diagnostics/src/diagnostics.rs | 45 +- crates/diagnostics/src/toolbar_controls.rs | 164 +++- crates/editor/src/editor.rs | 2 + crates/project/src/lsp_store.rs | 17 +- crates/project/src/project.rs | 7 + 7 files changed, 1124 insertions(+), 67 deletions(-) create mode 100644 crates/diagnostics/src/buffer_diagnostics.rs diff --git a/crates/diagnostics/src/buffer_diagnostics.rs b/crates/diagnostics/src/buffer_diagnostics.rs new file mode 100644 index 0000000000..ce2e666234 --- /dev/null +++ b/crates/diagnostics/src/buffer_diagnostics.rs @@ -0,0 +1,894 @@ +use crate::{ + DIAGNOSTICS_UPDATE_DELAY, IncludeWarnings, ToggleWarnings, context_range_for_entry, + diagnostic_renderer::{DiagnosticBlock, DiagnosticRenderer, DiagnosticsEditorHandle}, +}; +use anyhow::Result; +use collections::HashMap; +use editor::{ + DEFAULT_MULTIBUFFER_CONTEXT, Editor, EditorEvent, ExcerptRange, MultiBuffer, PathKey, + display_map::{BlockPlacement, BlockProperties, BlockStyle, CustomBlockId}, +}; +use gpui::{ + AnyElement, App, AppContext, Context, Entity, EntityId, EventEmitter, FocusHandle, Focusable, + InteractiveElement, IntoElement, ParentElement, Render, SharedString, Styled, Subscription, + Task, Window, actions, div, +}; +use language::{Buffer, DiagnosticEntry, Point}; +use project::{ + DiagnosticSummary, Event, Project, ProjectItem, ProjectPath, + project_settings::{DiagnosticSeverity, ProjectSettings}, +}; +use settings::Settings; +use std::{ + any::{Any, TypeId}, + cmp::Ordering, + sync::Arc, +}; +use text::{Anchor, BufferSnapshot, OffsetRangeExt}; +use ui::{Button, ButtonStyle, Icon, IconName, Label, Tooltip, h_flex, prelude::*}; +use util::{ResultExt, paths::PathExt}; +use workspace::{ + ItemHandle, ItemNavHistory, ToolbarItemLocation, Workspace, + item::{BreadcrumbText, Item, ItemEvent, TabContentParams}, +}; + +actions!( + diagnostics, + [ + /// Opens the project diagnostics view for the currently focused file. + DeployCurrentFile, + ] +); + +/// The `BufferDiagnosticsEditor` is meant to be used when dealing specifically +/// with diagnostics for a single buffer, as only the excerpts of the buffer +/// where diagnostics are available are displayed. +pub(crate) struct BufferDiagnosticsEditor { + pub project: Entity, + focus_handle: FocusHandle, + editor: Entity, + /// The current diagnostic entries in the `BufferDiagnosticsEditor`. Used to + /// allow quick comparison of updated diagnostics, to confirm if anything + /// has changed. + pub(crate) diagnostics: Vec>, + /// The blocks used to display the diagnostics' content in the editor, next + /// to the excerpts where the diagnostic originated. + blocks: Vec, + /// Multibuffer to contain all excerpts that contain diagnostics, which are + /// to be rendered in the editor. + multibuffer: Entity, + /// The buffer for which the editor is displaying diagnostics and excerpts + /// for. + buffer: Option>, + /// The path for which the editor is displaying diagnostics for. + project_path: ProjectPath, + /// Summary of the number of warnings and errors for the path. Used to + /// display the number of warnings and errors in the tab's content. + summary: DiagnosticSummary, + /// Whether to include warnings in the list of diagnostics shown in the + /// editor. + pub(crate) include_warnings: bool, + /// Keeps track of whether there's a background task already running to + /// update the excerpts, in order to avoid firing multiple tasks for this purpose. + pub(crate) update_excerpts_task: Option>>, + /// The project's subscription, responsible for processing events related to + /// diagnostics. + _subscription: Subscription, +} + +impl BufferDiagnosticsEditor { + /// Creates new instance of the `BufferDiagnosticsEditor` which can then be + /// displayed by adding it to a pane. + fn new( + project_path: ProjectPath, + project_handle: Entity, + include_warnings: bool, + window: &mut Window, + cx: &mut Context, + ) -> Self { + // Subscribe to project events related to diagnostics so the + // `BufferDiagnosticsEditor` can update its state accordingly. + let project_event_subscription = cx.subscribe_in( + &project_handle, + window, + |buffer_diagnostics_editor, _project, event, window, cx| match event { + Event::DiskBasedDiagnosticsStarted { .. } => { + cx.notify(); + } + Event::DiskBasedDiagnosticsFinished { .. } => { + buffer_diagnostics_editor.update_all_excerpts(window, cx); + } + Event::DiagnosticsUpdated { + paths, + language_server_id, + } => { + // When diagnostics have been updated, the + // `BufferDiagnosticsEditor` should update its state only if + // one of the paths matches its `project_path`, otherwise + // the event should be ignored. + if paths.contains(&buffer_diagnostics_editor.project_path) { + buffer_diagnostics_editor.update_diagnostic_summary(cx); + + if buffer_diagnostics_editor.editor.focus_handle(cx).contains_focused(window, cx) || buffer_diagnostics_editor.focus_handle.contains_focused(window, cx) { + log::debug!("diagnostics updated for server {language_server_id}. recording change"); + } else { + log::debug!("diagnostics updated for server {language_server_id}. updating excerpts"); + buffer_diagnostics_editor.update_all_excerpts(window, cx); + } + } + } + _ => {} + }, + ); + + let focus_handle = cx.focus_handle(); + + cx.on_focus_in( + &focus_handle, + window, + |buffer_diagnostics_editor, window, cx| buffer_diagnostics_editor.focus_in(window, cx), + ) + .detach(); + + cx.on_focus_out( + &focus_handle, + window, + |buffer_diagnostics_editor, _event, window, cx| { + buffer_diagnostics_editor.focus_out(window, cx) + }, + ) + .detach(); + + let summary = project_handle + .read(cx) + .diagnostic_summary_for_path(&project_path, cx); + + let multibuffer = cx.new(|cx| MultiBuffer::new(project_handle.read(cx).capability())); + let max_severity = Self::max_diagnostics_severity(include_warnings); + let editor = cx.new(|cx| { + let mut editor = Editor::for_multibuffer( + multibuffer.clone(), + Some(project_handle.clone()), + window, + cx, + ); + editor.set_vertical_scroll_margin(5, cx); + editor.disable_inline_diagnostics(); + editor.set_max_diagnostics_severity(max_severity, cx); + editor.set_all_diagnostics_active(cx); + editor + }); + + // Subscribe to events triggered by the editor in order to correctly + // update the buffer's excerpts. + cx.subscribe_in( + &editor, + window, + |buffer_diagnostics_editor, _editor, event: &EditorEvent, window, cx| { + cx.emit(event.clone()); + + match event { + // If the user tries to focus on the editor but there's actually + // no excerpts for the buffer, focus back on the + // `BufferDiagnosticsEditor` instance. + EditorEvent::Focused => { + if buffer_diagnostics_editor.multibuffer.read(cx).is_empty() { + window.focus(&buffer_diagnostics_editor.focus_handle); + } + } + EditorEvent::Blurred => { + buffer_diagnostics_editor.update_all_excerpts(window, cx) + } + _ => {} + } + }, + ) + .detach(); + + let diagnostics = vec![]; + let update_excerpts_task = None; + let buffer_diagnostics_editor = Self { + project: project_handle, + focus_handle, + editor, + diagnostics, + blocks: Default::default(), + multibuffer, + buffer: None, + project_path, + summary, + include_warnings, + update_excerpts_task, + _subscription: project_event_subscription, + }; + + let project = buffer_diagnostics_editor.project.clone(); + let project_path = buffer_diagnostics_editor.project_path.clone(); + + cx.spawn_in(window, async move |editor, cx| { + let buffer = project + .update(cx, |project, cx| project.open_buffer(project_path, cx))? + .await + .log_err(); + + editor.update_in(cx, |editor, window, cx| { + editor.buffer = buffer; + editor.update_all_diagnostics(window, cx); + cx.notify(); + }) + }) + .detach(); + + buffer_diagnostics_editor + } + + fn deploy( + workspace: &mut Workspace, + _: &DeployCurrentFile, + window: &mut Window, + cx: &mut Context, + ) { + // Determine the currently opened path by finding the active editor and + // finding the project path for the buffer. + // If there's no active editor with a project path, avoiding deploying + // the buffer diagnostics view. + if let Some(project_path) = workspace + .active_item_as::(cx) + .map_or(None, |editor| editor.project_path(cx)) + { + // Check if there's already a `BufferDiagnosticsEditor` tab for this + // same path, and if so, focus on that one instead of creating a new + // one. + let existing_editor = workspace + .items_of_type::(cx) + .find(|editor| editor.read(cx).project_path == project_path); + + if let Some(editor) = existing_editor { + workspace.activate_item(&editor, true, true, window, cx); + } else { + let include_warnings = match cx.try_global::() { + Some(include_warnings) => include_warnings.0, + None => ProjectSettings::get_global(cx).diagnostics.include_warnings, + }; + + let item = cx.new(|cx| { + Self::new( + project_path, + workspace.project().clone(), + include_warnings, + window, + cx, + ) + }); + + workspace.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx); + } + } + } + + pub fn register( + workspace: &mut Workspace, + _window: Option<&mut Window>, + _: &mut Context, + ) { + workspace.register_action(Self::deploy); + } + + fn update_all_diagnostics(&mut self, window: &mut Window, cx: &mut Context) { + self.update_all_excerpts(window, cx); + } + + fn update_diagnostic_summary(&mut self, cx: &mut Context) { + let project = self.project.read(cx); + + self.summary = project.diagnostic_summary_for_path(&self.project_path, cx); + } + + /// Enqueue an update to the excerpts and diagnostic blocks being shown in + /// the editor. + pub(crate) fn update_all_excerpts(&mut self, window: &mut Window, cx: &mut Context) { + // If there's already a task updating the excerpts, early return and let + // the other task finish. + if self.update_excerpts_task.is_some() { + return; + } + + let buffer = self.buffer.clone(); + + self.update_excerpts_task = Some(cx.spawn_in(window, async move |editor, cx| { + cx.background_executor() + .timer(DIAGNOSTICS_UPDATE_DELAY) + .await; + + if let Some(buffer) = buffer { + editor + .update_in(cx, |editor, window, cx| { + editor.update_excerpts(buffer, window, cx) + })? + .await?; + }; + + let _ = editor.update(cx, |editor, cx| { + editor.update_excerpts_task = None; + cx.notify(); + }); + + Ok(()) + })); + } + + /// Updates the excerpts in the `BufferDiagnosticsEditor` for a single + /// buffer. + fn update_excerpts( + &mut self, + buffer: Entity, + window: &mut Window, + cx: &mut Context, + ) -> Task> { + let was_empty = self.multibuffer.read(cx).is_empty(); + let buffer_snapshot = buffer.read(cx).snapshot(); + let buffer_snapshot_max = buffer_snapshot.max_point(); + let max_severity = Self::max_diagnostics_severity(self.include_warnings) + .into_lsp() + .unwrap_or(lsp::DiagnosticSeverity::WARNING); + + cx.spawn_in(window, async move |buffer_diagnostics_editor, mut cx| { + // Fetch the diagnostics for the whole of the buffer + // (`Point::zero()..buffer_snapshot.max_point()`) so we can confirm + // if the diagnostics changed, if it didn't, early return as there's + // nothing to update. + let diagnostics = buffer_snapshot + .diagnostics_in_range::<_, Anchor>(Point::zero()..buffer_snapshot_max, false) + .collect::>(); + + let unchanged = + buffer_diagnostics_editor.update(cx, |buffer_diagnostics_editor, _cx| { + if buffer_diagnostics_editor + .diagnostics_are_unchanged(&diagnostics, &buffer_snapshot) + { + return true; + } + + buffer_diagnostics_editor.set_diagnostics(&diagnostics); + return false; + })?; + + if unchanged { + return Ok(()); + } + + // Mapping between the Group ID and a vector of DiagnosticEntry. + let mut grouped: HashMap> = HashMap::default(); + for entry in diagnostics { + grouped + .entry(entry.diagnostic.group_id) + .or_default() + .push(DiagnosticEntry { + range: entry.range.to_point(&buffer_snapshot), + diagnostic: entry.diagnostic, + }) + } + + let mut blocks: Vec = Vec::new(); + for (_, group) in grouped { + // If the minimum severity of the group is higher than the + // maximum severity, or it doesn't even have severity, skip this + // group. + if group + .iter() + .map(|d| d.diagnostic.severity) + .min() + .is_none_or(|severity| severity > max_severity) + { + continue; + } + + let diagnostic_blocks = cx.update(|_window, cx| { + DiagnosticRenderer::diagnostic_blocks_for_group( + group, + buffer_snapshot.remote_id(), + Some(DiagnosticsEditorHandle::Buffer( + buffer_diagnostics_editor.clone(), + )), + cx, + ) + })?; + + // For each of the diagnostic blocks to be displayed in the + // editor, figure out its index in the list of blocks. + // + // The following rules are used to determine the order: + // 1. Blocks with a lower start position should come first. + // 2. If two blocks have the same start position, the one with + // the higher end position should come first. + for diagnostic_block in diagnostic_blocks { + let index = blocks.partition_point(|probe| { + match probe + .initial_range + .start + .cmp(&diagnostic_block.initial_range.start) + { + Ordering::Less => true, + Ordering::Greater => false, + Ordering::Equal => { + probe.initial_range.end > diagnostic_block.initial_range.end + } + } + }); + + blocks.insert(index, diagnostic_block); + } + } + + // Build the excerpt ranges for this specific buffer's diagnostics, + // so those excerpts can later be used to update the excerpts shown + // in the editor. + // This is done by iterating over the list of diagnostic blocks and + // determine what range does the diagnostic block span. + let mut excerpt_ranges: Vec> = Vec::new(); + + for diagnostic_block in blocks.iter() { + let excerpt_range = context_range_for_entry( + diagnostic_block.initial_range.clone(), + DEFAULT_MULTIBUFFER_CONTEXT, + buffer_snapshot.clone(), + &mut cx, + ) + .await; + + let index = excerpt_ranges + .binary_search_by(|probe| { + probe + .context + .start + .cmp(&excerpt_range.start) + .then(probe.context.end.cmp(&excerpt_range.end)) + .then( + probe + .primary + .start + .cmp(&diagnostic_block.initial_range.start), + ) + .then(probe.primary.end.cmp(&diagnostic_block.initial_range.end)) + .then(Ordering::Greater) + }) + .unwrap_or_else(|index| index); + + excerpt_ranges.insert( + index, + ExcerptRange { + context: excerpt_range, + primary: diagnostic_block.initial_range.clone(), + }, + ) + } + + // Finally, update the editor's content with the new excerpt ranges + // for this editor, as well as the diagnostic blocks. + buffer_diagnostics_editor.update_in(cx, |buffer_diagnostics_editor, window, cx| { + // Remove the list of `CustomBlockId` from the editor's display + // map, ensuring that if any diagnostics have been solved, the + // associated block stops being shown. + let block_ids = buffer_diagnostics_editor.blocks.clone(); + + buffer_diagnostics_editor.editor.update(cx, |editor, cx| { + editor.display_map.update(cx, |display_map, cx| { + display_map.remove_blocks(block_ids.into_iter().collect(), cx); + }) + }); + + let (anchor_ranges, _) = + buffer_diagnostics_editor + .multibuffer + .update(cx, |multibuffer, cx| { + multibuffer.set_excerpt_ranges_for_path( + PathKey::for_buffer(&buffer, cx), + buffer.clone(), + &buffer_snapshot, + excerpt_ranges, + cx, + ) + }); + + if was_empty { + if let Some(anchor_range) = anchor_ranges.first() { + let range_to_select = anchor_range.start..anchor_range.start; + + buffer_diagnostics_editor.editor.update(cx, |editor, cx| { + editor.change_selections(Default::default(), window, cx, |selection| { + selection.select_anchor_ranges([range_to_select]) + }) + }); + + // If the `BufferDiagnosticsEditor` is currently + // focused, move focus to its editor. + if buffer_diagnostics_editor.focus_handle.is_focused(window) { + buffer_diagnostics_editor + .editor + .read(cx) + .focus_handle(cx) + .focus(window); + } + } + } + + // Build new diagnostic blocks to be added to the editor's + // display map for the new diagnostics. Update the `blocks` + // property before finishing, to ensure the blocks are removed + // on the next execution. + let editor_blocks = + anchor_ranges + .into_iter() + .zip(blocks.into_iter()) + .map(|(anchor, block)| { + let editor = buffer_diagnostics_editor.editor.downgrade(); + + BlockProperties { + placement: BlockPlacement::Near(anchor.start), + height: Some(1), + style: BlockStyle::Flex, + render: Arc::new(move |block_context| { + block.render_block(editor.clone(), block_context) + }), + priority: 1, + } + }); + + let block_ids = buffer_diagnostics_editor.editor.update(cx, |editor, cx| { + editor.display_map.update(cx, |display_map, cx| { + display_map.insert_blocks(editor_blocks, cx) + }) + }); + + buffer_diagnostics_editor.blocks = block_ids; + cx.notify() + }) + }) + } + + fn set_diagnostics(&mut self, diagnostics: &Vec>) { + self.diagnostics = diagnostics.clone(); + } + + fn diagnostics_are_unchanged( + &self, + diagnostics: &Vec>, + snapshot: &BufferSnapshot, + ) -> bool { + if self.diagnostics.len() != diagnostics.len() { + return false; + } + + self.diagnostics + .iter() + .zip(diagnostics.iter()) + .all(|(existing, new)| { + existing.diagnostic.message == new.diagnostic.message + && existing.diagnostic.severity == new.diagnostic.severity + && existing.diagnostic.is_primary == new.diagnostic.is_primary + && existing.range.to_offset(snapshot) == new.range.to_offset(snapshot) + }) + } + + fn focus_in(&mut self, window: &mut Window, cx: &mut Context) { + // If the `BufferDiagnosticsEditor` is focused and the multibuffer is + // not empty, focus on the editor instead, which will allow the user to + // start interacting and editing the buffer's contents. + if self.focus_handle.is_focused(window) && !self.multibuffer.read(cx).is_empty() { + self.editor.focus_handle(cx).focus(window) + } + } + + fn focus_out(&mut self, window: &mut Window, cx: &mut Context) { + if !self.focus_handle.is_focused(window) && !self.editor.focus_handle(cx).is_focused(window) + { + self.update_all_excerpts(window, cx); + } + } + + pub fn toggle_warnings( + &mut self, + _: &ToggleWarnings, + window: &mut Window, + cx: &mut Context, + ) { + let include_warnings = !self.include_warnings; + let max_severity = Self::max_diagnostics_severity(include_warnings); + + self.editor.update(cx, |editor, cx| { + editor.set_max_diagnostics_severity(max_severity, cx); + }); + + self.include_warnings = include_warnings; + self.diagnostics.clear(); + self.update_all_diagnostics(window, cx); + } + + fn max_diagnostics_severity(include_warnings: bool) -> DiagnosticSeverity { + match include_warnings { + true => DiagnosticSeverity::Warning, + false => DiagnosticSeverity::Error, + } + } +} + +impl Focusable for BufferDiagnosticsEditor { + fn focus_handle(&self, _: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl EventEmitter for BufferDiagnosticsEditor {} + +impl Item for BufferDiagnosticsEditor { + type Event = EditorEvent; + + fn act_as_type<'a>( + &'a self, + type_id: std::any::TypeId, + self_handle: &'a Entity, + _: &'a App, + ) -> Option { + if type_id == TypeId::of::() { + Some(self_handle.to_any()) + } else if type_id == TypeId::of::() { + Some(self.editor.to_any()) + } else { + None + } + } + + fn added_to_workspace( + &mut self, + workspace: &mut Workspace, + window: &mut Window, + cx: &mut Context, + ) { + self.editor.update(cx, |editor, cx| { + editor.added_to_workspace(workspace, window, cx) + }); + } + + fn breadcrumb_location(&self, _: &App) -> ToolbarItemLocation { + ToolbarItemLocation::PrimaryLeft + } + + fn breadcrumbs(&self, theme: &theme::Theme, cx: &App) -> Option> { + self.editor.breadcrumbs(theme, cx) + } + + fn can_save(&self, _cx: &App) -> bool { + true + } + + fn clone_on_split( + &self, + _workspace_id: Option, + window: &mut Window, + cx: &mut Context, + ) -> Option> + where + Self: Sized, + { + Some(cx.new(|cx| { + BufferDiagnosticsEditor::new( + self.project_path.clone(), + self.project.clone(), + self.include_warnings, + window, + cx, + ) + })) + } + + fn deactivated(&mut self, window: &mut Window, cx: &mut Context) { + self.editor + .update(cx, |editor, cx| editor.deactivated(window, cx)); + } + + fn for_each_project_item(&self, cx: &App, f: &mut dyn FnMut(EntityId, &dyn ProjectItem)) { + self.editor.for_each_project_item(cx, f); + } + + fn has_conflict(&self, cx: &App) -> bool { + self.multibuffer.read(cx).has_conflict(cx) + } + + fn has_deleted_file(&self, cx: &App) -> bool { + self.multibuffer.read(cx).has_deleted_file(cx) + } + + fn is_dirty(&self, cx: &App) -> bool { + self.multibuffer.read(cx).is_dirty(cx) + } + + fn is_singleton(&self, _cx: &App) -> bool { + false + } + + fn navigate( + &mut self, + data: Box, + window: &mut Window, + cx: &mut Context, + ) -> bool { + self.editor + .update(cx, |editor, cx| editor.navigate(data, window, cx)) + } + + fn reload( + &mut self, + project: Entity, + window: &mut Window, + cx: &mut Context, + ) -> Task> { + self.editor.reload(project, window, cx) + } + + fn save( + &mut self, + options: workspace::item::SaveOptions, + project: Entity, + window: &mut Window, + cx: &mut Context, + ) -> Task> { + self.editor.save(options, project, window, cx) + } + + fn save_as( + &mut self, + _project: Entity, + _path: ProjectPath, + _window: &mut Window, + _cx: &mut Context, + ) -> Task> { + unreachable!() + } + + fn set_nav_history( + &mut self, + nav_history: ItemNavHistory, + _window: &mut Window, + cx: &mut Context, + ) { + self.editor.update(cx, |editor, _| { + editor.set_nav_history(Some(nav_history)); + }) + } + + // Builds the content to be displayed in the tab. + fn tab_content(&self, params: TabContentParams, _window: &Window, _app: &App) -> AnyElement { + let error_count = self.summary.error_count; + let warning_count = self.summary.warning_count; + let label = Label::new(self.project_path.path.to_sanitized_string()); + + h_flex() + .gap_1() + .child(label.color(params.text_color())) + .when(error_count == 0 && warning_count == 0, |parent| { + parent.child( + h_flex() + .gap_1() + .child(Icon::new(IconName::Check).color(Color::Success)), + ) + }) + .when(error_count > 0, |parent| { + parent.child( + h_flex() + .gap_1() + .child(Icon::new(IconName::XCircle).color(Color::Error)) + .child(Label::new(error_count.to_string()).color(params.text_color())), + ) + }) + .when(warning_count > 0, |parent| { + parent.child( + h_flex() + .gap_1() + .child(Icon::new(IconName::Warning).color(Color::Warning)) + .child(Label::new(warning_count.to_string()).color(params.text_color())), + ) + }) + .into_any_element() + } + + fn tab_content_text(&self, _detail: usize, _app: &App) -> SharedString { + "Buffer Diagnostics".into() + } + + fn tab_tooltip_text(&self, _: &App) -> Option { + Some( + format!( + "Buffer Diagnostics - {}", + self.project_path.path.to_sanitized_string() + ) + .into(), + ) + } + + fn telemetry_event_text(&self) -> Option<&'static str> { + Some("Buffer Diagnostics Opened") + } + + fn to_item_events(event: &EditorEvent, f: impl FnMut(ItemEvent)) { + Editor::to_item_events(event, f) + } +} + +impl Render for BufferDiagnosticsEditor { + fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { + let filename = self.project_path.path.to_sanitized_string(); + let error_count = self.summary.error_count; + let warning_count = match self.include_warnings { + true => self.summary.warning_count, + false => 0, + }; + + let child = if error_count + warning_count == 0 { + let label = match warning_count { + 0 => "No problems in", + _ => "No errors in", + }; + + v_flex() + .key_context("EmptyPane") + .size_full() + .gap_1() + .justify_center() + .items_center() + .text_center() + .bg(cx.theme().colors().editor_background) + .child( + div() + .h_flex() + .child(Label::new(label).color(Color::Muted)) + .child( + Button::new("open-file", filename) + .style(ButtonStyle::Transparent) + .tooltip(Tooltip::text("Open File")) + .on_click(cx.listener(|buffer_diagnostics, _, window, cx| { + if let Some(workspace) = window.root::().flatten() { + workspace.update(cx, |workspace, cx| { + workspace + .open_path( + buffer_diagnostics.project_path.clone(), + None, + true, + window, + cx, + ) + .detach_and_log_err(cx); + }) + } + })), + ), + ) + .when(self.summary.warning_count > 0, |div| { + let label = match self.summary.warning_count { + 1 => "Show 1 warning".into(), + warning_count => format!("Show {} warnings", warning_count), + }; + + div.child( + Button::new("diagnostics-show-warning-label", label).on_click(cx.listener( + |buffer_diagnostics_editor, _, window, cx| { + buffer_diagnostics_editor.toggle_warnings( + &Default::default(), + window, + cx, + ); + cx.notify(); + }, + )), + ) + }) + } else { + div().size_full().child(self.editor.clone()) + }; + + div() + .key_context("Diagnostics") + .track_focus(&self.focus_handle(cx)) + .size_full() + .child(child) + } +} diff --git a/crates/diagnostics/src/diagnostic_renderer.rs b/crates/diagnostics/src/diagnostic_renderer.rs index e9731f84ce..4456775e92 100644 --- a/crates/diagnostics/src/diagnostic_renderer.rs +++ b/crates/diagnostics/src/diagnostic_renderer.rs @@ -19,6 +19,41 @@ use ui::{ use util::maybe; use crate::ProjectDiagnosticsEditor; +use crate::buffer_diagnostics::BufferDiagnosticsEditor; + +#[derive(Clone)] +pub(crate) enum DiagnosticsEditorHandle { + Project(WeakEntity), + Buffer(WeakEntity), +} + +impl DiagnosticsEditorHandle { + fn get_diagnostics_for_buffer( + &self, + buffer_id: BufferId, + cx: &App, + ) -> Vec> { + match self { + // Not sure if possible, as currently there shouldn't be a way for this + // method to be called for a buffer other than the one the + // `BufferDiagnosticsEditor` is working with, but we should probably + // save the ID of the buffer that it is working with, so that, if it + // doesn't match the argument, we can return an empty vector. + Self::Buffer(editor) => editor + .read_with(cx, |editor, _cx| editor.diagnostics.clone()) + .unwrap_or(vec![]), + Self::Project(editor) => editor + .read_with(cx, |editor, _cx| { + editor + .diagnostics + .get(&buffer_id) + .cloned() + .unwrap_or_default() + }) + .unwrap_or(vec![]), + } + } +} pub struct DiagnosticRenderer; @@ -26,7 +61,7 @@ impl DiagnosticRenderer { pub fn diagnostic_blocks_for_group( diagnostic_group: Vec>, buffer_id: BufferId, - diagnostics_editor: Option>, + diagnostics_editor: Option, cx: &mut App, ) -> Vec { let Some(primary_ix) = diagnostic_group @@ -130,6 +165,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer { cx: &mut App, ) -> Vec> { let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, cx); + blocks .into_iter() .map(|block| { @@ -182,7 +218,7 @@ pub(crate) struct DiagnosticBlock { pub(crate) initial_range: Range, pub(crate) severity: DiagnosticSeverity, pub(crate) markdown: Entity, - pub(crate) diagnostics_editor: Option>, + pub(crate) diagnostics_editor: Option, } impl DiagnosticBlock { @@ -233,7 +269,7 @@ impl DiagnosticBlock { pub fn open_link( editor: &mut Editor, - diagnostics_editor: &Option>, + diagnostics_editor: &Option, link: SharedString, window: &mut Window, cx: &mut Context, @@ -254,18 +290,10 @@ impl DiagnosticBlock { if let Some(diagnostics_editor) = diagnostics_editor { if let Some(diagnostic) = diagnostics_editor - .read_with(cx, |diagnostics, _| { - diagnostics - .diagnostics - .get(&buffer_id) - .cloned() - .unwrap_or_default() - .into_iter() - .filter(|d| d.diagnostic.group_id == group_id) - .nth(ix) - }) - .ok() - .flatten() + .get_diagnostics_for_buffer(buffer_id, cx) + .into_iter() + .filter(|d| d.diagnostic.group_id == group_id) + .nth(ix) { let multibuffer = editor.buffer().read(cx); let Some(snapshot) = multibuffer @@ -297,9 +325,9 @@ impl DiagnosticBlock { }; } - fn jump_to( + fn jump_to( editor: &mut Editor, - range: Range, + range: Range, window: &mut Window, cx: &mut Context, ) { diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 1c27e820a0..817679b782 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -1,12 +1,14 @@ pub mod items; mod toolbar_controls; +mod buffer_diagnostics; mod diagnostic_renderer; #[cfg(test)] mod diagnostics_tests; use anyhow::Result; +use buffer_diagnostics::BufferDiagnosticsEditor; use collections::{BTreeSet, HashMap}; use diagnostic_renderer::DiagnosticBlock; use editor::{ @@ -44,6 +46,8 @@ use workspace::{ searchable::SearchableItemHandle, }; +use crate::diagnostic_renderer::DiagnosticsEditorHandle; + actions!( diagnostics, [ @@ -63,6 +67,7 @@ impl Global for IncludeWarnings {} pub fn init(cx: &mut App) { editor::set_diagnostic_renderer(diagnostic_renderer::DiagnosticRenderer {}, cx); cx.observe_new(ProjectDiagnosticsEditor::register).detach(); + cx.observe_new(BufferDiagnosticsEditor::register).detach(); } pub(crate) struct ProjectDiagnosticsEditor { @@ -84,6 +89,7 @@ pub(crate) struct ProjectDiagnosticsEditor { impl EventEmitter for ProjectDiagnosticsEditor {} const DIAGNOSTICS_UPDATE_DELAY: Duration = Duration::from_millis(50); +const DIAGNOSTICS_SUMMARY_UPDATE_DELAY: Duration = Duration::from_millis(30); impl Render for ProjectDiagnosticsEditor { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { @@ -94,11 +100,11 @@ impl Render for ProjectDiagnosticsEditor { }; let child = if warning_count + self.summary.error_count == 0 { - let label = if self.summary.warning_count == 0 { - SharedString::new_static("No problems in workspace") - } else { - SharedString::new_static("No errors in workspace") + let label = match self.summary.warning_count { + 0 => SharedString::new_static("No problems in workspace"), + _ => SharedString::new_static("No errors in workspace"), }; + v_flex() .key_context("EmptyPane") .size_full() @@ -142,7 +148,7 @@ impl Render for ProjectDiagnosticsEditor { } impl ProjectDiagnosticsEditor { - fn register( + pub fn register( workspace: &mut Workspace, _window: Option<&mut Window>, _: &mut Context, @@ -158,7 +164,7 @@ impl ProjectDiagnosticsEditor { cx: &mut Context, ) -> Self { let project_event_subscription = - cx.subscribe_in(&project_handle, window, |this, project, event, window, cx| match event { + cx.subscribe_in(&project_handle, window, |this, _project, event, window, cx| match event { project::Event::DiskBasedDiagnosticsStarted { .. } => { cx.notify(); } @@ -171,13 +177,12 @@ impl ProjectDiagnosticsEditor { paths, } => { this.paths_to_update.extend(paths.clone()); - let project = project.clone(); this.diagnostic_summary_update = cx.spawn(async move |this, cx| { cx.background_executor() - .timer(Duration::from_millis(30)) + .timer(DIAGNOSTICS_SUMMARY_UPDATE_DELAY) .await; this.update(cx, |this, cx| { - this.summary = project.read(cx).diagnostic_summary(false, cx); + this.update_diagnostic_summary(cx); }) .log_err(); }); @@ -323,6 +328,7 @@ impl ProjectDiagnosticsEditor { let is_active = workspace .active_item(cx) .is_some_and(|item| item.item_id() == existing.item_id()); + workspace.activate_item(&existing, true, !is_active, window, cx); } else { let workspace_handle = cx.entity().downgrade(); @@ -380,22 +386,25 @@ impl ProjectDiagnosticsEditor { /// currently have diagnostics or are currently present in this view. fn update_all_excerpts(&mut self, window: &mut Window, cx: &mut Context) { self.project.update(cx, |project, cx| { - let mut paths = project + let mut project_paths = project .diagnostic_summaries(false, cx) - .map(|(path, _, _)| path) + .map(|(project_path, _, _)| project_path) .collect::>(); + self.multibuffer.update(cx, |multibuffer, cx| { for buffer in multibuffer.all_buffers() { if let Some(file) = buffer.read(cx).file() { - paths.insert(ProjectPath { + project_paths.insert(ProjectPath { path: file.path().clone(), worktree_id: file.worktree_id(cx), }); } } }); - self.paths_to_update = paths; + + self.paths_to_update = project_paths; }); + self.update_stale_excerpts(window, cx); } @@ -425,6 +434,7 @@ impl ProjectDiagnosticsEditor { let was_empty = self.multibuffer.read(cx).is_empty(); let buffer_snapshot = buffer.read(cx).snapshot(); let buffer_id = buffer_snapshot.remote_id(); + let max_severity = if self.include_warnings { lsp::DiagnosticSeverity::WARNING } else { @@ -438,6 +448,7 @@ impl ProjectDiagnosticsEditor { false, ) .collect::>(); + let unchanged = this.update(cx, |this, _| { if this.diagnostics.get(&buffer_id).is_some_and(|existing| { this.diagnostics_are_unchanged(existing, &diagnostics, &buffer_snapshot) @@ -472,7 +483,7 @@ impl ProjectDiagnosticsEditor { crate::diagnostic_renderer::DiagnosticRenderer::diagnostic_blocks_for_group( group, buffer_snapshot.remote_id(), - Some(this.clone()), + Some(DiagnosticsEditorHandle::Project(this.clone())), cx, ) })?; @@ -501,6 +512,7 @@ impl ProjectDiagnosticsEditor { cx, ) .await; + let i = excerpt_ranges .binary_search_by(|probe| { probe @@ -570,6 +582,7 @@ impl ProjectDiagnosticsEditor { priority: 1, } }); + let block_ids = this.editor.update(cx, |editor, cx| { editor.display_map.update(cx, |display_map, cx| { display_map.insert_blocks(editor_blocks, cx) @@ -600,6 +613,10 @@ impl ProjectDiagnosticsEditor { }) }) } + + fn update_diagnostic_summary(&mut self, cx: &mut Context) { + self.summary = self.project.read(cx).diagnostic_summary(false, cx); + } } impl Focusable for ProjectDiagnosticsEditor { diff --git a/crates/diagnostics/src/toolbar_controls.rs b/crates/diagnostics/src/toolbar_controls.rs index 404db39164..0a2da959c2 100644 --- a/crates/diagnostics/src/toolbar_controls.rs +++ b/crates/diagnostics/src/toolbar_controls.rs @@ -1,33 +1,63 @@ -use crate::{ProjectDiagnosticsEditor, ToggleDiagnosticsRefresh}; -use gpui::{Context, Entity, EventEmitter, ParentElement, Render, WeakEntity, Window}; +use crate::{BufferDiagnosticsEditor, ProjectDiagnosticsEditor, ToggleDiagnosticsRefresh}; +use gpui::{Context, EventEmitter, ParentElement, Render, WeakEntity, Window}; use ui::prelude::*; use ui::{IconButton, IconButtonShape, IconName, Tooltip}; use workspace::{ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, item::ItemHandle}; pub struct ToolbarControls { - editor: Option>, + editor: Option, } +enum DiagnosticsEditorHandle { + Project(WeakEntity), + Buffer(WeakEntity), +} + +impl DiagnosticsEditorHandle {} + impl Render for ToolbarControls { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { - let mut include_warnings = false; let mut has_stale_excerpts = false; + let mut include_warnings = false; let mut is_updating = false; - if let Some(editor) = self.diagnostics() { - let diagnostics = editor.read(cx); - include_warnings = diagnostics.include_warnings; - has_stale_excerpts = !diagnostics.paths_to_update.is_empty(); - is_updating = diagnostics.update_excerpts_task.is_some() - || diagnostics - .project - .read(cx) - .language_servers_running_disk_based_diagnostics(cx) - .next() - .is_some(); + match &self.editor { + Some(DiagnosticsEditorHandle::Project(editor)) => { + if let Some(editor) = editor.upgrade() { + let diagnostics = editor.read(cx); + include_warnings = diagnostics.include_warnings; + has_stale_excerpts = !diagnostics.paths_to_update.is_empty(); + is_updating = diagnostics.update_excerpts_task.is_some() + || diagnostics + .project + .read(cx) + .language_servers_running_disk_based_diagnostics(cx) + .next() + .is_some(); + } + } + Some(DiagnosticsEditorHandle::Buffer(editor)) => { + if let Some(editor) = editor.upgrade() { + let diagnostics = editor.read(cx); + include_warnings = diagnostics.include_warnings; + // TODO: How to calculate this for the + // `BufferDiagnosticsEditor`? Should we simply keep track if + // there are any updates to the diagnostics for the path and + // mark that instead of automatically updating? + has_stale_excerpts = false; + is_updating = diagnostics.update_excerpts_task.is_some() + || diagnostics + .project + .read(cx) + .language_servers_running_disk_based_diagnostics(cx) + .next() + .is_some(); + } + } + None => {} } - let tooltip = if include_warnings { + let warning_tooltip = if include_warnings { "Exclude Warnings" } else { "Include Warnings" @@ -52,11 +82,32 @@ impl Render for ToolbarControls { &ToggleDiagnosticsRefresh, )) .on_click(cx.listener(move |toolbar_controls, _, _, cx| { - if let Some(diagnostics) = toolbar_controls.diagnostics() { - diagnostics.update(cx, |diagnostics, cx| { - diagnostics.update_excerpts_task = None; - cx.notify(); - }); + match toolbar_controls.editor() { + Some(DiagnosticsEditorHandle::Buffer( + buffer_diagnostics_editor, + )) => { + let _ = buffer_diagnostics_editor.update( + cx, + |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.update_excerpts_task = + None; + cx.notify(); + }, + ); + } + Some(DiagnosticsEditorHandle::Project( + project_diagnostics_editor, + )) => { + let _ = project_diagnostics_editor.update( + cx, + |project_diagnostics_editor, cx| { + project_diagnostics_editor.update_excerpts_task = + None; + cx.notify(); + }, + ); + } + None => {} } })), ) @@ -71,12 +122,32 @@ impl Render for ToolbarControls { &ToggleDiagnosticsRefresh, )) .on_click(cx.listener({ - move |toolbar_controls, _, window, cx| { - if let Some(diagnostics) = toolbar_controls.diagnostics() { - diagnostics.update(cx, move |diagnostics, cx| { - diagnostics.update_all_excerpts(window, cx); - }); + move |toolbar_controls, _, window, cx| match toolbar_controls + .editor() + { + Some(DiagnosticsEditorHandle::Buffer( + buffer_diagnostics_editor, + )) => { + let _ = buffer_diagnostics_editor.update( + cx, + |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor + .update_all_excerpts(window, cx); + }, + ); } + Some(DiagnosticsEditorHandle::Project( + project_diagnostics_editor, + )) => { + let _ = project_diagnostics_editor.update( + cx, + |project_diagnostics_editor, cx| { + project_diagnostics_editor + .update_all_excerpts(window, cx); + }, + ); + } + None => {} } })), ) @@ -86,13 +157,33 @@ impl Render for ToolbarControls { IconButton::new("toggle-warnings", IconName::Warning) .icon_color(warning_color) .shape(IconButtonShape::Square) - .tooltip(Tooltip::text(tooltip)) - .on_click(cx.listener(|this, _, window, cx| { - if let Some(editor) = this.diagnostics() { - editor.update(cx, |editor, cx| { - editor.toggle_warnings(&Default::default(), window, cx); - }); + .tooltip(Tooltip::text(warning_tooltip)) + .on_click(cx.listener(|this, _, window, cx| match &this.editor { + Some(DiagnosticsEditorHandle::Project(project_diagnostics_editor)) => { + let _ = project_diagnostics_editor.update( + cx, + |project_diagnostics_editor, cx| { + project_diagnostics_editor.toggle_warnings( + &Default::default(), + window, + cx, + ); + }, + ); } + Some(DiagnosticsEditorHandle::Buffer(buffer_diagnostics_editor)) => { + let _ = buffer_diagnostics_editor.update( + cx, + |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.toggle_warnings( + &Default::default(), + window, + cx, + ); + }, + ); + } + _ => {} })), ) } @@ -109,7 +200,10 @@ impl ToolbarItemView for ToolbarControls { ) -> ToolbarItemLocation { if let Some(pane_item) = active_pane_item.as_ref() { if let Some(editor) = pane_item.downcast::() { - self.editor = Some(editor.downgrade()); + self.editor = Some(DiagnosticsEditorHandle::Project(editor.downgrade())); + ToolbarItemLocation::PrimaryRight + } else if let Some(editor) = pane_item.downcast::() { + self.editor = Some(DiagnosticsEditorHandle::Buffer(editor.downgrade())); ToolbarItemLocation::PrimaryRight } else { ToolbarItemLocation::Hidden @@ -131,7 +225,7 @@ impl ToolbarControls { ToolbarControls { editor: None } } - fn diagnostics(&self) -> Option> { - self.editor.as_ref()?.upgrade() + fn editor(&self) -> Option<&DiagnosticsEditorHandle> { + self.editor.as_ref() } } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 29e009fdf8..2231cf2f05 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -18739,6 +18739,8 @@ impl Editor { } } + /// Returns the project path for the editor's buffer, if any buffer is + /// opened in the editor. pub fn project_path(&self, cx: &App) -> Option { if let Some(buffer) = self.buffer.read(cx).as_singleton() { buffer.read(cx).project_path(cx) diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index deebaedd74..201dbac56d 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -82,7 +82,6 @@ use node_runtime::read_package_installed_version; use parking_lot::Mutex; use postage::{mpsc, sink::Sink, stream::Stream, watch}; use rand::prelude::*; - use rpc::{ AnyProtoClient, proto::{FromProto, LspRequestId, LspRequestMessage as _, ToProto}, @@ -7118,6 +7117,22 @@ impl LspStore { summary } + /// Returns the diagnostic summary for a specific project path. + pub fn diagnostic_summary_for_path( + &self, + project_path: &ProjectPath, + _: &App, + ) -> DiagnosticSummary { + // TODO: If there's multiple `DiagnosticSummary` but for + // different language servers, which one should be returned? + self.diagnostic_summaries + .get(&project_path.worktree_id) + .and_then(|map| map.get(&project_path.path)) + .and_then(|summaries| summaries.iter().next()) + .map(|(_language_server_id, summary)| summary.clone()) + .unwrap_or_default() + } + pub fn diagnostic_summaries<'a>( &'a self, include_ignored: bool, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 9fd4eed641..d7179f949e 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4283,6 +4283,13 @@ impl Project { .diagnostic_summary(include_ignored, cx) } + /// Returns a summary of the diagnostics for the provided project path only. + pub fn diagnostic_summary_for_path(&self, path: &ProjectPath, cx: &App) -> DiagnosticSummary { + self.lsp_store + .read(cx) + .diagnostic_summary_for_path(path, cx) + } + pub fn diagnostic_summaries<'a>( &'a self, include_ignored: bool, From da6546d51b02f1ce63c802cef8f10cdc32433619 Mon Sep 17 00:00:00 2001 From: dinocosta Date: Thu, 14 Aug 2025 15:53:40 +0100 Subject: [PATCH 2/4] test: add buffer diagnostics editor tests Update the `diagnostics::diagnostics_tests` module with three new tests for the `BufferDiagnosticsEditor` view: - `test_buffer_diagnostics` - Asserts that the `BufferDiagnosticsEditor` displays diagnostics only for the provided project path, ignoring other paths with diagnostics. - `test_buffer_diagnostics_without_warnings` - Asserts that if `include_warnings` is `false`, the `BufferDiagnosticsEditor` displays only errors in the editor, without showing warnings. - `test_buffer_diagnostics_multiple_servers` - Asserts that if the project path has multiple diagnostics for different language servers, all diagnostics are shown and included in the summary. Additionally, the `project::lsp_store::LspStore.diagnostic_summary_for_path` method has been updated to return the total count of warnings and errors across all language servers for the provided path, rather than just the first language server. This change aligns it with the behavior when diagnostics are displayed, as all language servers are considered. This commit also updates the `BufferDiagnosticsEditor::deploy` function in order to fetch the buffer directly from the active editor and provide it to `BufferDiagnosticsEditor::new`. This avoids having to spawn the background task that would open the buffer and update the `BufferDiagnosticsEditor.buffer` field. --- crates/diagnostics/src/buffer_diagnostics.rs | 73 +++- crates/diagnostics/src/diagnostics_tests.rs | 434 +++++++++++++++++++ crates/diagnostics/src/toolbar_controls.rs | 5 - crates/project/src/lsp_store.rs | 26 +- 4 files changed, 503 insertions(+), 35 deletions(-) diff --git a/crates/diagnostics/src/buffer_diagnostics.rs b/crates/diagnostics/src/buffer_diagnostics.rs index ce2e666234..753e8ed5d8 100644 --- a/crates/diagnostics/src/buffer_diagnostics.rs +++ b/crates/diagnostics/src/buffer_diagnostics.rs @@ -26,7 +26,7 @@ use std::{ }; use text::{Anchor, BufferSnapshot, OffsetRangeExt}; use ui::{Button, ButtonStyle, Icon, IconName, Label, Tooltip, h_flex, prelude::*}; -use util::{ResultExt, paths::PathExt}; +use util::paths::PathExt; use workspace::{ ItemHandle, ItemNavHistory, ToolbarItemLocation, Workspace, item::{BreadcrumbText, Item, ItemEvent, TabContentParams}, @@ -79,9 +79,10 @@ pub(crate) struct BufferDiagnosticsEditor { impl BufferDiagnosticsEditor { /// Creates new instance of the `BufferDiagnosticsEditor` which can then be /// displayed by adding it to a pane. - fn new( + pub fn new( project_path: ProjectPath, project_handle: Entity, + buffer: Option>, include_warnings: bool, window: &mut Window, cx: &mut Context, @@ -187,14 +188,14 @@ impl BufferDiagnosticsEditor { let diagnostics = vec![]; let update_excerpts_task = None; - let buffer_diagnostics_editor = Self { + let mut buffer_diagnostics_editor = Self { project: project_handle, focus_handle, editor, diagnostics, blocks: Default::default(), multibuffer, - buffer: None, + buffer, project_path, summary, include_warnings, @@ -202,23 +203,7 @@ impl BufferDiagnosticsEditor { _subscription: project_event_subscription, }; - let project = buffer_diagnostics_editor.project.clone(); - let project_path = buffer_diagnostics_editor.project_path.clone(); - - cx.spawn_in(window, async move |editor, cx| { - let buffer = project - .update(cx, |project, cx| project.open_buffer(project_path, cx))? - .await - .log_err(); - - editor.update_in(cx, |editor, window, cx| { - editor.buffer = buffer; - editor.update_all_diagnostics(window, cx); - cx.notify(); - }) - }) - .detach(); - + buffer_diagnostics_editor.update_all_diagnostics(window, cx); buffer_diagnostics_editor } @@ -232,9 +217,8 @@ impl BufferDiagnosticsEditor { // finding the project path for the buffer. // If there's no active editor with a project path, avoiding deploying // the buffer diagnostics view. - if let Some(project_path) = workspace - .active_item_as::(cx) - .map_or(None, |editor| editor.project_path(cx)) + if let Some(editor) = workspace.active_item_as::(cx) + && let Some(project_path) = editor.project_path(cx) { // Check if there's already a `BufferDiagnosticsEditor` tab for this // same path, and if so, focus on that one instead of creating a new @@ -255,6 +239,7 @@ impl BufferDiagnosticsEditor { Self::new( project_path, workspace.project().clone(), + editor.read(cx).buffer().read(cx).as_singleton(), include_warnings, window, cx, @@ -512,6 +497,11 @@ impl BufferDiagnosticsEditor { } } + // Cloning the blocks before moving ownership so these can later + // be used to set the block contents for testing purposes. + #[cfg(test)] + let cloned_blocks = blocks.clone(); + // Build new diagnostic blocks to be added to the editor's // display map for the new diagnostics. Update the `blocks` // property before finishing, to ensure the blocks are removed @@ -540,6 +530,30 @@ impl BufferDiagnosticsEditor { }) }); + // In order to be able to verify which diagnostic blocks are + // rendered in the editor, the `set_block_content_for_tests` + // function must be used, so that the + // `editor::test::editor_content_with_blocks` function can then + // be called to fetch these blocks. + #[cfg(test)] + { + for (block_id, block) in block_ids.iter().zip(cloned_blocks.iter()) { + let markdown = block.markdown.clone(); + editor::test::set_block_content_for_tests( + &buffer_diagnostics_editor.editor, + *block_id, + cx, + move |cx| { + markdown::MarkdownElement::rendered_text( + markdown.clone(), + cx, + editor::hover_popover::diagnostics_markdown_style, + ) + }, + ); + } + } + buffer_diagnostics_editor.blocks = block_ids; cx.notify() }) @@ -610,6 +624,16 @@ impl BufferDiagnosticsEditor { false => DiagnosticSeverity::Error, } } + + #[cfg(test)] + pub fn editor(&self) -> &Entity { + &self.editor + } + + #[cfg(test)] + pub fn summary(&self) -> &DiagnosticSummary { + &self.summary + } } impl Focusable for BufferDiagnosticsEditor { @@ -674,6 +698,7 @@ impl Item for BufferDiagnosticsEditor { BufferDiagnosticsEditor::new( self.project_path.clone(), self.project.clone(), + self.buffer.clone(), self.include_warnings, window, cx, diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs index 4a544f9ea7..c1ad5d4ed6 100644 --- a/crates/diagnostics/src/diagnostics_tests.rs +++ b/crates/diagnostics/src/diagnostics_tests.rs @@ -1566,6 +1566,440 @@ async fn go_to_diagnostic_with_severity(cx: &mut TestAppContext) { cx.assert_editor_state(indoc! {"error ˇwarning info hint"}); } +#[gpui::test] +async fn test_buffer_diagnostics(cx: &mut TestAppContext) { + init_test(cx); + + // We'll be creating two different files, both with diagnostics, so we can + // later verify that, since the `BufferDiagnosticsEditor` only shows + // diagnostics for the provided path, the diagnostics for the other file + // will not be shown, contrary to what happens with + // `ProjectDiagnosticsEditor`. + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/test"), + json!({ + "main.rs": " + fn main() { + let x = vec![]; + let y = vec![]; + a(x); + b(y); + c(y); + d(x); + } + " + .unindent(), + "other.rs": " + fn other() { + let unused = 42; + undefined_function(); + } + " + .unindent(), + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await; + let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let project_path = project::ProjectPath { + worktree_id: project.read_with(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }), + path: Arc::from(Path::new("main.rs")), + }; + let buffer = project + .update(cx, |project, cx| { + project.open_buffer(project_path.clone(), cx) + }) + .await + .ok(); + + // Create the diagnostics for `main.rs`. + let language_server_id = LanguageServerId(0); + let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap(); + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); + + lsp_store.update(cx, |lsp_store, cx| { + lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams { + uri: uri.clone(), + diagnostics: vec![ + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)), + severity: Some(lsp::DiagnosticSeverity::WARNING), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: Some(vec![ + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 9))), + message: "move occurs because `y` has type `Vec`, which does not implement the `Copy` trait".to_string() + }, + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(4, 6), lsp::Position::new(4, 7))), + message: "value moved here".to_string() + }, + ]), + ..Default::default() + }, + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: Some(vec![ + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9))), + message: "move occurs because `x` has type `Vec`, which does not implement the `Copy` trait".to_string() + }, + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(3, 6), lsp::Position::new(3, 7))), + message: "value moved here".to_string() + }, + ]), + ..Default::default() + } + ], + version: None + }, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap(); + + // Create diagnostics for other.rs to ensure that the file and + // diagnostics are not included in `BufferDiagnosticsEditor` when it is + // deployed for main.rs. + lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams { + uri: lsp::Url::from_file_path(path!("/test/other.rs")).unwrap(), + diagnostics: vec![ + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 14)), + severity: Some(lsp::DiagnosticSeverity::WARNING), + message: "unused variable: `unused`".to_string(), + ..Default::default() + }, + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(2, 4), lsp::Position::new(2, 22)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: "cannot find function `undefined_function` in this scope".to_string(), + ..Default::default() + } + ], + version: None + }, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap(); + }); + + let buffer_diagnostics = window.build_entity(cx, |window, cx| { + BufferDiagnosticsEditor::new( + project_path.clone(), + project.clone(), + buffer, + true, + window, + cx, + ) + }); + let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _| { + buffer_diagnostics.editor().clone() + }); + + // Since the excerpt updates is handled by a background task, we need to + // wait a little bit to ensure that the buffer diagnostic's editor content + // is rendered. + cx.executor() + .advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10)); + + pretty_assertions::assert_eq!( + editor_content_with_blocks(&editor, cx), + indoc::indoc! { + "§ main.rs + § ----- + fn main() { + let x = vec![]; + § move occurs because `x` has type `Vec`, which does not implement + § the `Copy` trait (back) + let y = vec![]; + § move occurs because `y` has type `Vec`, which does not implement + § the `Copy` trait + a(x); § value moved here + b(y); § value moved here + c(y); + § use of moved value + § value used here after move + d(x); + § use of moved value + § value used here after move + § hint: move occurs because `x` has type `Vec`, which does not + § implement the `Copy` trait + }" + } + ); +} + +#[gpui::test] +async fn test_buffer_diagnostics_without_warnings(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/test"), + json!({ + "main.rs": " + fn main() { + let x = vec![]; + let y = vec![]; + a(x); + b(y); + c(y); + d(x); + } + " + .unindent(), + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await; + let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let project_path = project::ProjectPath { + worktree_id: project.read_with(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }), + path: Arc::from(Path::new("main.rs")), + }; + let buffer = project + .update(cx, |project, cx| { + project.open_buffer(project_path.clone(), cx) + }) + .await + .ok(); + + let language_server_id = LanguageServerId(0); + let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap(); + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); + + lsp_store.update(cx, |lsp_store, cx| { + lsp_store.update_diagnostics(language_server_id, lsp::PublishDiagnosticsParams { + uri: uri.clone(), + diagnostics: vec![ + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)), + severity: Some(lsp::DiagnosticSeverity::WARNING), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: Some(vec![ + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 9))), + message: "move occurs because `y` has type `Vec`, which does not implement the `Copy` trait".to_string() + }, + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(4, 6), lsp::Position::new(4, 7))), + message: "value moved here".to_string() + }, + ]), + ..Default::default() + }, + lsp::Diagnostic{ + range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)), + severity: Some(lsp::DiagnosticSeverity::ERROR), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: Some(vec![ + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9))), + message: "move occurs because `x` has type `Vec`, which does not implement the `Copy` trait".to_string() + }, + lsp::DiagnosticRelatedInformation { + location: lsp::Location::new(uri.clone(), lsp::Range::new(lsp::Position::new(3, 6), lsp::Position::new(3, 7))), + message: "value moved here".to_string() + }, + ]), + ..Default::default() + } + ], + version: None + }, None, DiagnosticSourceKind::Pushed, &[], cx).unwrap(); + }); + + let include_warnings = false; + let buffer_diagnostics = window.build_entity(cx, |window, cx| { + BufferDiagnosticsEditor::new( + project_path.clone(), + project.clone(), + buffer, + include_warnings, + window, + cx, + ) + }); + + let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _cx| { + buffer_diagnostics.editor().clone() + }); + + // Since the excerpt updates is handled by a background task, we need to + // wait a little bit to ensure that the buffer diagnostic's editor content + // is rendered. + cx.executor() + .advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10)); + + pretty_assertions::assert_eq!( + editor_content_with_blocks(&editor, cx), + indoc::indoc! { + "§ main.rs + § ----- + fn main() { + let x = vec![]; + § move occurs because `x` has type `Vec`, which does not implement + § the `Copy` trait (back) + let y = vec![]; + a(x); § value moved here + b(y); + c(y); + d(x); + § use of moved value + § value used here after move + § hint: move occurs because `x` has type `Vec`, which does not + § implement the `Copy` trait + }" + } + ); +} + +#[gpui::test] +async fn test_buffer_diagnostics_multiple_servers(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/test"), + json!({ + "main.rs": " + fn main() { + let x = vec![]; + let y = vec![]; + a(x); + b(y); + c(y); + d(x); + } + " + .unindent(), + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/test").as_ref()], cx).await; + let window = cx.add_window(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let cx = &mut VisualTestContext::from_window(*window, cx); + let project_path = project::ProjectPath { + worktree_id: project.read_with(cx, |project, cx| { + project.worktrees(cx).next().unwrap().read(cx).id() + }), + path: Arc::from(Path::new("main.rs")), + }; + let buffer = project + .update(cx, |project, cx| { + project.open_buffer(project_path.clone(), cx) + }) + .await + .ok(); + + // Create the diagnostics for `main.rs`. + // Two warnings are being created, one for each language server, in order to + // assert that both warnings are rendered in the editor. + let language_server_id_a = LanguageServerId(0); + let language_server_id_b = LanguageServerId(1); + let uri = lsp::Url::from_file_path(path!("/test/main.rs")).unwrap(); + let lsp_store = project.read_with(cx, |project, _| project.lsp_store()); + + lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .update_diagnostics( + language_server_id_a, + lsp::PublishDiagnosticsParams { + uri: uri.clone(), + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(5, 6), lsp::Position::new(5, 7)), + severity: Some(lsp::DiagnosticSeverity::WARNING), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: None, + ..Default::default() + }], + version: None, + }, + None, + DiagnosticSourceKind::Pushed, + &[], + cx, + ) + .unwrap(); + + lsp_store + .update_diagnostics( + language_server_id_b, + lsp::PublishDiagnosticsParams { + uri: uri.clone(), + diagnostics: vec![lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(6, 6), lsp::Position::new(6, 7)), + severity: Some(lsp::DiagnosticSeverity::WARNING), + message: "use of moved value\nvalue used here after move".to_string(), + related_information: None, + ..Default::default() + }], + version: None, + }, + None, + DiagnosticSourceKind::Pushed, + &[], + cx, + ) + .unwrap(); + }); + + let buffer_diagnostics = window.build_entity(cx, |window, cx| { + BufferDiagnosticsEditor::new( + project_path.clone(), + project.clone(), + buffer, + true, + window, + cx, + ) + }); + let editor = buffer_diagnostics.update(cx, |buffer_diagnostics, _| { + buffer_diagnostics.editor().clone() + }); + + // Since the excerpt updates is handled by a background task, we need to + // wait a little bit to ensure that the buffer diagnostic's editor content + // is rendered. + cx.executor() + .advance_clock(DIAGNOSTICS_UPDATE_DELAY + Duration::from_millis(10)); + + pretty_assertions::assert_eq!( + editor_content_with_blocks(&editor, cx), + indoc::indoc! { + "§ main.rs + § ----- + a(x); + b(y); + c(y); + § use of moved value + § value used here after move + d(x); + § use of moved value + § value used here after move + }" + } + ); + + buffer_diagnostics.update(cx, |buffer_diagnostics, _cx| { + assert_eq!( + *buffer_diagnostics.summary(), + DiagnosticSummary { + warning_count: 2, + error_count: 0 + } + ); + }) +} + fn init_test(cx: &mut TestAppContext) { cx.update(|cx| { zlog::init_test(); diff --git a/crates/diagnostics/src/toolbar_controls.rs b/crates/diagnostics/src/toolbar_controls.rs index 0a2da959c2..bb44ba0786 100644 --- a/crates/diagnostics/src/toolbar_controls.rs +++ b/crates/diagnostics/src/toolbar_controls.rs @@ -40,11 +40,6 @@ impl Render for ToolbarControls { if let Some(editor) = editor.upgrade() { let diagnostics = editor.read(cx); include_warnings = diagnostics.include_warnings; - // TODO: How to calculate this for the - // `BufferDiagnosticsEditor`? Should we simply keep track if - // there are any updates to the diagnostics for the path and - // mark that instead of automatically updating? - has_stale_excerpts = false; is_updating = diagnostics.update_excerpts_task.is_some() || diagnostics .project diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 201dbac56d..b3f03bbb20 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -7123,14 +7123,28 @@ impl LspStore { project_path: &ProjectPath, _: &App, ) -> DiagnosticSummary { - // TODO: If there's multiple `DiagnosticSummary` but for - // different language servers, which one should be returned? - self.diagnostic_summaries + if let Some(summaries) = self + .diagnostic_summaries .get(&project_path.worktree_id) .and_then(|map| map.get(&project_path.path)) - .and_then(|summaries| summaries.iter().next()) - .map(|(_language_server_id, summary)| summary.clone()) - .unwrap_or_default() + { + let (error_count, warning_count) = summaries.iter().fold( + (0, 0), + |(error_count, warning_count), (_language_server_id, summary)| { + ( + error_count + summary.error_count, + warning_count + summary.warning_count, + ) + }, + ); + + DiagnosticSummary { + error_count, + warning_count, + } + } else { + DiagnosticSummary::default() + } } pub fn diagnostic_summaries<'a>( From 67b3f80003a8e745b6508aa6920c7906ad85fb20 Mon Sep 17 00:00:00 2001 From: dinocosta Date: Mon, 25 Aug 2025 16:29:24 +0100 Subject: [PATCH 3/4] refactor: introduce diagnostics toolbar editor trait This commit refactors the interaction between the diagnostics editors (`BufferDiagnosticsEditor` and `ProjectDiagnosticsEditor`) so that, instead of passing an optional enum to the `ToolbarControls` struct, as well as the `DiagnosticBlock`, we pass a reference to a type that implements a new trait, `DiagnosticsToolbarEditor`, which defines the methods that the toolbar controls needs, namely: - `include_warnings` - `toggle_warnings` - `has_stale_excerpts` - `is_updating` - `stop_updating` - `refresh_diagnostics` - `get_diagnostics_for_buffer` This makes it so that both `ToolbarControls` and `DiagnosticBlock` implementations don't need to be aware of the internal details of each of the editors, as well as avoiding duplicating the `DiagnosticsEditorHandle` enum. --- crates/diagnostics/src/buffer_diagnostics.rs | 67 +++++++- crates/diagnostics/src/diagnostic_renderer.rs | 43 +---- crates/diagnostics/src/diagnostics.rs | 67 +++++++- crates/diagnostics/src/toolbar_controls.rs | 148 +++++------------- 4 files changed, 170 insertions(+), 155 deletions(-) diff --git a/crates/diagnostics/src/buffer_diagnostics.rs b/crates/diagnostics/src/buffer_diagnostics.rs index 753e8ed5d8..9cc3056085 100644 --- a/crates/diagnostics/src/buffer_diagnostics.rs +++ b/crates/diagnostics/src/buffer_diagnostics.rs @@ -1,6 +1,7 @@ use crate::{ DIAGNOSTICS_UPDATE_DELAY, IncludeWarnings, ToggleWarnings, context_range_for_entry, - diagnostic_renderer::{DiagnosticBlock, DiagnosticRenderer, DiagnosticsEditorHandle}, + diagnostic_renderer::{DiagnosticBlock, DiagnosticRenderer}, + toolbar_controls::DiagnosticsToolbarEditor, }; use anyhow::Result; use collections::HashMap; @@ -11,7 +12,7 @@ use editor::{ use gpui::{ AnyElement, App, AppContext, Context, Entity, EntityId, EventEmitter, FocusHandle, Focusable, InteractiveElement, IntoElement, ParentElement, Render, SharedString, Styled, Subscription, - Task, Window, actions, div, + Task, WeakEntity, Window, actions, div, }; use language::{Buffer, DiagnosticEntry, Point}; use project::{ @@ -246,7 +247,7 @@ impl BufferDiagnosticsEditor { ) }); - workspace.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx); + workspace.add_item_to_active_pane(Box::new(item), None, true, window, cx); } } } @@ -372,9 +373,7 @@ impl BufferDiagnosticsEditor { DiagnosticRenderer::diagnostic_blocks_for_group( group, buffer_snapshot.remote_id(), - Some(DiagnosticsEditorHandle::Buffer( - buffer_diagnostics_editor.clone(), - )), + Some(Arc::new(buffer_diagnostics_editor.clone())), cx, ) })?; @@ -917,3 +916,59 @@ impl Render for BufferDiagnosticsEditor { .child(child) } } + +impl DiagnosticsToolbarEditor for WeakEntity { + fn include_warnings(&self, cx: &App) -> bool { + self.read_with(cx, |buffer_diagnostics_editor, _cx| { + buffer_diagnostics_editor.include_warnings + }) + .unwrap_or(false) + } + + fn has_stale_excerpts(&self, _cx: &App) -> bool { + false + } + + fn is_updating(&self, cx: &App) -> bool { + self.read_with(cx, |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.update_excerpts_task.is_some() + || buffer_diagnostics_editor + .project + .read(cx) + .language_servers_running_disk_based_diagnostics(cx) + .next() + .is_some() + }) + .unwrap_or(false) + } + + fn stop_updating(&self, cx: &mut App) { + let _ = self.update(cx, |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.update_excerpts_task = None; + cx.notify(); + }); + } + + fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App) { + let _ = self.update(cx, |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.update_all_excerpts(window, cx); + }); + } + + fn toggle_warnings(&self, window: &mut Window, cx: &mut App) { + let _ = self.update(cx, |buffer_diagnostics_editor, cx| { + buffer_diagnostics_editor.toggle_warnings(&Default::default(), window, cx); + }); + } + + fn get_diagnostics_for_buffer( + &self, + _buffer_id: text::BufferId, + cx: &App, + ) -> Vec> { + self.read_with(cx, |buffer_diagnostics_editor, _cx| { + buffer_diagnostics_editor.diagnostics.clone() + }) + .unwrap_or_default() + } +} diff --git a/crates/diagnostics/src/diagnostic_renderer.rs b/crates/diagnostics/src/diagnostic_renderer.rs index 4456775e92..e22065afa5 100644 --- a/crates/diagnostics/src/diagnostic_renderer.rs +++ b/crates/diagnostics/src/diagnostic_renderer.rs @@ -18,42 +18,7 @@ use ui::{ }; use util::maybe; -use crate::ProjectDiagnosticsEditor; -use crate::buffer_diagnostics::BufferDiagnosticsEditor; - -#[derive(Clone)] -pub(crate) enum DiagnosticsEditorHandle { - Project(WeakEntity), - Buffer(WeakEntity), -} - -impl DiagnosticsEditorHandle { - fn get_diagnostics_for_buffer( - &self, - buffer_id: BufferId, - cx: &App, - ) -> Vec> { - match self { - // Not sure if possible, as currently there shouldn't be a way for this - // method to be called for a buffer other than the one the - // `BufferDiagnosticsEditor` is working with, but we should probably - // save the ID of the buffer that it is working with, so that, if it - // doesn't match the argument, we can return an empty vector. - Self::Buffer(editor) => editor - .read_with(cx, |editor, _cx| editor.diagnostics.clone()) - .unwrap_or(vec![]), - Self::Project(editor) => editor - .read_with(cx, |editor, _cx| { - editor - .diagnostics - .get(&buffer_id) - .cloned() - .unwrap_or_default() - }) - .unwrap_or(vec![]), - } - } -} +use crate::toolbar_controls::DiagnosticsToolbarEditor; pub struct DiagnosticRenderer; @@ -61,7 +26,7 @@ impl DiagnosticRenderer { pub fn diagnostic_blocks_for_group( diagnostic_group: Vec>, buffer_id: BufferId, - diagnostics_editor: Option, + diagnostics_editor: Option>, cx: &mut App, ) -> Vec { let Some(primary_ix) = diagnostic_group @@ -218,7 +183,7 @@ pub(crate) struct DiagnosticBlock { pub(crate) initial_range: Range, pub(crate) severity: DiagnosticSeverity, pub(crate) markdown: Entity, - pub(crate) diagnostics_editor: Option, + pub(crate) diagnostics_editor: Option>, } impl DiagnosticBlock { @@ -269,7 +234,7 @@ impl DiagnosticBlock { pub fn open_link( editor: &mut Editor, - diagnostics_editor: &Option, + diagnostics_editor: &Option>, link: SharedString, window: &mut Window, cx: &mut Context, diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 817679b782..6e4cce376c 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -37,6 +37,7 @@ use std::{ }; use text::{BufferId, OffsetRangeExt}; use theme::ActiveTheme; +use toolbar_controls::DiagnosticsToolbarEditor; pub use toolbar_controls::ToolbarControls; use ui::{Icon, IconName, Label, h_flex, prelude::*}; use util::ResultExt; @@ -46,8 +47,6 @@ use workspace::{ searchable::SearchableItemHandle, }; -use crate::diagnostic_renderer::DiagnosticsEditorHandle; - actions!( diagnostics, [ @@ -483,7 +482,7 @@ impl ProjectDiagnosticsEditor { crate::diagnostic_renderer::DiagnosticRenderer::diagnostic_blocks_for_group( group, buffer_snapshot.remote_id(), - Some(DiagnosticsEditorHandle::Project(this.clone())), + Some(Arc::new(this.clone())), cx, ) })?; @@ -825,6 +824,68 @@ impl Item for ProjectDiagnosticsEditor { } } +impl DiagnosticsToolbarEditor for WeakEntity { + fn include_warnings(&self, cx: &App) -> bool { + self.read_with(cx, |project_diagnostics_editor, _cx| { + project_diagnostics_editor.include_warnings + }) + .unwrap_or(false) + } + + fn has_stale_excerpts(&self, cx: &App) -> bool { + self.read_with(cx, |project_diagnostics_editor, _cx| { + !project_diagnostics_editor.paths_to_update.is_empty() + }) + .unwrap_or(false) + } + + fn is_updating(&self, cx: &App) -> bool { + self.read_with(cx, |project_diagnostics_editor, cx| { + project_diagnostics_editor.update_excerpts_task.is_some() + || project_diagnostics_editor + .project + .read(cx) + .language_servers_running_disk_based_diagnostics(cx) + .next() + .is_some() + }) + .unwrap_or(false) + } + + fn stop_updating(&self, cx: &mut App) { + let _ = self.update(cx, |project_diagnostics_editor, cx| { + project_diagnostics_editor.update_excerpts_task = None; + cx.notify(); + }); + } + + fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App) { + let _ = self.update(cx, |project_diagnostics_editor, cx| { + project_diagnostics_editor.update_all_excerpts(window, cx); + }); + } + + fn toggle_warnings(&self, window: &mut Window, cx: &mut App) { + let _ = self.update(cx, |project_diagnostics_editor, cx| { + project_diagnostics_editor.toggle_warnings(&Default::default(), window, cx); + }); + } + + fn get_diagnostics_for_buffer( + &self, + buffer_id: text::BufferId, + cx: &App, + ) -> Vec> { + self.read_with(cx, |project_diagnostics_editor, _cx| { + project_diagnostics_editor + .diagnostics + .get(&buffer_id) + .cloned() + .unwrap_or_default() + }) + .unwrap_or_default() + } +} const DIAGNOSTIC_EXPANSION_ROW_LIMIT: u32 = 32; async fn context_range_for_entry( diff --git a/crates/diagnostics/src/toolbar_controls.rs b/crates/diagnostics/src/toolbar_controls.rs index bb44ba0786..ac7bfb0f69 100644 --- a/crates/diagnostics/src/toolbar_controls.rs +++ b/crates/diagnostics/src/toolbar_controls.rs @@ -1,20 +1,40 @@ use crate::{BufferDiagnosticsEditor, ProjectDiagnosticsEditor, ToggleDiagnosticsRefresh}; -use gpui::{Context, EventEmitter, ParentElement, Render, WeakEntity, Window}; +use gpui::{Context, EventEmitter, ParentElement, Render, Window}; +use language::DiagnosticEntry; +use text::{Anchor, BufferId}; use ui::prelude::*; use ui::{IconButton, IconButtonShape, IconName, Tooltip}; use workspace::{ToolbarItemEvent, ToolbarItemLocation, ToolbarItemView, item::ItemHandle}; pub struct ToolbarControls { - editor: Option, + editor: Option>, } -enum DiagnosticsEditorHandle { - Project(WeakEntity), - Buffer(WeakEntity), +pub(crate) trait DiagnosticsToolbarEditor: Send + Sync { + /// Informs the toolbar whether warnings are included in the diagnostics. + fn include_warnings(&self, cx: &App) -> bool; + /// Toggles whether warning diagnostics should be displayed by the + /// diagnostics editor. + fn toggle_warnings(&self, window: &mut Window, cx: &mut App); + /// Indicates whether any of the excerpts displayed by the diagnostics + /// editor are stale. + fn has_stale_excerpts(&self, cx: &App) -> bool; + /// Indicates whether the diagnostics editor is currently updating the + /// diagnostics. + fn is_updating(&self, cx: &App) -> bool; + /// Requests that the diagnostics editor stop updating the diagnostics. + fn stop_updating(&self, cx: &mut App); + /// Requests that the diagnostics editor updates the displayed diagnostics + /// with the latest information. + fn refresh_diagnostics(&self, window: &mut Window, cx: &mut App); + /// Returns a list of diagnostics for the provided buffer id. + fn get_diagnostics_for_buffer( + &self, + buffer_id: BufferId, + cx: &App, + ) -> Vec>; } -impl DiagnosticsEditorHandle {} - impl Render for ToolbarControls { fn render(&mut self, _: &mut Window, cx: &mut Context) -> impl IntoElement { let mut has_stale_excerpts = false; @@ -22,32 +42,10 @@ impl Render for ToolbarControls { let mut is_updating = false; match &self.editor { - Some(DiagnosticsEditorHandle::Project(editor)) => { - if let Some(editor) = editor.upgrade() { - let diagnostics = editor.read(cx); - include_warnings = diagnostics.include_warnings; - has_stale_excerpts = !diagnostics.paths_to_update.is_empty(); - is_updating = diagnostics.update_excerpts_task.is_some() - || diagnostics - .project - .read(cx) - .language_servers_running_disk_based_diagnostics(cx) - .next() - .is_some(); - } - } - Some(DiagnosticsEditorHandle::Buffer(editor)) => { - if let Some(editor) = editor.upgrade() { - let diagnostics = editor.read(cx); - include_warnings = diagnostics.include_warnings; - is_updating = diagnostics.update_excerpts_task.is_some() - || diagnostics - .project - .read(cx) - .language_servers_running_disk_based_diagnostics(cx) - .next() - .is_some(); - } + Some(editor) => { + include_warnings = editor.include_warnings(cx); + has_stale_excerpts = editor.has_stale_excerpts(cx); + is_updating = editor.is_updating(cx); } None => {} } @@ -78,29 +76,9 @@ impl Render for ToolbarControls { )) .on_click(cx.listener(move |toolbar_controls, _, _, cx| { match toolbar_controls.editor() { - Some(DiagnosticsEditorHandle::Buffer( - buffer_diagnostics_editor, - )) => { - let _ = buffer_diagnostics_editor.update( - cx, - |buffer_diagnostics_editor, cx| { - buffer_diagnostics_editor.update_excerpts_task = - None; - cx.notify(); - }, - ); - } - Some(DiagnosticsEditorHandle::Project( - project_diagnostics_editor, - )) => { - let _ = project_diagnostics_editor.update( - cx, - |project_diagnostics_editor, cx| { - project_diagnostics_editor.update_excerpts_task = - None; - cx.notify(); - }, - ); + Some(editor) => { + editor.stop_updating(cx); + cx.notify(); } None => {} } @@ -120,28 +98,7 @@ impl Render for ToolbarControls { move |toolbar_controls, _, window, cx| match toolbar_controls .editor() { - Some(DiagnosticsEditorHandle::Buffer( - buffer_diagnostics_editor, - )) => { - let _ = buffer_diagnostics_editor.update( - cx, - |buffer_diagnostics_editor, cx| { - buffer_diagnostics_editor - .update_all_excerpts(window, cx); - }, - ); - } - Some(DiagnosticsEditorHandle::Project( - project_diagnostics_editor, - )) => { - let _ = project_diagnostics_editor.update( - cx, - |project_diagnostics_editor, cx| { - project_diagnostics_editor - .update_all_excerpts(window, cx); - }, - ); - } + Some(editor) => editor.refresh_diagnostics(window, cx), None => {} } })), @@ -154,31 +111,8 @@ impl Render for ToolbarControls { .shape(IconButtonShape::Square) .tooltip(Tooltip::text(warning_tooltip)) .on_click(cx.listener(|this, _, window, cx| match &this.editor { - Some(DiagnosticsEditorHandle::Project(project_diagnostics_editor)) => { - let _ = project_diagnostics_editor.update( - cx, - |project_diagnostics_editor, cx| { - project_diagnostics_editor.toggle_warnings( - &Default::default(), - window, - cx, - ); - }, - ); - } - Some(DiagnosticsEditorHandle::Buffer(buffer_diagnostics_editor)) => { - let _ = buffer_diagnostics_editor.update( - cx, - |buffer_diagnostics_editor, cx| { - buffer_diagnostics_editor.toggle_warnings( - &Default::default(), - window, - cx, - ); - }, - ); - } - _ => {} + Some(editor) => editor.toggle_warnings(window, cx), + None => {} })), ) } @@ -195,10 +129,10 @@ impl ToolbarItemView for ToolbarControls { ) -> ToolbarItemLocation { if let Some(pane_item) = active_pane_item.as_ref() { if let Some(editor) = pane_item.downcast::() { - self.editor = Some(DiagnosticsEditorHandle::Project(editor.downgrade())); + self.editor = Some(Box::new(editor.downgrade())); ToolbarItemLocation::PrimaryRight } else if let Some(editor) = pane_item.downcast::() { - self.editor = Some(DiagnosticsEditorHandle::Buffer(editor.downgrade())); + self.editor = Some(Box::new(editor.downgrade())); ToolbarItemLocation::PrimaryRight } else { ToolbarItemLocation::Hidden @@ -220,7 +154,7 @@ impl ToolbarControls { ToolbarControls { editor: None } } - fn editor(&self) -> Option<&DiagnosticsEditorHandle> { - self.editor.as_ref() + fn editor(&self) -> Option<&dyn DiagnosticsToolbarEditor> { + self.editor.as_deref() } } From b6600d07666ae38a090acdc604720d882612bab5 Mon Sep 17 00:00:00 2001 From: dinocosta Date: Tue, 26 Aug 2025 17:33:00 +0100 Subject: [PATCH 4/4] chore: use only filename in tab content Co-authored-by: Conrad Irwin --- crates/diagnostics/src/buffer_diagnostics.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/crates/diagnostics/src/buffer_diagnostics.rs b/crates/diagnostics/src/buffer_diagnostics.rs index 9cc3056085..4e71dc1796 100644 --- a/crates/diagnostics/src/buffer_diagnostics.rs +++ b/crates/diagnostics/src/buffer_diagnostics.rs @@ -781,14 +781,20 @@ impl Item for BufferDiagnosticsEditor { } // Builds the content to be displayed in the tab. - fn tab_content(&self, params: TabContentParams, _window: &Window, _app: &App) -> AnyElement { + fn tab_content(&self, params: TabContentParams, _window: &Window, _cx: &App) -> AnyElement { let error_count = self.summary.error_count; let warning_count = self.summary.warning_count; - let label = Label::new(self.project_path.path.to_sanitized_string()); + let label = Label::new( + self.project_path + .path + .file_name() + .map(|f| f.to_sanitized_string()) + .unwrap_or_else(|| self.project_path.path.to_sanitized_string()), + ); h_flex() .gap_1() - .child(label.color(params.text_color())) + .child(label) .when(error_count == 0 && warning_count == 0, |parent| { parent.child( h_flex()