Code review

helix-term/src/application.rs

@@ -768,7 +768,6 @@ impl Application {
                             uri,
                             params.version,
                             params.diagnostics,
-                            None,
                         );
                     }
                     Notification::ShowMessage(params) => {

helix-term/src/handlers/diagnostics.rs

@@ -1,4 +1,3 @@
-use std::collections::{HashMap, HashSet};
 use std::time::Duration;
 
 use helix_core::diagnostic::DiagnosticProvider;
@@ -15,7 +14,7 @@ use helix_view::{DocumentId, Editor};
 use tokio::time::Instant;
 
 use crate::events::OnModeSwitch;
-use crate::job;
+use crate::job::{self, dispatch_blocking};
 
 pub(super) fn register_hooks(handlers: &Handlers) {
     register_hook!(move |event: &mut DiagnosticsDidChange<'_>| {
@@ -46,39 +45,23 @@ pub(super) fn register_hooks(handlers: &Handlers) {
     });
 
     register_hook!(move |event: &mut DocumentDidOpen<'_>| {
-        if event
+        for language_server in event
             .doc
-            .has_language_server_with_feature(LanguageServerFeature::PullDiagnostics)
+            .language_servers_with_feature(LanguageServerFeature::PullDiagnostics)
         {
-            let document_id = event.doc.id();
-            job::dispatch_blocking(move |editor, _| {
-                let Some(doc) = editor.document(document_id) else {
-                    return;
-                };
-
-                let language_servers =
-                    doc.language_servers_with_feature(LanguageServerFeature::PullDiagnostics);
-
-                for language_server in language_servers {
-                    pull_diagnostics_for_document(doc, language_server);
-                }
-            })
+            pull_diagnostics_for_document(event.doc, language_server);
         }
 
         Ok(())
     });
 }
 
 #[derive(Debug)]
-pub(super) struct PullDiagnosticsHandler {
-    document_ids: HashSet<DocumentId>,
-}
+pub(super) struct PullDiagnosticsHandler {}
 
 impl PullDiagnosticsHandler {
     pub fn new() -> PullDiagnosticsHandler {
-        PullDiagnosticsHandler {
-            document_ids: [].into(),
-        }
+        PullDiagnosticsHandler {}
     }
 }
 
@@ -94,14 +77,16 @@ impl helix_event::AsyncHook for PullDiagnosticsHandler {
             dispatch_pull_diagnostic_for_document(event.document_id);
         }
 
-        self.document_ids.insert(event.document_id);
         Some(Instant::now() + Duration::from_millis(500))
     }
 
     fn finish_debounce(&mut self) {
-        for document_id in self.document_ids.clone() {
-            dispatch_pull_diagnostic_for_document(document_id);
-        }
+        dispatch_blocking(move |editor, _| {
+            let ids = editor.tree.views().map(|(view, _)| view.doc);
+            for document_id in ids {
+                dispatch_pull_diagnostic_for_document(document_id);
+            }
+        });
     }
 }
 
@@ -162,64 +147,45 @@ fn handle_pull_diagnostics_response(
     uri: Uri,
     document_id: DocumentId,
 ) {
-    let Some(doc) = editor.document_mut(document_id) else {
-        return;
-    };
-
-    match response {
+    let (result_id, related_documents) = match response {
         lsp::DocumentDiagnosticReport::Full(report) => {
-            // Diagnostic for requested file
             editor.handle_lsp_diagnostics(
                 provider,
                 uri,
                 None,
                 report.full_document_diagnostic_report.items,
-                report.full_document_diagnostic_report.result_id,
             );
 
-            // Diagnostic for related files
-            handle_document_diagnostic_report_kind(
-                editor,
-                document_id,
+            (
+                report.full_document_diagnostic_report.result_id,
                 report.related_documents,
-                provider,
-            );
+            )
         }
-        lsp::DocumentDiagnosticReport::Unchanged(report) => {
-            doc.previous_diagnostic_id =
-                Some(report.unchanged_document_diagnostic_report.result_id);
+        lsp::DocumentDiagnosticReport::Unchanged(report) => (
+            Some(report.unchanged_document_diagnostic_report.result_id),
+            report.related_documents,
+        ),
+    };
 
-            handle_document_diagnostic_report_kind(
-                editor,
-                document_id,
-                report.related_documents,
-                provider,
-            );
-        }
-    }
-}
+    if let Some(doc) = editor.document_mut(document_id) {
+        doc.previous_diagnostic_id = result_id;
+    };
 
-fn handle_document_diagnostic_report_kind(
-    editor: &mut Editor,
-    document_id: DocumentId,
-    report: Option<HashMap<lsp::Url, lsp::DocumentDiagnosticReportKind>>,
-    provider: DiagnosticProvider,
-) {
-    for (url, report) in report.into_iter().flatten() {
-        match report {
+    for (url, report) in related_documents.into_iter().flatten() {
+        let result_id = match report {
             lsp::DocumentDiagnosticReportKind::Full(report) => {
                 let Ok(uri) = Uri::try_from(url) else {
-                    return;
+                    continue;
                 };
 
-                editor.handle_lsp_diagnostics(provider, uri, None, report.items, report.result_id);
-            }
-            lsp::DocumentDiagnosticReportKind::Unchanged(report) => {
-                let Some(doc) = editor.document_mut(document_id) else {
-                    return;
-                };
-                doc.previous_diagnostic_id = Some(report.result_id);
+                editor.handle_lsp_diagnostics(provider, uri, None, report.items);
+                report.result_id
             }
+            lsp::DocumentDiagnosticReportKind::Unchanged(report) => Some(report.result_id),
+        };
+
+        if let Some(doc) = editor.document_mut(document_id) {
+            doc.previous_diagnostic_id = result_id;
         }
     }
 }

helix-view/src/handlers/lsp.rs

@@ -285,7 +285,6 @@ impl Editor {
         uri: Uri,
         version: Option<i32>,
         mut diagnostics: Vec<lsp::Diagnostic>,
-        report_id: Option<String>,
     ) {
         let doc = self
             .documents
@@ -367,10 +366,6 @@ impl Editor {
                 Some(diagnostic_provider),
             );
 
-            if report_id.is_some() {
-                doc.previous_diagnostic_id = report_id;
-            }
-
             let doc = doc.id();
             helix_event::dispatch(DiagnosticsDidChange { editor: self, doc });
         }