From d2b49de0e4f575c4b3e56b6208f465864e55d3fd Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 26 Feb 2025 22:30:23 +0200 Subject: [PATCH] Dismiss active diagnostics on invalidation (#25646) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When migrating to gpui2, https://github.com/zed-industries/zed/commit/588976d27a22bb36a5aac7e762b21e180a66a2f9#diff-a3da3181e4ab4f73aa1697d7b6dc0caa0c17b2a187fb83b076dfc0234ec91f54R21 removed the diagnostic style for "active but invalid" case: presumably, it served as some sort of a cursor to show where to move on after the diagnostics update, on the next `GoTo[Prev]Diagnostic` action call. As this change went unchanged for some time, another approach is tested now, to be more integrated with inline diagnostics: now, the active state is cleared Same as before this change, another `GoTo[Prev]Diagnostic` action call will be needed to re-expand a new diagnostics, but this change makes this expansion to happen after the cursor — before the change, Zed would continue from the stale diagnostics. Release Notes: - Fixed active diagnostics becoming stale --- crates/diagnostics/src/diagnostics.rs | 4 +- crates/editor/src/editor.rs | 27 +++---- crates/editor/src/editor_tests.rs | 100 ++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 15 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 76606f54a7..42b1800fe7 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -562,9 +562,7 @@ impl ProjectDiagnosticsEditor { )), height: diagnostic.message.matches('\n').count() as u32 + 1, style: BlockStyle::Fixed, - render: diagnostic_block_renderer( - diagnostic, None, true, true, - ), + render: diagnostic_block_renderer(diagnostic, None, true), priority: 0, }); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8f9ea78e06..04d5db9bf3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -994,7 +994,7 @@ struct RegisteredInlineCompletionProvider { _subscription: Subscription, } -#[derive(Debug)] +#[derive(Debug, PartialEq, Eq)] struct ActiveDiagnosticGroup { primary_range: Range, primary_message: String, @@ -12539,16 +12539,20 @@ impl Editor { if is_valid != active_diagnostics.is_valid { active_diagnostics.is_valid = is_valid; - let mut new_styles = HashMap::default(); - for (block_id, diagnostic) in &active_diagnostics.blocks { - new_styles.insert( - *block_id, - diagnostic_block_renderer(diagnostic.clone(), None, true, is_valid), - ); + if is_valid { + let mut new_styles = HashMap::default(); + for (block_id, diagnostic) in &active_diagnostics.blocks { + new_styles.insert( + *block_id, + diagnostic_block_renderer(diagnostic.clone(), None, true), + ); + } + self.display_map.update(cx, |display_map, _cx| { + display_map.replace_blocks(new_styles); + }); + } else { + self.dismiss_diagnostics(cx); } - self.display_map.update(cx, |display_map, _cx| { - display_map.replace_blocks(new_styles) - }); } } } @@ -12599,7 +12603,7 @@ impl Editor { buffer.anchor_after(entry.range.start), ), height: message_height, - render: diagnostic_block_renderer(diagnostic, None, true, true), + render: diagnostic_block_renderer(diagnostic, None, true), priority: 0, } }), @@ -17795,7 +17799,6 @@ pub fn diagnostic_block_renderer( diagnostic: Diagnostic, max_message_rows: Option, allow_closing: bool, - _is_valid: bool, ) -> RenderBlock { let (text_without_backticks, code_ranges) = highlight_diagnostic_message(&diagnostic, max_message_rows); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 36b9108bc4..5d2bcbdf52 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -10970,6 +10970,106 @@ async fn cycle_through_same_place_diagnostics( "}); } +#[gpui::test] +async fn active_diagnostics_dismiss_after_invalidation( + 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.as_ref().unwrap().read(cx).lsp_store()); + + cx.set_state(indoc! {" + ˇfn func(abc def: i32) -> u32 { + } + "}); + + let message = "Something's wrong!"; + cx.update(|_, cx| { + lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .update_diagnostics( + LanguageServerId(0), + lsp::PublishDiagnosticsParams { + uri: lsp::Url::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), + message: message.to_string(), + ..Default::default() + }], + }, + &[], + cx, + ) + .unwrap() + }); + }); + executor.run_until_parked(); + + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic, window, cx); + assert_eq!( + editor + .active_diagnostics + .as_ref() + .map(|diagnostics_group| diagnostics_group.primary_message.as_str()), + Some(message), + "Should have a diagnostics group activated" + ); + }); + cx.assert_editor_state(indoc! {" + fn func(abcˇ def: i32) -> u32 { + } + "}); + + cx.update(|_, cx| { + lsp_store.update(cx, |lsp_store, cx| { + lsp_store + .update_diagnostics( + LanguageServerId(0), + lsp::PublishDiagnosticsParams { + uri: lsp::Url::from_file_path(path!("/root/file")).unwrap(), + version: None, + diagnostics: Vec::new(), + }, + &[], + cx, + ) + .unwrap() + }); + }); + executor.run_until_parked(); + cx.update_editor(|editor, _, _| { + assert_eq!( + editor.active_diagnostics, None, + "After no diagnostics set to the editor, no diagnostics should be active" + ); + }); + cx.assert_editor_state(indoc! {" + fn func(abcˇ def: i32) -> u32 { + } + "}); + + cx.update_editor(|editor, window, cx| { + editor.go_to_diagnostic(&GoToDiagnostic, window, cx); + assert_eq!( + editor.active_diagnostics, None, + "Should be no diagnostics to go to and activate" + ); + }); + cx.assert_editor_state(indoc! {" + fn func(abcˇ def: i32) -> u32 { + } + "}); +} + #[gpui::test] async fn test_diagnostics_with_links(cx: &mut TestAppContext) { init_test(cx, |_| {});