From 4567360fd944538e7fbc60c64e42161acdd828e6 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 26 May 2025 22:19:02 +0300 Subject: [PATCH] Allow LSP adapters to decide, which diagnostics to underline (#31450) Closes https://github.com/zed-industries/zed/pull/31355#issuecomment-2910439798 image Release Notes: - N/A --- crates/editor/src/display_map.rs | 7 ++----- crates/editor/src/display_map/fold_map.rs | 3 +++ crates/language/src/buffer.rs | 12 ++++++++++++ crates/language/src/language.rs | 14 ++++++++++++++ crates/language/src/proto.rs | 2 ++ crates/languages/src/c.rs | 4 ++++ crates/project/src/lsp_store.rs | 6 ++++++ crates/project/src/lsp_store/clangd_ext.rs | 9 +++++++++ crates/proto/proto/buffer.proto | 1 + 9 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/editor/src/display_map.rs b/crates/editor/src/display_map.rs index 6df1d32e26..374f9ed0ba 100644 --- a/crates/editor/src/display_map.rs +++ b/crates/editor/src/display_map.rs @@ -960,11 +960,8 @@ impl DisplaySnapshot { }) { if chunk.is_unnecessary { diagnostic_highlight.fade_out = Some(editor_style.unnecessary_code_fade); - // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticTag - // states that - // > Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle. - // for the unnecessary diagnostics, so do not underline them. - } else if editor_style.show_underlines { + } + if chunk.underline && editor_style.show_underlines { let diagnostic_color = super::diagnostic_style(severity, &editor_style.status); diagnostic_highlight.underline = Some(UnderlineStyle { color: Some(diagnostic_color), diff --git a/crates/editor/src/display_map/fold_map.rs b/crates/editor/src/display_map/fold_map.rs index 8d199f5b3e..e9a611d390 100644 --- a/crates/editor/src/display_map/fold_map.rs +++ b/crates/editor/src/display_map/fold_map.rs @@ -1255,6 +1255,8 @@ pub struct Chunk<'a> { pub diagnostic_severity: Option, /// Whether this chunk of text is marked as unnecessary. pub is_unnecessary: bool, + /// Whether this chunk of text should be underlined. + pub underline: bool, /// Whether this chunk of text was originally a tab character. pub is_tab: bool, /// An optional recipe for how the chunk should be presented. @@ -1422,6 +1424,7 @@ impl<'a> Iterator for FoldChunks<'a> { diagnostic_severity: chunk.diagnostic_severity, is_unnecessary: chunk.is_unnecessary, is_tab: chunk.is_tab, + underline: chunk.underline, renderer: None, }); } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 38a1034d0f..2d298ff24b 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -231,6 +231,8 @@ pub struct Diagnostic { pub is_unnecessary: bool, /// Data from language server that produced this diagnostic. Passed back to the LS when we request code actions for this diagnostic. pub data: Option, + /// Whether to underline the corresponding text range in the editor. + pub underline: bool, } /// An operation used to synchronize this buffer with its other replicas. @@ -462,6 +464,7 @@ pub struct BufferChunks<'a> { information_depth: usize, hint_depth: usize, unnecessary_depth: usize, + underline: bool, highlights: Option>, } @@ -482,6 +485,8 @@ pub struct Chunk<'a> { pub is_unnecessary: bool, /// Whether this chunk of text was originally a tab character. pub is_tab: bool, + /// Whether to underline the corresponding text range in the editor. + pub underline: bool, } /// A set of edits to a given version of a buffer, computed asynchronously. @@ -496,6 +501,7 @@ pub struct Diff { pub(crate) struct DiagnosticEndpoint { offset: usize, is_start: bool, + underline: bool, severity: DiagnosticSeverity, is_unnecessary: bool, } @@ -4388,6 +4394,7 @@ impl<'a> BufferChunks<'a> { information_depth: 0, hint_depth: 0, unnecessary_depth: 0, + underline: true, highlights, }; this.initialize_diagnostic_endpoints(); @@ -4448,12 +4455,14 @@ impl<'a> BufferChunks<'a> { is_start: true, severity: entry.diagnostic.severity, is_unnecessary: entry.diagnostic.is_unnecessary, + underline: entry.diagnostic.underline, }); diagnostic_endpoints.push(DiagnosticEndpoint { offset: entry.range.end, is_start: false, severity: entry.diagnostic.severity, is_unnecessary: entry.diagnostic.is_unnecessary, + underline: entry.diagnostic.underline, }); } diagnostic_endpoints @@ -4559,6 +4568,7 @@ impl<'a> Iterator for BufferChunks<'a> { if endpoint.offset <= self.range.start { self.update_diagnostic_depths(endpoint); diagnostic_endpoints.next(); + self.underline = endpoint.underline; } else { next_diagnostic_endpoint = endpoint.offset; break; @@ -4590,6 +4600,7 @@ impl<'a> Iterator for BufferChunks<'a> { Some(Chunk { text: slice, syntax_highlight_id: highlight_id, + underline: self.underline, diagnostic_severity: self.current_diagnostic_severity(), is_unnecessary: self.current_code_is_unnecessary(), ..Chunk::default() @@ -4632,6 +4643,7 @@ impl Default for Diagnostic { is_primary: false, is_disk_based: false, is_unnecessary: false, + underline: true, data: None, } } diff --git a/crates/language/src/language.rs b/crates/language/src/language.rs index 3da6101f41..553e715ffa 100644 --- a/crates/language/src/language.rs +++ b/crates/language/src/language.rs @@ -245,6 +245,10 @@ impl CachedLspAdapter { self.adapter.retain_old_diagnostic(previous_diagnostic, cx) } + pub fn underline_diagnostic(&self, diagnostic: &lsp::Diagnostic) -> bool { + self.adapter.underline_diagnostic(diagnostic) + } + pub fn diagnostic_message_to_markdown(&self, message: &str) -> Option { self.adapter.diagnostic_message_to_markdown(message) } @@ -470,6 +474,16 @@ pub trait LspAdapter: 'static + Send + Sync { false } + /// Whether to underline a given diagnostic or not, when rendering in the editor. + /// + /// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticTag + /// states that + /// > Clients are allowed to render diagnostics with this tag faded out instead of having an error squiggle. + /// for the unnecessary diagnostics, so do not underline them. + fn underline_diagnostic(&self, _diagnostic: &lsp::Diagnostic) -> bool { + true + } + /// Post-processes completions provided by the language server. async fn process_completions(&self, _: &mut [lsp::CompletionItem]) {} diff --git a/crates/language/src/proto.rs b/crates/language/src/proto.rs index afedad0f2c..831b7d627b 100644 --- a/crates/language/src/proto.rs +++ b/crates/language/src/proto.rs @@ -213,6 +213,7 @@ pub fn serialize_diagnostics<'a>( } as i32, group_id: entry.diagnostic.group_id as u64, is_primary: entry.diagnostic.is_primary, + underline: entry.diagnostic.underline, code: entry.diagnostic.code.as_ref().map(|s| s.to_string()), code_description: entry .diagnostic @@ -429,6 +430,7 @@ pub fn deserialize_diagnostics( is_primary: diagnostic.is_primary, is_disk_based: diagnostic.is_disk_based, is_unnecessary: diagnostic.is_unnecessary, + underline: diagnostic.underline, data, }, }) diff --git a/crates/languages/src/c.rs b/crates/languages/src/c.rs index 8c7a0147a0..446519c3dc 100644 --- a/crates/languages/src/c.rs +++ b/crates/languages/src/c.rs @@ -285,6 +285,10 @@ impl super::LspAdapter for CLspAdapter { fn retain_old_diagnostic(&self, previous_diagnostic: &Diagnostic, _: &App) -> bool { clangd_ext::is_inactive_region(previous_diagnostic) } + + fn underline_diagnostic(&self, diagnostic: &lsp::Diagnostic) -> bool { + !clangd_ext::is_lsp_inactive_region(diagnostic) + } } async fn get_cached_server_binary(container_dir: PathBuf) -> Option { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index d4601a20b1..f8fdf0450b 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -8782,6 +8782,10 @@ impl LspStore { .as_ref() .map_or(false, |tags| tags.contains(&DiagnosticTag::UNNECESSARY)); + let underline = self + .language_server_adapter_for_id(language_server_id) + .map_or(true, |adapter| adapter.underline_diagnostic(diagnostic)); + if is_supporting { supporting_diagnostics.insert( (source, diagnostic.code.clone(), range), @@ -8814,6 +8818,7 @@ impl LspStore { is_primary: true, is_disk_based, is_unnecessary, + underline, data: diagnostic.data.clone(), }, }); @@ -8839,6 +8844,7 @@ impl LspStore { is_primary: false, is_disk_based, is_unnecessary: false, + underline, data: diagnostic.data.clone(), }, }); diff --git a/crates/project/src/lsp_store/clangd_ext.rs b/crates/project/src/lsp_store/clangd_ext.rs index d211513929..d12015ec31 100644 --- a/crates/project/src/lsp_store/clangd_ext.rs +++ b/crates/project/src/lsp_store/clangd_ext.rs @@ -37,6 +37,15 @@ pub fn is_inactive_region(diag: &Diagnostic) -> bool { .is_some_and(|v| v == CLANGD_SERVER_NAME) } +pub fn is_lsp_inactive_region(diag: &lsp::Diagnostic) -> bool { + diag.severity == Some(INACTIVE_DIAGNOSTIC_SEVERITY) + && diag.message == INACTIVE_REGION_MESSAGE + && diag + .source + .as_ref() + .is_some_and(|v| v == CLANGD_SERVER_NAME) +} + pub fn register_notifications( lsp_store: WeakEntity, language_server: &LanguageServer, diff --git a/crates/proto/proto/buffer.proto b/crates/proto/proto/buffer.proto index defe449dfc..e7692da481 100644 --- a/crates/proto/proto/buffer.proto +++ b/crates/proto/proto/buffer.proto @@ -261,6 +261,7 @@ message Diagnostic { bool is_disk_based = 10; bool is_unnecessary = 11; + bool underline = 15; enum Severity { None = 0;