Fix sorting issues of the editor wide diagnostics and apply diagnostics related review suggestions

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>

3 files changed, 22 insertions, 18 deletions

diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index e159cb83..728aa46a 100644
--- a/helix-term/src/application.rs
+++ b/helix-term/src/application.rs
@@ -721,7 +721,7 @@ impl Application {
                             ));
                         }
                     }
-                    Notification::PublishDiagnostics(mut params) => {
+                    Notification::PublishDiagnostics(params) => {
                         let path = match params.uri.to_file_path() {
                             Ok(path) => path,
                             Err(_) => {
@@ -841,15 +841,10 @@ impl Application {
                             doc.replace_diagnostics(diagnostics, server_id);
                         }
 
-                        // Sort diagnostics first by severity and then by line numbers.
-                        // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
-                        params
-                            .diagnostics
-                            .sort_unstable_by_key(|d| (d.severity, d.range.start));
-                        let diagnostics = params
+                        let mut diagnostics = params
                             .diagnostics
                             .into_iter()
-                            .map(|d| (d, server_id, offset_encoding))
+                            .map(|d| (d, server_id))
                             .collect();
 
                         // Insert the original lsp::Diagnostics here because we may have no open document
@@ -859,10 +854,16 @@ impl Application {
                             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.extend(diagnostics);
+                                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);
                             }
                         };
diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index efef1211..1a1233a9 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -262,7 +262,7 @@ enum DiagnosticsFormat {
 
 fn diag_picker(
     cx: &Context,
-    diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize, OffsetEncoding)>>,
+    diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
     current_path: Option<lsp::Url>,
     format: DiagnosticsFormat,
 ) -> FilePicker<PickerDiagnostic> {
@@ -272,12 +272,15 @@ fn diag_picker(
     let mut flat_diag = Vec::new();
     for (url, diags) in diagnostics {
         flat_diag.reserve(diags.len());
-        for (diag, _, offset_encoding) in diags {
-            flat_diag.push(PickerDiagnostic {
-                url: url.clone(),
-                diag,
-                offset_encoding,
-            });
+
+        for (diag, ls) in diags {
+            if let Some(ls) = cx.editor.language_servers.get_by_id(ls) {
+                flat_diag.push(PickerDiagnostic {
+                    url: url.clone(),
+                    diag,
+                    offset_encoding: ls.offset_encoding(),
+                });
+            }
         }
     }
 
diff --git a/helix-term/src/ui/statusline.rs b/helix-term/src/ui/statusline.rs
index b10e8076..60997956 100644
--- a/helix-term/src/ui/statusline.rs
+++ b/helix-term/src/ui/statusline.rs
@@ -266,7 +266,7 @@ where
             .diagnostics
             .values()
             .flatten()
-            .fold((0, 0), |mut counts, (diag, _, _)| {
+            .fold((0, 0), |mut counts, (diag, _)| {
                 match diag.severity {
                     Some(DiagnosticSeverity::WARNING) => counts.0 += 1,
                     Some(DiagnosticSeverity::ERROR) | None => counts.1 += 1,