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 <sergeradinovich@gmail.com>

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:

<img width="551" alt="Screenshot 2025-04-29 at 16 13 45"
src="https://github.com/user-attachments/assets/d289be31-717d-404f-a76a-a0cda3e96fbe"
/>

Co-authored-by: Serge Radinovich <sergeradinovich@gmail.com>
This commit is contained in:
Conrad Irwin 2025-04-29 19:53:05 -06:00 committed by GitHub
parent b4732235e3
commit e364e48266
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 106 additions and 99 deletions

View file

@ -7,7 +7,7 @@ use editor::{
scroll::Autoscroll, scroll::Autoscroll,
}; };
use gpui::{AppContext, Entity, Focusable, WeakEntity}; use gpui::{AppContext, Entity, Focusable, WeakEntity};
use language::{BufferId, DiagnosticEntry}; use language::{BufferId, Diagnostic, DiagnosticEntry};
use lsp::DiagnosticSeverity; use lsp::DiagnosticSeverity;
use markdown::{Markdown, MarkdownElement}; use markdown::{Markdown, MarkdownElement};
use settings::Settings; use settings::Settings;
@ -28,7 +28,6 @@ impl DiagnosticRenderer {
diagnostic_group: Vec<DiagnosticEntry<Point>>, diagnostic_group: Vec<DiagnosticEntry<Point>>,
buffer_id: BufferId, buffer_id: BufferId,
diagnostics_editor: Option<WeakEntity<ProjectDiagnosticsEditor>>, diagnostics_editor: Option<WeakEntity<ProjectDiagnosticsEditor>>,
merge_same_row: bool,
cx: &mut App, cx: &mut App,
) -> Vec<DiagnosticBlock> { ) -> Vec<DiagnosticBlock> {
let Some(primary_ix) = diagnostic_group let Some(primary_ix) = diagnostic_group
@ -38,106 +37,88 @@ impl DiagnosticRenderer {
return Vec::new(); return Vec::new();
}; };
let primary = diagnostic_group[primary_ix].clone(); 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; 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 { if entry.diagnostic.is_primary {
continue; let mut markdown = Self::markdown(&entry.diagnostic);
} let diagnostic = &primary.diagnostic;
if entry.range.start.row == primary.range.start.row && merge_same_row { if diagnostic.source.is_some() || diagnostic.code.is_some() {
same_row.push(entry) 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 { } 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 { } 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 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 { impl editor::DiagnosticRenderer for DiagnosticRenderer {
@ -149,7 +130,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
editor: WeakEntity<Editor>, editor: WeakEntity<Editor>,
cx: &mut App, cx: &mut App,
) -> Vec<BlockProperties<Anchor>> { ) -> Vec<BlockProperties<Anchor>> {
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 blocks
.into_iter() .into_iter()
.map(|block| { .map(|block| {
@ -176,8 +157,7 @@ impl editor::DiagnosticRenderer for DiagnosticRenderer {
buffer_id: BufferId, buffer_id: BufferId,
cx: &mut App, cx: &mut App,
) -> Option<Entity<Markdown>> { ) -> Option<Entity<Markdown>> {
let blocks = let blocks = Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, cx);
Self::diagnostic_blocks_for_group(diagnostic_group, buffer_id, None, false, cx);
blocks.into_iter().find_map(|block| { blocks.into_iter().find_map(|block| {
if block.initial_range == range { if block.initial_range == range {
Some(block.markdown) Some(block.markdown)
@ -211,7 +191,7 @@ impl DiagnosticBlock {
let cx = &bcx.app; let cx = &bcx.app;
let status_colors = bcx.app.theme().status(); 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 { let (background_color, border_color) = match self.severity {
DiagnosticSeverity::ERROR => (status_colors.error_background, status_colors.error), DiagnosticSeverity::ERROR => (status_colors.error_background, status_colors.error),

View file

@ -416,7 +416,6 @@ impl ProjectDiagnosticsEditor {
group, group,
buffer_snapshot.remote_id(), buffer_snapshot.remote_id(),
Some(this.clone()), Some(this.clone()),
true,
cx, cx,
) )
})?; })?;

View file

@ -633,7 +633,7 @@ pub fn hover_markdown_style(window: &Window, cx: &App) -> MarkdownStyle {
base_text_style, base_text_style,
code_block: StyleRefinement::default().my(rems(1.)).font_buffer(cx), code_block: StyleRefinement::default().my(rems(1.)).font_buffer(cx),
inline_code: TextStyleRefinement { 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_family: Some(buffer_font_family),
font_fallbacks: buffer_font_fallbacks, font_fallbacks: buffer_font_fallbacks,
..Default::default() ..Default::default()

View file

@ -213,6 +213,8 @@ pub struct Diagnostic {
pub severity: DiagnosticSeverity, pub severity: DiagnosticSeverity,
/// The human-readable message associated with this diagnostic. /// The human-readable message associated with this diagnostic.
pub message: String, pub message: String,
/// The human-readable message (in markdown format)
pub markdown: Option<String>,
/// An id that identifies the group to which this diagnostic belongs. /// An id that identifies the group to which this diagnostic belongs.
/// ///
/// When a language server produces a diagnostic with /// When a language server produces a diagnostic with
@ -4616,6 +4618,7 @@ impl Default for Diagnostic {
code_description: None, code_description: None,
severity: DiagnosticSeverity::ERROR, severity: DiagnosticSeverity::ERROR,
message: Default::default(), message: Default::default(),
markdown: None,
group_id: 0, group_id: 0,
is_primary: false, is_primary: false,
is_disk_based: false, is_disk_based: false,

View file

@ -239,6 +239,10 @@ impl CachedLspAdapter {
.process_diagnostics(params, server_id, existing_diagnostics) .process_diagnostics(params, server_id, existing_diagnostics)
} }
pub fn diagnostic_message_to_markdown(&self, message: &str) -> Option<String> {
self.adapter.diagnostic_message_to_markdown(message)
}
pub async fn process_completions(&self, completion_items: &mut [lsp::CompletionItem]) { pub async fn process_completions(&self, completion_items: &mut [lsp::CompletionItem]) {
self.adapter.process_completions(completion_items).await 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. /// Post-processes completions provided by the language server.
async fn process_completions(&self, _: &mut [lsp::CompletionItem]) {} async fn process_completions(&self, _: &mut [lsp::CompletionItem]) {}
fn diagnostic_message_to_markdown(&self, _message: &str) -> Option<String> {
None
}
async fn labels_for_completions( async fn labels_for_completions(
self: Arc<Self>, self: Arc<Self>,
completions: &[lsp::CompletionItem], completions: &[lsp::CompletionItem],

View file

@ -203,6 +203,7 @@ pub fn serialize_diagnostics<'a>(
start: Some(serialize_anchor(&entry.range.start)), start: Some(serialize_anchor(&entry.range.start)),
end: Some(serialize_anchor(&entry.range.end)), end: Some(serialize_anchor(&entry.range.end)),
message: entry.diagnostic.message.clone(), message: entry.diagnostic.message.clone(),
markdown: entry.diagnostic.markdown.clone(),
severity: match entry.diagnostic.severity { severity: match entry.diagnostic.severity {
DiagnosticSeverity::ERROR => proto::diagnostic::Severity::Error, DiagnosticSeverity::ERROR => proto::diagnostic::Severity::Error,
DiagnosticSeverity::WARNING => proto::diagnostic::Severity::Warning, DiagnosticSeverity::WARNING => proto::diagnostic::Severity::Warning,
@ -422,6 +423,7 @@ pub fn deserialize_diagnostics(
proto::diagnostic::Severity::None => return None, proto::diagnostic::Severity::None => return None,
}, },
message: diagnostic.message, message: diagnostic.message,
markdown: diagnostic.markdown,
group_id: diagnostic.group_id as usize, group_id: diagnostic.group_id as usize,
code: diagnostic.code.map(lsp::NumberOrString::from_string), code: diagnostic.code.map(lsp::NumberOrString::from_string),
code_description: diagnostic code_description: diagnostic

View file

@ -283,6 +283,12 @@ impl LspAdapter for RustLspAdapter {
} }
} }
fn diagnostic_message_to_markdown(&self, message: &str) -> Option<String> {
static REGEX: LazyLock<Regex> =
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( async fn label_for_completion(
&self, &self,
completion: &lsp::CompletionItem, completion: &lsp::CompletionItem,

View file

@ -8575,6 +8575,8 @@ impl LspStore {
let mut sources_by_group_id = HashMap::default(); let mut sources_by_group_id = HashMap::default();
let mut supporting_diagnostics = 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 // Ensure that primary diagnostics are always the most severe
params.diagnostics.sort_by_key(|item| item.severity); params.diagnostics.sort_by_key(|item| item.severity);
@ -8622,6 +8624,9 @@ impl LspStore {
.as_ref() .as_ref()
.map(|d| d.href.clone()), .map(|d| d.href.clone()),
severity: diagnostic.severity.unwrap_or(DiagnosticSeverity::ERROR), 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(), message: diagnostic.message.trim().to_string(),
group_id, group_id,
is_primary: true, is_primary: true,
@ -8644,6 +8649,9 @@ impl LspStore {
.as_ref() .as_ref()
.map(|c| c.href.clone()), .map(|c| c.href.clone()),
severity: DiagnosticSeverity::INFORMATION, severity: DiagnosticSeverity::INFORMATION,
markdown: adapter.as_ref().and_then(|adapter| {
adapter.diagnostic_message_to_markdown(&info.message)
}),
message: info.message.trim().to_string(), message: info.message.trim().to_string(),
group_id, group_id,
is_primary: false, is_primary: false,

View file

@ -271,6 +271,7 @@ message Diagnostic {
} }
optional string data = 12; optional string data = 12;
optional string code_description = 13; optional string code_description = 13;
optional string markdown = 14;
} }
message SearchQuery { message SearchQuery {