Change DiagnosticProvider

helix-core/src/diagnostic.rs

@@ -51,17 +51,77 @@ 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),
+    Lsp {
+        server_id: LanguageServerId,
+        identifier: Option<String>,
+    },
+    // In the future, other non-LSP providers like spell checking go here...
+}
+
+impl DiagnosticProvider {
+    pub fn from_server_id(server_id: LanguageServerId) -> DiagnosticProvider {
+        DiagnosticProvider::Lsp {
+            server_id,
+            identifier: None,
+        }
+    }
+
+    pub fn from_server_and_identifier(
+        server_id: LanguageServerId,
+        identifier: Option<String>,
+    ) -> DiagnosticProvider {
+        DiagnosticProvider::Lsp {
+            server_id,
+            identifier,
+        }
+    }
+
+    pub fn server_id(&self) -> &LanguageServerId {
+        match self {
+            DiagnosticProvider::Lsp {
+                server_id,
+                identifier: _,
+            } => server_id,
+        }
+    }
+
+    pub fn has_server_id(&self, server_id: &LanguageServerId) -> bool {
+        match self {
+            DiagnosticProvider::Lsp {
+                server_id: id,
+                identifier: _,
+            } => server_id == id,
+        }
+    }
+
+    pub fn equals(&self, diagnostic_provider: &DiagnosticProvider) -> bool {
+        let (other_identifier, other_server_id) = match diagnostic_provider {
+            DiagnosticProvider::Lsp {
+                server_id,
+                identifier,
+            } => (identifier, server_id),
+        };
+
+        let (identifier, server_id) = match self {
+            DiagnosticProvider::Lsp {
+                server_id,
+                identifier,
+            } => (identifier, server_id),
+        };
+
+        identifier == other_identifier && server_id == other_server_id
+    }
 }
 
 impl From<DiagnosticProvider> for LanguageServerId {
     fn from(value: DiagnosticProvider) -> Self {
         match value {
-            DiagnosticProvider::PublishDiagnosticProvider(id) => id,
-            DiagnosticProvider::PullDiagnosticProvider(id) => id,
+            DiagnosticProvider::Lsp {
+                server_id,
+                identifier: _,
+            } => server_id,
         }
     }
 }
@@ -86,3 +146,85 @@ 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::from_server_id(LanguageServerId(KeyData::from_ffi(1)));
+        let second_provider =
+            DiagnosticProvider::from_server_id(LanguageServerId(KeyData::from_ffi(1)));
+
+        assert!(first_provider.equals(&second_provider));
+    }
+
+    #[test]
+    fn can_compare_equal_diagnostic_provider_with_identifier() {
+        let first_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            Some("provider".to_string()),
+        );
+        let second_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            Some("provider".to_string()),
+        );
+
+        assert!(first_provider.equals(&second_provider));
+    }
+
+    #[test]
+    fn can_distinguish_diagnostic_provider() {
+        let first_provider =
+            DiagnosticProvider::from_server_id(LanguageServerId(KeyData::from_ffi(1)));
+        let second_provider =
+            DiagnosticProvider::from_server_id(LanguageServerId(KeyData::from_ffi(2)));
+
+        assert!(!first_provider.equals(&second_provider));
+    }
+
+    #[test]
+    fn can_distinguish_diagnostic_provider_by_identifier() {
+        let first_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            Some("provider".to_string()),
+        );
+        let second_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            None,
+        );
+
+        assert!(!first_provider.equals(&second_provider));
+    }
+
+    #[test]
+    fn can_distinguish_diagnostic_provider_by_language_server_id() {
+        let first_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            Some("provider".to_string()),
+        );
+        let second_provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(2)),
+            Some("provider".to_string()),
+        );
+
+        assert!(!first_provider.equals(&second_provider));
+    }
+
+    #[test]
+    fn can_compare_language_server_id() {
+        let provider = DiagnosticProvider::from_server_and_identifier(
+            LanguageServerId(KeyData::from_ffi(1)),
+            Some("provider".to_string()),
+        );
+
+        let language_server_id = LanguageServerId(KeyData::from_ffi(1));
+
+        assert!(provider.has_server_id(&language_server_id));
+    }
+}

helix-term/src/application.rs

@@ -761,10 +761,10 @@ impl Application {
                         }
 
                         let diagnostic_provider =
-                            DiagnosticProvider::PublishDiagnosticProvider(language_server.id());
+                            DiagnosticProvider::from_server_id(language_server.id());
 
                         self.editor.handle_lsp_diagnostics(
-                            diagnostic_provider,
+                            &diagnostic_provider,
                             uri,
                             params.version,
                             params.diagnostics,
@@ -878,7 +878,7 @@ impl Application {
                         // an empty diagnostic list for said files
                         for diags in self.editor.diagnostics.values_mut() {
                             diags.retain(|(_, diagnostic_provider)| {
-                                Into::<LanguageServerId>::into(*diagnostic_provider) != server_id
+                                !diagnostic_provider.has_server_id(&server_id)
                             });
                         }
 

helix-term/src/handlers/diagnostics.rs

@@ -119,7 +119,16 @@ pub fn pull_diagnostics_for_document(
         return;
     };
 
-    let provider = DiagnosticProvider::PullDiagnosticProvider(language_server.id());
+    let identifier = language_server
+        .capabilities()
+        .diagnostic_provider
+        .as_ref()
+        .and_then(|diagnostic_provider| match diagnostic_provider {
+            lsp::DiagnosticServerCapabilities::Options(options) => options.identifier.clone(),
+            lsp::DiagnosticServerCapabilities::RegistrationOptions(_) => None,
+        });
+
+    let provider = DiagnosticProvider::from_server_and_identifier(language_server.id(), identifier);
     let document_id = doc.id();
 
     tokio::spawn(async move {
@@ -150,7 +159,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,
@@ -178,7 +187,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),

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<InlineAnnotation>| {
@@ -2028,13 +2033,16 @@ impl Document {
         &mut self,
         diagnostics: impl IntoIterator<Item = Diagnostic>,
         unchanged_sources: &[String],
-        language_server_id: Option<DiagnosticProvider>,
+        diagnostic_provider: Option<DiagnosticProvider>,
     ) {
         if unchanged_sources.is_empty() {
-            self.clear_diagnostics(language_server_id);
+            self.clear_diagnostics(diagnostic_provider);
         } else {
             self.diagnostics.retain(|d| {
-                if language_server_id.is_some_and(|id| id != 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<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<LanguageServerId>) {
         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();
         }

helix-view/src/editor.rs

@@ -2022,8 +2022,7 @@ impl Editor {
                 diags
                     .iter()
                     .filter_map(move |(diagnostic, diagnostic_provider)| {
-                        let ls = language_servers
-                            .get_by_id(Into::<LanguageServerId>::into(*diagnostic_provider))?;
+                        let ls = language_servers.get_by_id(*diagnostic_provider.server_id())?;
                         language_config
                             .as_ref()
                             .and_then(|c| {
@@ -2033,12 +2032,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 {

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| &ls.id() == d.provider.server_id())
                 });
             diagnostics_on_line.max_by_key(|d| d.severity).map(|d| {
                 write!(out, "●").ok();

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<i32>,
         mut diagnostics: Vec<lsp::Diagnostic>,
@@ -315,7 +315,7 @@ impl Editor {
                 let old_diagnostics = old_diagnostics
                     .iter()
                     .filter(|(d, d_server)| {
-                        *d_server == diagnostic_provider && d.source.as_ref() == Some(source)
+                        *d_server == *diagnostic_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,9 @@ 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(|(_, diagnostic_provider)| {
+                    !diagnostic_provider.equals(diagnostic_provider)
+                });
                 current_diagnostics.extend(diagnostics);
                 current_diagnostics
                 // Sort diagnostics first by severity and then by line numbers.
@@ -343,12 +347,14 @@ 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, diagnostic_provider)| {
+            (d.severity, d.range.start, diagnostic_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_id: DiagnosticProvider| {
+                    ls_id.equals(diagnostic_provider)
                         && diagnostic
                             .source
                             .as_ref()
@@ -363,7 +369,7 @@ impl Editor {
             doc.replace_diagnostics(
                 diagnostics,
                 &unchanged_diag_sources,
-                Some(diagnostic_provider),
+                Some(diagnostic_provider.clone()),
             );
 
             let doc = doc.id();