From f9b08656f41cbb9573ffb144f5dc2e24ea764ac9 Mon Sep 17 00:00:00 2001
From: Philipp Mildenberger <philipp@mildenberger.me>
Date: Fri, 17 Mar 2023 15:30:49 +0100
Subject: [PATCH] Fix sorting issues of the editor wide diagnostics and apply
 diagnostics related review suggestions

Co-authored-by: Pascal Kuthe <pascal.kuthe@semimod.de>
---
 helix-term/src/application.rs   | 21 +++++++++++----------
 helix-term/src/commands/lsp.rs  | 17 ++++++++++-------
 helix-term/src/ui/statusline.rs |  2 +-
 helix-view/src/editor.rs        |  2 +-
 4 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs
index e159cb836..728aa46a3 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 efef1211c..1a1233a9b 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 b10e8076d..609979562 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,
diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs
index 697d4459d..2bd48af8f 100644
--- a/helix-view/src/editor.rs
+++ b/helix-view/src/editor.rs
@@ -818,7 +818,7 @@ pub struct Editor {
     pub macro_recording: Option<(char, Vec<KeyEvent>)>,
     pub macro_replaying: Vec<char>,
     pub language_servers: helix_lsp::Registry,
-    pub diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize, OffsetEncoding)>>,
+    pub diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>,
     pub diff_providers: DiffProviderRegistry,
 
     pub debugger: Option<dap::Client>,