Refactor and share code with publish diagnostics

helix-term/src/application.rs

@@ -11,11 +11,10 @@
     align_view,
     document::{DocumentOpenError, DocumentSavedEventResult},
     editor::{ConfigEvent, EditorEvent},
-    events::DiagnosticsDidChange,
     graphics::Rect,
     theme,
     tree::Layout,
-    Align, Document, Editor,
+    Align, Editor,
 };
 use serde_json::json;
 use tui::backend::Backend;
@@ -33,7 +32,7 @@
 use log::{debug, error, info, warn};
 #[cfg(not(feature = "integration"))]
 use std::io::stdout;
-use std::{collections::btree_map::Entry, io::stdin, path::Path, sync::Arc};
+use std::{io::stdin, path::Path, sync::Arc};
 
 #[cfg(not(windows))]
 use anyhow::Context;
@@ -750,18 +749,6 @@ macro_rules! language_server {
                             log::error!("Discarding publishDiagnostic notification sent by an uninitialized server: {}", language_server.name());
                             return;
                         }
-                        // have to inline the function because of borrow checking...
-                        let doc = self.editor.documents.values_mut()
-                            .find(|doc| doc.uri().is_some_and(|u| u == uri))
-                            .filter(|doc| {
-                                if let Some(version) = params.version {
-                                    if version != doc.version() {
-                                        log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
-                                        return false;
-                                    }
-                                }
-                                true
-                            });
 
                         let diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)> = params
                             .diagnostics
@@ -769,64 +756,13 @@ macro_rules! language_server {
                             .map(|d| (d, server_id))
                             .collect();
 
-                        let mut unchanged_diag_sources = Vec::new();
+                        self.editor.add_diagnostics(
-                        if let Some(doc) = &doc {
+                            diagnostics,
-                            if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) {
+                            server_id,
-                                unchanged_diag_sources = get_unchanged_diagnostic_sources(
+                            uri,
-                                    doc,
+                            params.version,
-                                    &diagnostics,
+                            None,
-                                    old_diagnostics,
+                        );
-                                    server_id,
-                                );
-                            }
-                        }
-
-                        // 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.
-                        let diagnostics = match self.editor.diagnostics.entry(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.extend(diagnostics);
-                                current_diagnostics
-                                // Sort diagnostics first by severity and then by line numbers.
-                            }
-                            Entry::Vacant(v) => v.insert(diagnostics),
-                        };
-
-                        // 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));
-
-                        if let Some(doc) = doc {
-                            let diagnostic_of_language_server_and_not_in_unchanged_sources =
-                                |diagnostic: &lsp::Diagnostic, ls_id| {
-                                    ls_id == server_id
-                                        && diagnostic.source.as_ref().map_or(true, |source| {
-                                            !unchanged_diag_sources.contains(source)
-                                        })
-                                };
-                            let diagnostics = Editor::doc_diagnostics_with_filter(
-                                &self.editor.language_servers,
-                                &self.editor.diagnostics,
-                                doc,
-                                diagnostic_of_language_server_and_not_in_unchanged_sources,
-                            );
-                            doc.replace_diagnostics(
-                                diagnostics,
-                                &unchanged_diag_sources,
-                                Some(server_id),
-                            );
-
-                            let doc = doc.id();
-                            helix_event::dispatch(DiagnosticsDidChange {
-                                editor: &mut self.editor,
-                                doc,
-                            });
-                        }
                     }
                     Notification::ShowMessage(params) => {
                         log::warn!("unhandled window/showMessage: {:?}", params);
@@ -1240,30 +1176,3 @@ pub async fn close(&mut self) -> Vec<anyhow::Error> {
         errs
     }
 }
-
-pub fn get_unchanged_diagnostic_sources(
-    doc: &Document,
-    diagnostics: &[(lsp::Diagnostic, LanguageServerId)],
-    old_diagnostics: &[(lsp::Diagnostic, LanguageServerId)],
-    server_id: LanguageServerId,
-) -> Vec<String> {
-    let mut unchanged_diag_sources = Vec::new();
-    let lang_conf = doc.language.clone();
-
-    if let Some(lang_conf) = &lang_conf {
-        for source in &lang_conf.persistent_diagnostic_sources {
-            let new_diagnostics = diagnostics
-                .iter()
-                .filter(|d| d.0.source.as_ref() == Some(source));
-            let old_diagnostics = old_diagnostics
-                .iter()
-                .filter(|(d, d_server)| *d_server == server_id && d.source.as_ref() == Some(source))
-                .map(|(d, _)| d);
-            if new_diagnostics.map(|x| &x.0).eq(old_diagnostics) {
-                unchanged_diag_sources.push(source.clone())
-            }
-        }
-    }
-
-    unchanged_diag_sources
-}

helix-term/src/commands/lsp.rs

@@ -26,7 +26,6 @@
 };
 
 use crate::{
-    application::get_unchanged_diagnostic_sources,
     compositor::{self, Compositor},
     job::Callback,
     ui::{self, overlay::overlaid, FileLocation, Picker, Popup, PromptEvent},
@@ -34,7 +33,7 @@
 
 use std::{
     cmp::Ordering,
-    collections::{btree_map::Entry, BTreeMap, HashMap, HashSet},
+    collections::{BTreeMap, HashMap, HashSet},
     fmt::{Display, Write},
     future::Future,
     path::{Path, PathBuf},
@@ -1447,79 +1446,17 @@ pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::J
     let callback = super::make_job_callback(
         future.expect("safety: language server supports pull diagnostics"),
         move |editor, _compositor, response: Option<lsp::DocumentDiagnosticReport>| {
-            let doc = match editor.document_by_path_mut(&original_path) {
-                Some(doc) => doc,
-                None => return,
-            };
-            let Some(language_server) = doc.language_servers().find(|ls| ls.id() == server_id)
-            else {
-                return;
-            };
-            // Pass them separately to satisfy borrow-checker
-            let offset_encoding = language_server.offset_encoding();
-            let server_id = language_server.id();
-
             let parse_diagnostic = |editor: &mut Editor,
                                     path: PathBuf,
                                     report: Vec<lsp::Diagnostic>,
                                     result_id: Option<String>| {
-                let uri = helix_core::Uri::try_from(path.clone()).unwrap();
+                let uri = helix_core::Uri::try_from(path);
-                let mut diagnostics: Vec<(Diagnostic, LanguageServerId)> =
+                let diagnostics: Vec<(Diagnostic, LanguageServerId)> =
                     report.into_iter().map(|d| (d, server_id)).collect();
 
-                let old_diagnostics = editor.diagnostics.get(&uri).cloned();
+                if let Ok(uri) = uri {
-
+                    editor.add_diagnostics(diagnostics, server_id, uri, None, result_id);
-                if let Some(doc) = editor.document_by_path_mut(&path) {
-                    let new_diagnostics: Vec<helix_core::Diagnostic> = diagnostics
-                        .iter()
-                        .map(|d| {
-                            Document::lsp_diagnostic_to_diagnostic(
-                                doc.text(),
-                                doc.language_config(),
-                                &d.0,
-                                server_id,
-                                offset_encoding,
-                            )
-                            .unwrap()
-                        })
-                        .collect();
-
-                    doc.previous_diagnostic_id = result_id;
-
-                    let mut unchanged_diag_sources = Vec::new();
-                    if let Some(old_diagnostics) = old_diagnostics {
-                        unchanged_diag_sources = get_unchanged_diagnostic_sources(
-                            doc,
-                            &diagnostics,
-                            &old_diagnostics,
-                            server_id,
-                        );
-                    }
-
-                    doc.replace_diagnostics(
-                        new_diagnostics,
-                        &unchanged_diag_sources,
-                        Some(server_id),
-                    );
                 }
-
-                // TODO: Maybe share code with application.rs:802
-                match editor.diagnostics.entry(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);
-                    }
-                };
             };
 
             let handle_document_diagnostic_report_kind = |editor: &mut Editor,
@@ -1543,6 +1480,10 @@ pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::J
             };
 
             if let Some(response) = response {
+                let doc = match editor.document_by_path_mut(&original_path) {
+                    Some(doc) => doc,
+                    None => return,
+                };
                 match response {
                     lsp::DocumentDiagnosticReport::Full(report) => {
                         // Original file diagnostic

helix-view/src/editor.rs

@@ -2154,6 +2154,92 @@ pub fn get_synced_view_id(&mut self, id: DocumentId) -> ViewId {
             current_view.id
         }
     }
+
+    pub fn add_diagnostics(
+        &mut self,
+        diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)>,
+        server_id: LanguageServerId,
+        uri: helix_core::Uri,
+        document_version: Option<i32>,
+        result_id: Option<String>,
+    ) {
+        let doc = self.documents.values_mut()
+                            .find(|doc| doc.uri().is_some_and(|u| u == uri))
+                            .filter(|doc| {
+                                if let Some(version) = document_version{
+                                    if version != doc.version() {
+                                        log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version());
+                                        return false;
+                                    }
+                                }
+                                true
+                            });
+
+        if let Some(doc) = doc {
+            let mut unchanged_diag_sources = Vec::new();
+            if let Some(old_diagnostics) = self.diagnostics.get(&uri) {
+                if let Some(lang_conf) = doc.language_config() {
+                    for source in &lang_conf.persistent_diagnostic_sources {
+                        let new_diagnostics = diagnostics
+                            .iter()
+                            .filter(|d| d.0.source.as_ref() == Some(source));
+                        let old_diagnostics = old_diagnostics
+                            .iter()
+                            .filter(|(d, d_server)| {
+                                *d_server == server_id && d.source.as_ref() == Some(source)
+                            })
+                            .map(|(d, _)| d);
+                        if new_diagnostics.map(|x| &x.0).eq(old_diagnostics) {
+                            unchanged_diag_sources.push(source.clone())
+                        }
+                    }
+                }
+            }
+
+            // 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.
+            let diagnostics = match self.diagnostics.entry(uri) {
+                std::collections::btree_map::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
+                    // Sort diagnostics first by severity and then by line numbers.
+                }
+                std::collections::btree_map::Entry::Vacant(v) => v.insert(diagnostics),
+            };
+
+            // 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));
+
+            let diagnostic_of_language_server_and_not_in_unchanged_sources =
+                |diagnostic: &lsp::Diagnostic, ls_id| {
+                    ls_id == server_id
+                        && diagnostic
+                            .source
+                            .as_ref()
+                            .map_or(true, |source| !unchanged_diag_sources.contains(source))
+                };
+            let diagnostics = Editor::doc_diagnostics_with_filter(
+                &self.language_servers,
+                &self.diagnostics,
+                doc,
+                diagnostic_of_language_server_and_not_in_unchanged_sources,
+            );
+            doc.replace_diagnostics(diagnostics, &unchanged_diag_sources, Some(server_id));
+
+            if result_id.is_some() {
+                doc.previous_diagnostic_id = result_id;
+            }
+
+            let doc = doc.id();
+
+            helix_event::dispatch(crate::events::DiagnosticsDidChange { editor: self, doc });
+        }
+    }
 }
 
 fn try_restore_indent(doc: &mut Document, view: &mut View) {