refactor: move add_diagnostics to separate method

Author: woojiq

helix-term/src/application.rs

@@ -32,7 +32,6 @@ use log::{debug, error, warn};
 #[cfg(not(feature = "integration"))]
 use std::io::stdout;
 use std::{
-    collections::btree_map::Entry,
     io::stdin,
     path::Path,
     sync::{atomic, Arc},
@@ -764,32 +763,8 @@ impl Application {
                             doc.replace_diagnostics(diagnostics, server_id);
                         }
 
-                        let mut diagnostics = params
-                            .diagnostics
-                            .into_iter()
-                            .map(|d| (d, server_id))
-                            .collect();
-
-                        // Insert the original lsp::Diagnostics here because we may have no open document
-                        // for diagnosic message and so we can't calculate the exact position.
-                        // When using them later in the diagnostics picker, we calculate them on-demand.
-                        match self.editor.diagnostics.entry(params.uri) {
-                            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 != server_id);
-                                current_diagnostics.append(&mut diagnostics);
-                                // Sort diagnostics first by severity and then by line numbers.
-                                // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
-                                current_diagnostics
-                                    .sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
-                            }
-                            Entry::Vacant(v) => {
-                                diagnostics
-                                    .sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
-                                v.insert(diagnostics);
-                            }
-                        };
+                        self.editor
+                            .add_diagnostics(params.diagnostics, params.uri, server_id);
                     }
                     Notification::ShowMessage(params) => {
                         log::warn!("unhandled window/showMessage: {:?}", params);

helix-term/src/commands/lsp.rs

@@ -1497,53 +1497,7 @@ pub fn select_references_to_symbol_under_cursor(cx: &mut Context) {
     );
 }
 
-pub fn pull_diagnostic_for_current_doc(editor: &mut Editor, jobs: &mut crate::job::Jobs) {
-    fn parse_diagnostic(
-        doc: &mut Document,
-        offset_encoding: OffsetEncoding,
-        server_id: usize,
-        report: Vec<lsp::Diagnostic>,
-        result_id: Option<String>,
-    ) {
-        let diagnostics = lsp_diagnostic_to_diagnostic(
-            &report,
-            doc.text(),
-            offset_encoding,
-            doc.language_config(),
-            server_id,
-        );
-        doc.previous_diagnostic_id = result_id;
-        doc.replace_diagnostics(diagnostics, server_id);
-    }
-
-    fn handle_document_diagnostic_report_kind(
-        editor: &mut Editor,
-        offset_encoding: OffsetEncoding,
-        server_id: usize,
-        report: Option<HashMap<lsp::Url, lsp::DocumentDiagnosticReportKind>>,
-    ) {
-        for (url, report) in report.into_iter().flatten() {
-            let Some(doc) = editor.document_by_path_mut(url.path())
-            else {
-                continue;
-            };
-            match report {
-                lsp::DocumentDiagnosticReportKind::Full(report) => {
-                    parse_diagnostic(
-                        doc,
-                        offset_encoding,
-                        server_id,
-                        report.items,
-                        report.result_id,
-                    );
-                }
-                lsp::DocumentDiagnosticReportKind::Unchanged(report) => {
-                    doc.previous_diagnostic_id = Some(report.result_id);
-                }
-            }
-        }
-    }
-
+pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::Jobs) {
     let doc = doc!(editor);
     let Some(language_server) = doc
         .language_servers_with_feature(LanguageServerFeature::PullDiagnostics)
@@ -1584,35 +1538,63 @@ pub fn pull_diagnostic_for_current_doc(editor: &mut Editor, jobs: &mut crate::jo
             let offset_encoding = language_server.offset_encoding();
             let server_id = language_server.id();
 
+            let parse_diagnostic = |editor: &mut Editor,
+                                    path,
+                                    report: Vec<lsp::Diagnostic>,
+                                    result_id: Option<String>| {
+                if let Some(doc) = editor.document_by_path_mut(&path) {
+                    let diagnostics = lsp_diagnostic_to_diagnostic(
+                        &report,
+                        doc.text(),
+                        offset_encoding,
+                        doc.language_config(),
+                        server_id,
+                    );
+                    doc.previous_diagnostic_id = result_id;
+                    doc.replace_diagnostics(diagnostics, server_id);
+                }
+                let url = lsp::Url::from_file_path(&path).unwrap();
+                editor.add_diagnostics(report, url, server_id);
+            };
+
+            let handle_document_diagnostic_report_kind = |editor: &mut Editor,
+                                                          report: Option<
+                HashMap<lsp::Url, lsp::DocumentDiagnosticReportKind>,
+            >| {
+                for (url, report) in report.into_iter().flatten() {
+                    match report {
+                        lsp::DocumentDiagnosticReportKind::Full(report) => {
+                            let path = url.to_file_path().unwrap();
+                            parse_diagnostic(editor, path, report.items, report.result_id);
+                        }
+                        lsp::DocumentDiagnosticReportKind::Unchanged(report) => {
+                            let Some(doc) = editor.document_by_path_mut(url.path()) else {
+                                return;
+                            };
+                            doc.previous_diagnostic_id = Some(report.result_id);
+                        }
+                    }
+                }
+            };
+
             if let Some(response) = response {
                 match response {
                     lsp::DocumentDiagnosticReport::Full(report) => {
                         // Original file diagnostic
                         parse_diagnostic(
-                            doc,
-                            offset_encoding,
-                            server_id,
+                            editor,
+                            original_path,
                             report.full_document_diagnostic_report.items,
                             report.full_document_diagnostic_report.result_id,
                         );
 
-                        // Related file diagnostic
-                        handle_document_diagnostic_report_kind(
-                            editor,
-                            offset_encoding,
-                            server_id,
-                            report.related_documents,
-                        );
+                        // Related files diagnostic
+                        handle_document_diagnostic_report_kind(editor, report.related_documents);
                     }
                     lsp::DocumentDiagnosticReport::Unchanged(report) => {
                         doc.previous_diagnostic_id =
                             Some(report.unchanged_document_diagnostic_report.result_id);
-                        handle_document_diagnostic_report_kind(
-                            editor,
-                            offset_encoding,
-                            server_id,
-                            report.related_documents,
-                        );
+                        handle_document_diagnostic_report_kind(editor, report.related_documents);
                     }
                 }
             }

helix-view/src/editor.rs

@@ -21,7 +21,7 @@ use tokio_stream::wrappers::UnboundedReceiverStream;
 use std::{
     borrow::Cow,
     cell::Cell,
-    collections::{BTreeMap, HashMap},
+    collections::{btree_map::Entry, BTreeMap, HashMap},
     io::stdin,
     num::NonZeroUsize,
     path::{Path, PathBuf},
@@ -1735,6 +1735,31 @@ impl Editor {
         .map(|_| ())
     }
 
+    /// Insert the original [`lsp::Diagnostic`] for later use in the diagnostics picker.
+    pub fn add_diagnostics(
+        &mut self,
+        diagnostics: Vec<lsp::Diagnostic>,
+        path: lsp::Url,
+        server_id: usize,
+    ) {
+        let mut diagnostics = diagnostics.into_iter().map(|d| (d, server_id)).collect();
+        match self.diagnostics.entry(path) {
+            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 != server_id);
+                current_diagnostics.append(&mut diagnostics);
+                // Sort diagnostics first by severity and then by line numbers.
+                // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
+                current_diagnostics.sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
+            }
+            Entry::Vacant(v) => {
+                diagnostics.sort_unstable_by_key(|(d, _)| (d.severity, d.range.start));
+                v.insert(diagnostics);
+            }
+        };
+    }
+
     pub async fn wait_event(&mut self) -> EditorEvent {
         // the loop only runs once or twice and would be better implemented with a recursion + const generic
         // however due to limitations with async functions that can not be implemented right now