From e364e482669f29fb010a7da28b8f5a78de4baf16 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 29 Apr 2025 19:53:05 -0600 Subject: [PATCH] Tidy up diagnostics more (#29629) - Stop merging same row diagnostics - (for Rust) show code fragments surrounded by `'s in monospace Co-authored-by: Serge Radinovich Closes #29362 Release Notes: - diagnostics: Diagnostics are no longer merged when they're on the same line - rust: Diagnostics now show code snippets in monospace font: Screenshot 2025-04-29 at 16 13 45 Co-authored-by: Serge Radinovich --- crates/diagnostics/src/diagnostic_renderer.rs | 174 ++++++++---------- crates/diagnostics/src/diagnostics.rs | 1 - crates/editor/src/hover_popover.rs | 2 +- crates/language/src/buffer.rs | 3 + crates/language/src/language.rs | 8 + crates/language/src/proto.rs | 2 + crates/languages/src/rust.rs | 6 + crates/project/src/lsp_store.rs | 8 + crates/proto/proto/buffer.proto | 1 + 9 files changed, 106 insertions(+), 99 deletions(-) diff --git a/crates/diagnostics/src/diagnostic_renderer.rs b/crates/diagnostics/src/diagnostic_renderer.rs index 538fdb0e4b..c5537573c7 100644 --- a/crates/diagnostics/src/diagnostic_renderer.rs +++ b/crates/diagnostics/src/diagnostic_renderer.rs @@ -7,7 +7,7 @@ use editor::{ scroll::Autoscroll, }; use gpui::{AppContext, Entity, Focusable, WeakEntity}; -use language::{BufferId, DiagnosticEntry}; +use language::{BufferId, Diagnostic, DiagnosticEntry}; use lsp::DiagnosticSeverity; use markdown::{Markdown, MarkdownElement}; use settings::Settings; @@ -28,7 +28,6 @@ impl DiagnosticRenderer { diagnostic_group: Vec>, buffer_id: BufferId, diagnostics_editor: Option>, - merge_same_row: bool, cx: &mut App, ) -> Vec { let Some(primary_ix) = diagnostic_group @@ -38,106 +37,88 @@ impl DiagnosticRenderer { return Vec::new(); }; let primary = diagnostic_group[primary_ix].clone(); - let mut same_row = Vec::new(); - let mut close = Vec::new(); - let mut distant = Vec::new(); let group_id = primary.diagnostic.group_id; - for (ix, entry) in diagnostic_group.into_iter().enumerate() { + let mut results = vec![]; + for entry in diagnostic_group.iter() { if entry.diagnostic.is_primary { - continue; - } - if entry.range.start.row == primary.range.start.row && merge_same_row { - same_row.push(entry) + let mut markdown = Self::markdown(&entry.diagnostic); + let diagnostic = &primary.diagnostic; + if diagnostic.source.is_some() || diagnostic.code.is_some() { + markdown.push_str(" ("); + } + if let Some(source) = diagnostic.source.as_ref() { + markdown.push_str(&Markdown::escape(&source)); + } + if diagnostic.source.is_some() && diagnostic.code.is_some() { + markdown.push(' '); + } + if let Some(code) = diagnostic.code.as_ref() { + if let Some(description) = diagnostic.code_description.as_ref() { + markdown.push('['); + markdown.push_str(&Markdown::escape(&code.to_string())); + markdown.push_str("]("); + markdown.push_str(&Markdown::escape(description.as_ref())); + markdown.push(')'); + } else { + markdown.push_str(&Markdown::escape(&code.to_string())); + } + } + if diagnostic.source.is_some() || diagnostic.code.is_some() { + markdown.push(')'); + } + + for (ix, entry) in diagnostic_group.iter().enumerate() { + if entry.range.start.row.abs_diff(primary.range.start.row) >= 5 { + markdown.push_str("\n- hint: ["); + markdown.push_str(&Markdown::escape(&entry.diagnostic.message)); + markdown.push_str(&format!( + "](file://#diagnostic-{buffer_id}-{group_id}-{ix})\n", + )) + } + } + results.push(DiagnosticBlock { + initial_range: primary.range.clone(), + severity: primary.diagnostic.severity, + diagnostics_editor: diagnostics_editor.clone(), + markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), + }); } else if entry.range.start.row.abs_diff(primary.range.start.row) < 5 { - close.push(entry) + let markdown = Self::markdown(&entry.diagnostic); + + results.push(DiagnosticBlock { + initial_range: entry.range.clone(), + severity: entry.diagnostic.severity, + diagnostics_editor: diagnostics_editor.clone(), + markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), + }); } else { - distant.push((ix, entry)) + let mut markdown = Self::markdown(&entry.diagnostic); + markdown.push_str(&format!( + " ([back](file://#diagnostic-{buffer_id}-{group_id}-{primary_ix}))" + )); + + results.push(DiagnosticBlock { + initial_range: entry.range.clone(), + severity: entry.diagnostic.severity, + diagnostics_editor: diagnostics_editor.clone(), + markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), + }); } } - let mut markdown = String::new(); - let diagnostic = &primary.diagnostic; - markdown.push_str(&Markdown::escape(&diagnostic.message)); - for entry in same_row { - markdown.push_str("\n- hint: "); - markdown.push_str(&Markdown::escape(&entry.diagnostic.message)) - } - if diagnostic.source.is_some() || diagnostic.code.is_some() { - markdown.push_str(" ("); - } - if let Some(source) = diagnostic.source.as_ref() { - markdown.push_str(&Markdown::escape(&source)); - } - if diagnostic.source.is_some() && diagnostic.code.is_some() { - markdown.push(' '); - } - if let Some(code) = diagnostic.code.as_ref() { - if let Some(description) = diagnostic.code_description.as_ref() { - markdown.push('['); - markdown.push_str(&Markdown::escape(&code.to_string())); - markdown.push_str("]("); - markdown.push_str(&Markdown::escape(description.as_ref())); - markdown.push(')'); - } else { - markdown.push_str(&Markdown::escape(&code.to_string())); - } - } - if diagnostic.source.is_some() || diagnostic.code.is_some() { - markdown.push(')'); - } - - for (ix, entry) in &distant { - markdown.push_str("\n- hint: ["); - markdown.push_str(&Markdown::escape(&entry.diagnostic.message)); - markdown.push_str(&format!( - "](file://#diagnostic-{buffer_id}-{group_id}-{ix})\n", - )) - } - - let mut results = vec![DiagnosticBlock { - initial_range: primary.range, - severity: primary.diagnostic.severity, - diagnostics_editor: diagnostics_editor.clone(), - markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), - }]; - - for entry in close { - let markdown = if let Some(source) = entry.diagnostic.source.as_ref() { - format!("{}: {}", source, entry.diagnostic.message) - } else { - entry.diagnostic.message - }; - let markdown = Markdown::escape(&markdown).to_string(); - - results.push(DiagnosticBlock { - initial_range: entry.range, - severity: entry.diagnostic.severity, - diagnostics_editor: diagnostics_editor.clone(), - markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), - }); - } - - for (_, entry) in distant { - let markdown = if let Some(source) = entry.diagnostic.source.as_ref() { - format!("{}: {}", source, entry.diagnostic.message) - } else { - entry.diagnostic.message - }; - let mut markdown = Markdown::escape(&markdown).to_string(); - markdown.push_str(&format!( - " ([back](file://#diagnostic-{buffer_id}-{group_id}-{primary_ix}))" - )); - - results.push(DiagnosticBlock { - initial_range: entry.range, - severity: entry.diagnostic.severity, - diagnostics_editor: diagnostics_editor.clone(), - markdown: cx.new(|cx| Markdown::new(markdown.into(), None, None, cx)), - }); - } - results } + + fn markdown(diagnostic: &Diagnostic) -> String { + let mut markdown = String::new(); + + if let Some(md) = &diagnostic.markdown { + markdown.push_str(md); + } else { + markdown.push_str(&Markdown::escape(&diagnostic.message)); + }; + markdown + } } impl editor::DiagnosticRenderer for DiagnosticRenderer { @@ -149,7 +130,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer { editor: WeakEntity, cx: &mut App, ) -> Vec> { - let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, true, cx); + let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, cx); blocks .into_iter() .map(|block| { @@ -176,8 +157,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer { buffer_id: BufferId, cx: &mut App, ) -> Option> { - let blocks = - Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, false, cx); + let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, cx); blocks.into_iter().find_map(|block| { if block.initial_range == range { Some(block.markdown) @@ -211,7 +191,7 @@ impl DiagnosticBlock { let cx = &bcx.app; let status_colors = bcx.app.theme().status(); - let max_width = bcx.em_width * 100.; + let max_width = bcx.em_width * 120.; let (background_color, border_color) = match self.severity { DiagnosticSeverity::ERROR => (status_colors.error_background, status_colors.error), diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index b0ead9ea9b..b8dfad0a6b 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -416,7 +416,6 @@ impl ProjectDiagnosticsEditor { group, buffer_snapshot.remote_id(), Some(this.clone()), - true, cx, ) })?; diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index 586aa7bb02..5a42b57514 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -633,7 +633,7 @@ pub fn hover_markdown_style(window: &Window, cx: &App) -> MarkdownStyle { base_text_style, code_block: StyleRefinement::default().my(rems(1.)).font_buffer(cx), inline_code: TextStyleRefinement { - background_color: Some(cx.theme().colors().background), + background_color: Some(cx.theme().colors().editor_background.opacity(0.5)), font_family: Some(buffer_font_family), font_fallbacks: buffer_font_fallbacks, ..Default::default() diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index ce6405514f..dc3bd40de7 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -213,6 +213,8 @@ pub struct Diagnostic { pub severity: DiagnosticSeverity, /// The human-readable message associated with this diagnostic. pub message: String, + /// The human-readable message (in markdown format) + pub markdown: Option, /// An id that identifies the group to which this diagnostic belongs. /// /// When a language server produces a diagnostic with @@ -4616,6 +4618,7 @@ impl Default for Diagnostic { code_description: None, severity: DiagnosticSeverity::ERROR, message: Default::default(), + markdown: None, group_id: 0, is_primary: false, is_disk_based: false, diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 86e6b8cae9..b19dcd5f9c 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -239,6 +239,10 @@ impl CachedLspAdapter { .process_diagnostics(params, server_id, existing_diagnostics) } + pub fn diagnostic_message_to_markdown(&self, message: &str) -> Option { + self.adapter.diagnostic_message_to_markdown(message) + } + pub async fn process_completions(&self, completion_items: &mut [lsp::CompletionItem]) { self.adapter.process_completions(completion_items).await } @@ -460,6 +464,10 @@ pub trait LspAdapter: 'static + Send + Sync { /// Post-processes completions provided by the language server. async fn process_completions(&self, _: &mut [lsp::CompletionItem]) {} + fn diagnostic_message_to_markdown(&self, _message: &str) -> Option { + None + } + async fn labels_for_completions( self: Arc, completions: &[lsp::CompletionItem], diff --git a/crates/language/src/proto.rs b/crates/language/src/proto.rs index d918b86dd7..03f6b69713 100644 --- a/crates/language/src/proto.rs +++ b/crates/language/src/proto.rs @@ -203,6 +203,7 @@ pub fn serialize_diagnostics<'a>( start: Some(serialize_anchor(&entry.range.start)), end: Some(serialize_anchor(&entry.range.end)), message: entry.diagnostic.message.clone(), + markdown: entry.diagnostic.markdown.clone(), severity: match entry.diagnostic.severity { DiagnosticSeverity::ERROR => proto::diagnostic::Severity::Error, DiagnosticSeverity::WARNING => proto::diagnostic::Severity::Warning, @@ -422,6 +423,7 @@ pub fn deserialize_diagnostics( proto::diagnostic::Severity::None => return None, }, message: diagnostic.message, + markdown: diagnostic.markdown, group_id: diagnostic.group_id as usize, code: diagnostic.code.map(lsp::NumberOrString::from_string), code_description: diagnostic diff --git a/crates/languages/src/rust.rs b/crates/languages/src/rust.rs index b1769023eb..5d58616f57 100644 --- a/crates/languages/src/rust.rs +++ b/crates/languages/src/rust.rs @@ -283,6 +283,12 @@ impl LspAdapter for RustLspAdapter { } } + fn diagnostic_message_to_markdown(&self, message: &str) -> Option { + static REGEX: LazyLock = + LazyLock::new(|| Regex::new(r"(?m)\n *").expect("Failed to create REGEX")); + Some(REGEX.replace_all(message, "\n\n").to_string()) + } + async fn label_for_completion( &self, completion: &lsp::CompletionItem, diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index bad9e423a0..2ffa2e39c6 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -8575,6 +8575,8 @@ impl LspStore { let mut sources_by_group_id = HashMap::default(); let mut supporting_diagnostics = HashMap::default(); + let adapter = self.language_server_adapter_for_id(language_server_id); + // Ensure that primary diagnostics are always the most severe params.diagnostics.sort_by_key(|item| item.severity); @@ -8622,6 +8624,9 @@ impl LspStore { .as_ref() .map(|d| d.href.clone()), severity: diagnostic.severity.unwrap_or(DiagnosticSeverity::ERROR), + markdown: adapter.as_ref().and_then(|adapter| { + adapter.diagnostic_message_to_markdown(&diagnostic.message) + }), message: diagnostic.message.trim().to_string(), group_id, is_primary: true, @@ -8644,6 +8649,9 @@ impl LspStore { .as_ref() .map(|c| c.href.clone()), severity: DiagnosticSeverity::INFORMATION, + markdown: adapter.as_ref().and_then(|adapter| { + adapter.diagnostic_message_to_markdown(&info.message) + }), message: info.message.trim().to_string(), group_id, is_primary: false, diff --git a/crates/proto/proto/buffer.proto b/crates/proto/proto/buffer.proto index 5bb18e54d4..defe449dfc 100644 --- a/crates/proto/proto/buffer.proto +++ b/crates/proto/proto/buffer.proto @@ -271,6 +271,7 @@ message Diagnostic { } optional string data = 12; optional string code_description = 13; + optional string markdown = 14; } message SearchQuery {