diff --git a/crates/diagnostics/src/diagnostics_tests.rs b/crates/diagnostics/src/diagnostics_tests.rs index 6352532484..613b485c94 100644 --- a/crates/diagnostics/src/diagnostics_tests.rs +++ b/crates/diagnostics/src/diagnostics_tests.rs @@ -1550,6 +1550,8 @@ async fn go_to_diagnostic_with_severity(cx: &mut TestAppContext) { // Default, should cycle through all diagnostics go!(GoToDiagnosticSeverityFilter::default()); + cx.assert_editor_state(indoc! {"error warning info ˇhint"}); + go!(GoToDiagnosticSeverityFilter::default()); cx.assert_editor_state(indoc! {"ˇerror warning info hint"}); go!(GoToDiagnosticSeverityFilter::default()); cx.assert_editor_state(indoc! {"error ˇwarning info hint"}); diff --git a/crates/diagnostics/src/items.rs b/crates/diagnostics/src/items.rs index 7733dab820..9f243c7810 100644 --- a/crates/diagnostics/src/items.rs +++ b/crates/diagnostics/src/items.rs @@ -64,13 +64,19 @@ impl Render for DiagnosticIndicator { .message .split_once('\n') .map_or(&*diagnostic.message, |(first, _)| first); + let diagnostics_already_active = self.any_active_diagnostics(cx); + let tooltip = if !diagnostics_already_active { + "Expand Diagnostics" + } else { + "Next Diagnostic" + }; Some( Button::new("diagnostic_message", SharedString::new(message)) .label_size(LabelSize::Small) .truncate(true) - .tooltip(|_window, cx| { + .tooltip(move |_window, cx| { Tooltip::for_action( - "Next Diagnostic", + tooltip, &editor::actions::GoToDiagnostic::default(), cx, ) @@ -154,10 +160,18 @@ impl DiagnosticIndicator { } } + fn any_active_diagnostics(&self, cx: &mut Context) -> bool { + if let Some(editor) = self.active_editor.as_ref().and_then(|e| e.upgrade()) { + editor.read(cx).any_active_diagnostics() + } else { + false + } + } + fn go_to_next_diagnostic(&mut self, window: &mut Window, cx: &mut Context) { if let Some(editor) = self.active_editor.as_ref().and_then(|e| e.upgrade()) { editor.update(cx, |editor, cx| { - editor.go_to_diagnostic_impl( + editor.go_to_diagnostic_at_cursor( editor::Direction::Next, GoToDiagnosticSeverityFilter::default(), window, diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 03557c029f..827c9067e3 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -323,7 +323,8 @@ pub struct SplitSelectionIntoLines { pub keep_selections: bool, } -/// Goes to the next diagnostic in the file. +/// Expands the diagnostic under the cursor, if any, in case diagnostics are not +/// yet active. Otherwise, goes to the next diagnostic in the file. #[derive(PartialEq, Clone, Default, Debug, Deserialize, JsonSchema, Action)] #[action(namespace = editor)] #[serde(deny_unknown_fields)] @@ -332,7 +333,8 @@ pub struct GoToDiagnostic { pub severity: GoToDiagnosticSeverityFilter, } -/// Goes to the previous diagnostic in the file. +/// Expands the diagnostic under the cursor, if any, in case diagnostics are not +/// yet active. Otherwise, goes to the previous diagnostic in the file. #[derive(PartialEq, Clone, Default, Debug, Deserialize, JsonSchema, Action)] #[action(namespace = editor)] #[serde(deny_unknown_fields)] diff --git a/crates/editor/src/diagnostics.rs b/crates/editor/src/diagnostics.rs index b13b4b699f..cdbb341a41 100644 --- a/crates/editor/src/diagnostics.rs +++ b/crates/editor/src/diagnostics.rs @@ -77,7 +77,8 @@ impl Editor { if !self.diagnostics_enabled() { return; } - self.go_to_diagnostic_impl(Direction::Next, action.severity, window, cx) + + self.go_to_diagnostic_at_cursor(Direction::Next, action.severity, window, cx); } pub fn go_to_prev_diagnostic( @@ -89,10 +90,43 @@ impl Editor { if !self.diagnostics_enabled() { return; } - self.go_to_diagnostic_impl(Direction::Prev, action.severity, window, cx) + + self.go_to_diagnostic_at_cursor(Direction::Prev, action.severity, window, cx); } - pub fn go_to_diagnostic_impl( + fn diagnostics_before_cursor<'a>( + buffer: &'a MultiBufferSnapshot, + cursor: MultiBufferOffset, + severity: GoToDiagnosticSeverityFilter, + ) -> impl Iterator> { + buffer + .diagnostics_in_range(MultiBufferOffset(0)..cursor) + .filter(move |entry| entry.range.start <= cursor) + .filter(move |entry| severity.matches(entry.diagnostic.severity)) + .filter(|entry| entry.range.start != entry.range.end) + .filter(|entry| !entry.diagnostic.is_unnecessary) + } + + fn diagnostics_after_cursor<'a>( + buffer: &'a MultiBufferSnapshot, + cursor: MultiBufferOffset, + severity: GoToDiagnosticSeverityFilter, + ) -> impl Iterator> { + buffer + .diagnostics_in_range(cursor..buffer.len()) + .filter(move |entry| entry.range.start >= cursor) + .filter(move |entry| severity.matches(entry.diagnostic.severity)) + .filter(|entry| entry.range.start != entry.range.end) + .filter(|entry| !entry.diagnostic.is_unnecessary) + } + + /// Attempts to expand the diagnostic at the current cursor position, + /// updating the cursor position to the diagnostic's start point. + /// + /// In case there's no diagnostic at the current cursor position, this will + /// fallback to finding the next or previous diagnostic instead, depending + /// on the provided `direction`. + pub fn go_to_diagnostic_at_cursor( &mut self, direction: Direction, severity: GoToDiagnosticSeverityFilter, @@ -104,6 +138,71 @@ impl Editor { .selections .newest::(&self.display_snapshot(cx)); + let before = Self::diagnostics_before_cursor(&buffer, selection.start, severity); + let after = Self::diagnostics_after_cursor(&buffer, selection.start, severity); + let active_group_id = match &self.active_diagnostics { + ActiveDiagnostic::Group(group) => Some(group.group_id), + _ => None, + }; + + let mut cursor_on_active = false; + let mut target = None; + + for diagnostic in after.chain(before) { + let contains_cursor = diagnostic.range.contains(&selection.start) + || diagnostic.range.end == selection.head(); + + if !contains_cursor { + continue; + } + + if active_group_id == Some(diagnostic.diagnostic.group_id) { + cursor_on_active = true; + } else if target.is_none() { + target = Some(diagnostic); + } + } + + match (target, cursor_on_active) { + (Some(diagnostic), false) => self.activate_diagnostic(&buffer, diagnostic, window, cx), + _ => self.go_to_diagnostic_in_direction( + &buffer, &selection, direction, severity, window, cx, + ), + } + } + + fn activate_diagnostic( + &mut self, + buffer: &MultiBufferSnapshot, + diagnostic: DiagnosticEntryRef, + window: &mut Window, + cx: &mut Context, + ) { + let diagnostic_start = buffer.anchor_after(diagnostic.range.start); + let Some((buffer_anchor, _)) = buffer.anchor_to_buffer_anchor(diagnostic_start) else { + return; + }; + let buffer_id = buffer_anchor.buffer_id; + let snapshot = self.snapshot(window, cx); + if snapshot.intersects_fold(diagnostic.range.start) { + self.unfold_ranges(std::slice::from_ref(&diagnostic.range), true, false, cx); + } + self.change_selections(Default::default(), window, cx, |s| { + s.select_ranges(vec![diagnostic.range.start..diagnostic.range.start]) + }); + self.activate_diagnostics(buffer_id, diagnostic, window, cx); + self.refresh_edit_prediction(false, true, window, cx); + } + + pub fn go_to_diagnostic_in_direction( + &mut self, + buffer: &MultiBufferSnapshot, + selection: &Selection, + direction: Direction, + severity: GoToDiagnosticSeverityFilter, + window: &mut Window, + cx: &mut Context, + ) { let mut active_group_id = None; if let ActiveDiagnostic::Group(active_group) = &self.active_diagnostics && active_group.active_range.start.to_offset(&buffer) == selection.start @@ -111,28 +210,8 @@ impl Editor { active_group_id = Some(active_group.group_id); } - fn filtered<'a>( - severity: GoToDiagnosticSeverityFilter, - diagnostics: impl Iterator>, - ) -> impl Iterator> { - diagnostics - .filter(move |entry| severity.matches(entry.diagnostic.severity)) - .filter(|entry| entry.range.start != entry.range.end) - .filter(|entry| !entry.diagnostic.is_unnecessary) - } - - let before = filtered( - severity, - buffer - .diagnostics_in_range(MultiBufferOffset(0)..selection.start) - .filter(|entry| entry.range.start <= selection.start), - ); - let after = filtered( - severity, - buffer - .diagnostics_in_range(selection.start..buffer.len()) - .filter(|entry| entry.range.start >= selection.start), - ); + let before = Self::diagnostics_before_cursor(&buffer, selection.start, severity); + let after = Self::diagnostics_after_cursor(&buffer, selection.start, severity); let mut found: Option> = None; if direction == Direction::Prev { @@ -158,31 +237,12 @@ impl Editor { } } } + let Some(next_diagnostic) = found else { return; }; - let next_diagnostic_start = buffer.anchor_after(next_diagnostic.range.start); - let Some((buffer_anchor, _)) = buffer.anchor_to_buffer_anchor(next_diagnostic_start) else { - return; - }; - let buffer_id = buffer_anchor.buffer_id; - let snapshot = self.snapshot(window, cx); - if snapshot.intersects_fold(next_diagnostic.range.start) { - self.unfold_ranges( - std::slice::from_ref(&next_diagnostic.range), - true, - false, - cx, - ); - } - self.change_selections(Default::default(), window, cx, |s| { - s.select_ranges(vec![ - next_diagnostic.range.start..next_diagnostic.range.start, - ]) - }); - self.activate_diagnostics(buffer_id, next_diagnostic, window, cx); - self.refresh_edit_prediction(false, true, window, cx); + self.activate_diagnostic(&buffer, next_diagnostic, window, cx); } #[cfg(any(test, feature = "test-support"))] @@ -324,32 +384,37 @@ impl Editor { return; } self.dismiss_diagnostics(cx); - let snapshot = self.snapshot(window, cx); let buffer = self.buffer.read(cx).snapshot(cx); - let Some(renderer) = GlobalDiagnosticRenderer::global(cx) else { - return; + + let blocks = if let Some(renderer) = GlobalDiagnosticRenderer::global(cx) { + let snapshot = self.snapshot(window, cx); + let diagnostic_group = buffer + .diagnostic_group(buffer_id, diagnostic.diagnostic.group_id) + .collect::>(); + + let language_registry = self + .project() + .map(|project| project.read(cx).languages().clone()); + + let blocks = renderer.render_group( + diagnostic_group, + buffer_id, + snapshot, + cx.weak_entity(), + language_registry, + cx, + ); + + self.display_map.update(cx, |display_map, cx| { + display_map.insert_blocks(blocks, cx).into_iter().collect() + }) + } else { + // Ensure that, even if there's no global renderer set, we still use + // an empty set of blocks, such that we can record the active group + // below instead of bailing out. + HashSet::default() }; - let diagnostic_group = buffer - .diagnostic_group(buffer_id, diagnostic.diagnostic.group_id) - .collect::>(); - - let language_registry = self - .project() - .map(|project| project.read(cx).languages().clone()); - - let blocks = renderer.render_group( - diagnostic_group, - buffer_id, - snapshot, - cx.weak_entity(), - language_registry, - cx, - ); - - let blocks = self.display_map.update(cx, |display_map, cx| { - display_map.insert_blocks(blocks, cx).into_iter().collect() - }); self.active_diagnostics = ActiveDiagnostic::Group(ActiveDiagnosticGroup { active_range: buffer.anchor_before(diagnostic.range.start) ..buffer.anchor_after(diagnostic.range.end), @@ -516,4 +581,12 @@ impl Editor { self.scrollbar_marker_state.dirty = true; cx.notify(); } + + pub fn any_active_diagnostics(&self) -> bool { + match &self.active_diagnostics { + ActiveDiagnostic::None => false, + ActiveDiagnostic::All => true, + ActiveDiagnostic::Group(_) => true, + } + } } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 3b82a9580c..3eca954d6d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -20506,6 +20506,130 @@ async fn go_to_prev_overlapping_diagnostic(executor: BackgroundExecutor, cx: &mu "}); } +#[gpui::test] +async fn go_to_diagnostic(executor: BackgroundExecutor, cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + let mut cx = EditorTestContext::new(cx).await; + let lsp_store = + cx.update_editor(|editor, _, cx| editor.project().unwrap().read(cx).lsp_store()); + + // Place the cursor inside the `def` diagnostic (`[12, 15)`) before any + // diagnostic is active so we can later confirm that running `editor: go to + // diagnostic` will activate this diagnostic instead of advancing to the + // next one. + cx.set_state(indoc! {" + fn func(abc dˇef: i32) -> u32 { + } + "}); + + // Set up the diagnostics: + // + // * `[11, 12)` (the space before `def`), + // * `[12, 15)` (`def`), + // * `[25, 28)` (`u32`). + cx.update(|_, cx| { + lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .update_diagnostics( + LanguageServerId(0), + lsp::PublishDiagnosticsParams { + uri: lsp::Uri::from_file_path(path!("/root/file")).unwrap(), + version: None, + diagnostics: vec![ + lsp::Diagnostic { + range: lsp::Range::new( + lsp::Position::new(0, 11), + lsp::Position::new(0, 12), + ), + severity: Some(lsp::DiagnosticSeverity::ERROR), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new( + lsp::Position::new(0, 12), + lsp::Position::new(0, 15), + ), + severity: Some(lsp::DiagnosticSeverity::ERROR), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new( + lsp::Position::new(0, 25), + lsp::Position::new(0, 28), + ), + severity: Some(lsp::DiagnosticSeverity::ERROR), + ..Default::default() + }, + ], + }, + None, + DiagnosticSourceKind::Pushed, + &[], + cx, + ) + .unwrap() + }); + }); + + executor.run_until_parked(); + + // When the cursor is at an inactive diagnostic, cursor should be moved to + // the start of that same diagnostic and activate it. + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx); + }); + cx.assert_editor_state(indoc! {" + fn func(abc ˇdef: i32) -> u32 { + } + "}); + + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx); + }); + cx.assert_editor_state(indoc! {" + fn func(abc def: i32) -> ˇu32 { + } + "}); + + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx); + }); + cx.assert_editor_state(indoc! {" + fn func(abcˇ def: i32) -> u32 { + } + "}); + + // Manually move the cursor to a different, not yet active diagnostic to + // confirm that using `editor: go to diagnostic` will now activate this one. + cx.update_editor(|editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.select_ranges([Point::new(0, 26)..Point::new(0, 26)]) + }); + }); + + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx); + }); + cx.assert_editor_state(indoc! {" + fn func(abc def: i32) -> ˇu32 { + } + "}); + + cx.update_editor(|editor, window, cx| { + editor.change_selections(Default::default(), window, cx, |s| { + s.select_ranges([Point::new(0, 0)..Point::new(0, 0)]) + }); + }); + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic::default(), window, cx); + }); + cx.assert_editor_state(indoc! {" + fn func(abcˇ def: i32) -> u32 { + } + "}); +} + #[gpui::test] async fn test_go_to_hunk(executor: BackgroundExecutor, cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/docs/src/reference/all-settings.md b/docs/src/reference/all-settings.md index e5b7d7d2de..b103b8f61f 100644 --- a/docs/src/reference/all-settings.md +++ b/docs/src/reference/all-settings.md @@ -2098,6 +2098,7 @@ To interpret all `.c` files as C++, files called `MyLockFile` as TOML and files ```json [settings] { "diagnostics": { + "button": true, "include_warnings": true, "inline": { "enabled": false @@ -2106,6 +2107,11 @@ To interpret all `.c` files as C++, files called `MyLockFile` as TOML and files } ``` +**Options** + +- `button`: Whether to show the project diagnostics button in the status bar +- `include_warnings`: Whether to show warnings or not by default + ### Inline Diagnostics - Description: Whether or not to show diagnostics information inline.