From 96cf04288db8d3d7f6df6d688cd147c70a9967bc Mon Sep 17 00:00:00 2001 From: Sofus Addington Date: Fri, 7 Mar 2025 06:07:09 +0100 Subject: [PATCH] Prepare for indentifier in diagnostic provider --- helix-core/src/diagnostic.rs | 67 +++++++++++++++++++++++++- helix-term/src/application.rs | 4 +- helix-term/src/handlers/diagnostics.rs | 4 +- helix-view/src/document.rs | 32 ++++++++---- helix-view/src/editor.rs | 7 ++- helix-view/src/gutter.rs | 2 +- helix-view/src/handlers/lsp.rs | 20 +++++--- 7 files changed, 109 insertions(+), 27 deletions(-) diff --git a/helix-core/src/diagnostic.rs b/helix-core/src/diagnostic.rs index 78d48edb..a133fae1 100644 --- a/helix-core/src/diagnostic.rs +++ b/helix-core/src/diagnostic.rs @@ -51,7 +51,7 @@ pub struct Diagnostic { } // TODO turn this into a feature flag when lsp becomes optional -#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum DiagnosticProvider { PublishDiagnosticProvider(LanguageServerId), PullDiagnosticProvider(LanguageServerId), @@ -74,6 +74,33 @@ slotmap::new_key_type! { pub struct LanguageServerId; } +impl DiagnosticProvider { + pub fn server_id(&self) -> &LanguageServerId { + match self { + DiagnosticProvider::PublishDiagnosticProvider(language_server_id) => language_server_id, + DiagnosticProvider::PullDiagnosticProvider(language_server_id) => language_server_id, + } + } + + pub fn has_server_id(&self, server_id: &LanguageServerId) -> bool { + match self { + DiagnosticProvider::PublishDiagnosticProvider(language_server_id) => { + language_server_id == server_id + } + DiagnosticProvider::PullDiagnosticProvider(language_server_id) => { + language_server_id == server_id + } + } + } + + pub fn equals(&self, diagnostic_provider: &DiagnosticProvider) -> bool { + let server_id = self.server_id(); + let other_server_id = diagnostic_provider.server_id(); + + server_id == other_server_id + } +} + impl fmt::Display for LanguageServerId { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{:?}", self.0) @@ -86,3 +113,41 @@ impl Diagnostic { self.severity.unwrap_or(Severity::Warning) } } + +#[cfg(test)] +mod tests { + use slotmap::KeyData; + + use super::DiagnosticProvider; + use crate::diagnostic::LanguageServerId; + + #[test] + fn can_compare_equal_diagnostic_provider() { + let first_provider = + DiagnosticProvider::PublishDiagnosticProvider(LanguageServerId(KeyData::from_ffi(1))); + let second_provider = + DiagnosticProvider::PublishDiagnosticProvider(LanguageServerId(KeyData::from_ffi(1))); + + assert!(first_provider.equals(&second_provider)); + } + + #[test] + fn can_distinguish_diagnostic_provider() { + let first_provider = + DiagnosticProvider::PublishDiagnosticProvider(LanguageServerId(KeyData::from_ffi(1))); + let second_provider = + DiagnosticProvider::PullDiagnosticProvider(LanguageServerId(KeyData::from_ffi(1))); + + assert!(first_provider.equals(&second_provider)); + } + + #[test] + fn can_compare_language_server_id() { + let provider = + DiagnosticProvider::PublishDiagnosticProvider(LanguageServerId(KeyData::from_ffi(1))); + + let language_server_id = LanguageServerId(KeyData::from_ffi(1)); + + assert!(provider.has_server_id(&language_server_id)); + } +} diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 92ab8261..57c82634 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -757,7 +757,7 @@ impl Application { DiagnosticProvider::PublishDiagnosticProvider(language_server.id()); self.editor.handle_lsp_diagnostics( - diagnostic_provider, + &diagnostic_provider, uri, params.version, params.diagnostics, @@ -871,7 +871,7 @@ impl Application { // an empty diagnostic list for said files for diags in self.editor.diagnostics.values_mut() { diags.retain(|(_, diagnostic_provider)| { - Into::::into(*diagnostic_provider) != server_id + !diagnostic_provider.has_server_id(&server_id) }); } diff --git a/helix-term/src/handlers/diagnostics.rs b/helix-term/src/handlers/diagnostics.rs index a93fc396..bdd1408c 100644 --- a/helix-term/src/handlers/diagnostics.rs +++ b/helix-term/src/handlers/diagnostics.rs @@ -153,7 +153,7 @@ fn handle_pull_diagnostics_response( let (result_id, related_documents) = match response { lsp::DocumentDiagnosticReport::Full(report) => { editor.handle_lsp_diagnostics( - provider, + &provider, uri, None, report.full_document_diagnostic_report.items, @@ -181,7 +181,7 @@ fn handle_pull_diagnostics_response( continue; }; - editor.handle_lsp_diagnostics(provider, uri, None, report.items); + editor.handle_lsp_diagnostics(&provider, uri, None, report.items); report.result_id } lsp::DocumentDiagnosticReportKind::Unchanged(report) => Some(report.result_id), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 5e066bb5..1398520b 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1407,8 +1407,13 @@ impl Document { true }); - self.diagnostics - .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider)); + self.diagnostics.sort_by_key(|diagnostic| { + ( + diagnostic.range, + diagnostic.severity, + diagnostic.provider.clone(), + ) + }); // Update the inlay hint annotations' positions, helping ensure they are displayed in the proper place let apply_inlay_hint_changes = |annotations: &mut Vec| { @@ -2028,13 +2033,16 @@ impl Document { &mut self, diagnostics: impl IntoIterator, unchanged_sources: &[String], - diagnostic_provider: Option, + diagnostic_provider: Option<&DiagnosticProvider>, ) { if unchanged_sources.is_empty() { self.clear_diagnostics(diagnostic_provider); } else { self.diagnostics.retain(|d| { - if diagnostic_provider.is_some_and(|provider| provider != d.provider) { + if diagnostic_provider + .as_ref() + .is_some_and(|provider| !provider.equals(&d.provider)) + { return true; } @@ -2046,14 +2054,19 @@ impl Document { }); } self.diagnostics.extend(diagnostics); - self.diagnostics - .sort_by_key(|diagnostic| (diagnostic.range, diagnostic.severity, diagnostic.provider)); + self.diagnostics.sort_by_key(|diagnostic| { + ( + diagnostic.range, + diagnostic.severity, + diagnostic.provider.clone(), + ) + }); } /// clears diagnostics for a given diagnostic provider if set, otherwise all diagnostics are cleared - pub fn clear_diagnostics(&mut self, provider: Option) { + pub fn clear_diagnostics(&mut self, provider: Option<&DiagnosticProvider>) { if let Some(provider) = provider { - self.diagnostics.retain(|d| d.provider != provider); + self.diagnostics.retain(|d| !d.provider.equals(provider)); } else { self.diagnostics.clear(); } @@ -2062,7 +2075,8 @@ impl Document { /// clears diagnostics for a given language_server if set, otherwise all diagnostics are cleared pub fn clear_all_language_server_diagnostics(&mut self, server_id: Option) { if let Some(server_id) = server_id { - self.diagnostics.retain(|d| server_id != d.provider.into()); + self.diagnostics + .retain(|d| !d.provider.has_server_id(&server_id)); } else { self.diagnostics.clear(); } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index bcfe68d7..041efb3b 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -2030,8 +2030,7 @@ impl Editor { diags .iter() .filter_map(move |(diagnostic, diagnostic_provider)| { - let ls = language_servers - .get_by_id(Into::::into(*diagnostic_provider))?; + let ls = language_servers.get_by_id(*diagnostic_provider.server_id())?; language_config .as_ref() .and_then(|c| { @@ -2041,12 +2040,12 @@ impl Editor { }) }) .and_then(|_| { - if filter(diagnostic, *diagnostic_provider) { + if filter(diagnostic, diagnostic_provider.clone()) { Document::lsp_diagnostic_to_diagnostic( &text, language_config.as_deref(), diagnostic, - *diagnostic_provider, + diagnostic_provider.clone(), ls.offset_encoding(), ) } else { diff --git a/helix-view/src/gutter.rs b/helix-view/src/gutter.rs index b8456ffc..5e5c1182 100644 --- a/helix-view/src/gutter.rs +++ b/helix-view/src/gutter.rs @@ -71,7 +71,7 @@ pub fn diagnostic<'doc>( d.line == line && doc .language_servers_with_feature(LanguageServerFeature::Diagnostics) - .any(|ls| ls.id() == d.provider.into()) + .any(|ls| d.provider.has_server_id(&ls.id())) }); diagnostics_on_line.max_by_key(|d| d.severity).map(|d| { write!(out, "●").ok(); diff --git a/helix-view/src/handlers/lsp.rs b/helix-view/src/handlers/lsp.rs index b9932727..49d03905 100644 --- a/helix-view/src/handlers/lsp.rs +++ b/helix-view/src/handlers/lsp.rs @@ -281,7 +281,7 @@ impl Editor { pub fn handle_lsp_diagnostics( &mut self, - diagnostic_provider: DiagnosticProvider, + diagnostic_provider: &DiagnosticProvider, uri: Uri, version: Option, mut diagnostics: Vec, @@ -314,8 +314,8 @@ impl Editor { .filter(|d| d.source.as_ref() == Some(source)); let old_diagnostics = old_diagnostics .iter() - .filter(|(d, d_server)| { - *d_server == diagnostic_provider && d.source.as_ref() == Some(source) + .filter(|(d, d_provider)| { + diagnostic_provider.equals(d_provider) && d.source.as_ref() == Some(source) }) .map(|(d, _)| d); if new_diagnostics.eq(old_diagnostics) { @@ -324,7 +324,9 @@ impl Editor { } } - let diagnostics = diagnostics.into_iter().map(|d| (d, diagnostic_provider)); + let diagnostics = diagnostics + .into_iter() + .map(|d| (d, diagnostic_provider.clone())); // Insert the original lsp::Diagnostics here because we may have no open document // for diagnostic message and so we can't calculate the exact position. @@ -333,7 +335,8 @@ impl Editor { Entry::Occupied(o) => { let current_diagnostics = o.into_mut(); // there may entries of other language servers, which is why we can't overwrite the whole entry - current_diagnostics.retain(|(_, lsp_id)| *lsp_id != diagnostic_provider); + current_diagnostics + .retain(|(_, ls_provider)| !diagnostic_provider.equals(ls_provider)); current_diagnostics.extend(diagnostics); current_diagnostics // Sort diagnostics first by severity and then by line numbers. @@ -343,12 +346,13 @@ impl Editor { // Sort diagnostics first by severity and then by line numbers. // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order - diagnostics.sort_by_key(|(d, server_id)| (d.severity, d.range.start, *server_id)); + diagnostics + .sort_by_key(|(d, ls_provider)| (d.severity, d.range.start, ls_provider.clone())); if let Some(doc) = doc { let diagnostic_of_language_server_and_not_in_unchanged_sources = - |diagnostic: &lsp::Diagnostic, ls_id| { - ls_id == diagnostic_provider + |diagnostic: &lsp::Diagnostic, ls_provider| { + diagnostic_provider.equals(&ls_provider) && diagnostic .source .as_ref()