Refactored conversion of diagnostics into editor and removed Document…

…::shown_diagnostics

helix-core/src/syntax.rs

@@ -1793,7 +1793,6 @@ impl HighlightConfiguration {
                 let mut best_index = None;
                 let mut best_match_len = 0;
                 for (i, recognized_name) in recognized_names.iter().enumerate() {
-                    let recognized_name = recognized_name;
                     let mut len = 0;
                     let mut matches = true;
                     for (i, part) in recognized_name.split('.').enumerate() {

helix-term/src/application.rs

@@ -737,20 +737,29 @@ impl Application {
                             true
                         });
 
+                        // Due to borrow-checking issues `Editor::doc_diagnostics` is not easily possible here
                         if let Some(doc) = doc {
-                            let diagnostics = params
-                                .diagnostics
-                                .iter()
-                                .filter_map(|diagnostic| {
-                                    doc.lsp_diagnostic_to_diagnostic(
-                                        diagnostic,
-                                        server_id,
-                                        offset_encoding,
-                                    )
-                                })
-                                .collect();
-
-                            doc.replace_diagnostics(diagnostics, server_id);
+                            let supports_diagnostics = doc
+                                .language_servers_with_feature(
+                                    syntax::LanguageServerFeature::Diagnostics,
+                                )
+                                .any(|l| l.id() == server_id);
+                            if supports_diagnostics {
+                                let text = doc.text().clone();
+                                let config = doc.language.clone();
+                                let diagnostics =
+                                    params.diagnostics.iter().filter_map(|diagnostic| {
+                                        helix_view::Document::lsp_diagnostic_to_diagnostic(
+                                            &text,
+                                            config.as_deref(),
+                                            diagnostic,
+                                            server_id,
+                                            offset_encoding,
+                                        )
+                                    });
+
+                                doc.replace_diagnostics(diagnostics, Some(server_id));
+                            }
                         }
 
                         let mut diagnostics = params

helix-term/src/commands.rs

@@ -3350,7 +3350,7 @@ fn exit_select_mode(cx: &mut Context) {
 
 fn goto_first_diag(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
-    let selection = match doc.shown_diagnostics().next() {
+    let selection = match doc.diagnostics().first() {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
         None => return,
     };
@@ -3359,7 +3359,7 @@ fn goto_first_diag(cx: &mut Context) {
 
 fn goto_last_diag(cx: &mut Context) {
     let (view, doc) = current!(cx.editor);
-    let selection = match doc.shown_diagnostics().last() {
+    let selection = match doc.diagnostics().last() {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
         None => return,
     };
@@ -3375,9 +3375,10 @@ fn goto_next_diag(cx: &mut Context) {
         .cursor(doc.text().slice(..));
 
     let diag = doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .find(|diag| diag.range.start > cursor_pos)
-        .or_else(|| doc.shown_diagnostics().next());
+        .or_else(|| doc.diagnostics().first());
 
     let selection = match diag {
         Some(diag) => Selection::single(diag.range.start, diag.range.end),
@@ -3395,10 +3396,11 @@ fn goto_prev_diag(cx: &mut Context) {
         .cursor(doc.text().slice(..));
 
     let diag = doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .rev()
         .find(|diag| diag.range.start < cursor_pos)
-        .or_else(|| doc.shown_diagnostics().last());
+        .or_else(|| doc.diagnostics().last());
 
     let selection = match diag {
         // NOTE: the selection is reversed because we're jumping to the
@@ -4185,9 +4187,13 @@ fn replace_with_yanked(cx: &mut Context) {
 }
 
 fn replace_with_yanked_impl(editor: &mut Editor, register: char, count: usize) {
-    let Some(values) = editor.registers
+    let Some(values) = editor
+        .registers
         .read(register, editor)
-        .filter(|values| values.len() > 0) else { return };
+        .filter(|values| values.len() > 0)
+    else {
+        return;
+    };
     let values: Vec<_> = values.map(|value| value.to_string()).collect();
 
     let (view, doc) = current!(editor);
@@ -4224,7 +4230,9 @@ fn replace_selections_with_primary_clipboard(cx: &mut Context) {
 }
 
 fn paste(editor: &mut Editor, register: char, pos: Paste, count: usize) {
-    let Some(values) = editor.registers.read(register, editor) else { return };
+    let Some(values) = editor.registers.read(register, editor) else {
+        return;
+    };
     let values: Vec<_> = values.map(|value| value.to_string()).collect();
 
     let (view, doc) = current!(editor);

helix-term/src/ui/editor.rs

@@ -386,7 +386,7 @@ impl EditorView {
         let mut warning_vec = Vec::new();
         let mut error_vec = Vec::new();
 
-        for diagnostic in doc.shown_diagnostics() {
+        for diagnostic in doc.diagnostics() {
             // Separate diagnostics into different Vecs by severity.
             let (vec, scope) = match diagnostic.severity {
                 Some(Severity::Info) => (&mut info_vec, info),
@@ -684,7 +684,7 @@ impl EditorView {
             .primary()
             .cursor(doc.text().slice(..));
 
-        let diagnostics = doc.shown_diagnostics().filter(|diagnostic| {
+        let diagnostics = doc.diagnostics().iter().filter(|diagnostic| {
             diagnostic.range.start <= cursor && diagnostic.range.end >= cursor
         });
 

helix-term/src/ui/picker.rs

@@ -427,8 +427,12 @@ impl<T: Item + 'static> Picker<T> {
                             (size, _) if size > MAX_FILE_SIZE_FOR_PREVIEW => {
                                 CachedPreview::LargeFile
                             }
-                            _ => Document::open(path, None, None, editor)
-                                .map(|doc| CachedPreview::Document(Box::new(doc)))
+                            _ => Document::open(path, None, None, editor.config.clone())
+                                .map(|mut doc| {
+                                    let diagnostics = editor.doc_diagnostics(&doc, |_, _| true);
+                                    doc.replace_diagnostics(diagnostics, None);
+                                    CachedPreview::Document(Box::new(doc))
+                                })
                                 .unwrap_or(CachedPreview::NotFound),
                         },
                     )

helix-term/src/ui/statusline.rs

@@ -227,7 +227,8 @@ where
 {
     let (warnings, errors) = context
         .doc
-        .shown_diagnostics()
+        .diagnostics()
+        .iter()
         .fold((0, 0), |mut counts, diag| {
             use helix_core::diagnostic::Severity;
             match diag.severity {

helix-view/src/document.rs

@@ -693,20 +693,20 @@ impl Document {
         path: &Path,
         encoding: Option<&'static Encoding>,
         config_loader: Option<Arc<syntax::Loader>>,
-        editor: &Editor,
+        config: Arc<dyn DynAccess<Config>>,
     ) -> Result<Self, Error> {
         // Open the file if it exists, otherwise assume it is a new file (and thus empty).
         let (rope, encoding, has_bom) = if path.exists() {
             let mut file =
                 std::fs::File::open(path).context(format!("unable to open {:?}", path))?;
             from_reader(&mut file, encoding)?
         } else {
-            let line_ending: LineEnding = editor.config.load().default_line_ending.into();
+            let line_ending: LineEnding = config.load().default_line_ending.into();
             let encoding = encoding.unwrap_or(encoding::UTF_8);
             (Rope::from(line_ending.as_str()), encoding, false)
         };
 
-        let mut doc = Self::from(rope, Some((encoding, has_bom)), editor.config.clone());
+        let mut doc = Self::from(rope, Some((encoding, has_bom)), config);
 
         // set the path and try detecting the language
         doc.set_path(Some(path));
@@ -716,24 +716,6 @@ impl Document {
 
         doc.detect_indent_and_line_ending();
 
-        let diagnostics = Url::from_file_path(path) // TODO log error?
-            .ok()
-            .and_then(|uri| editor.diagnostics.get(&uri))
-            .map(|diags| {
-                diags
-                    .iter()
-                    .filter_map(|(diagnostic, lsp_id)| {
-                        let offset_encoding =
-                            editor.language_server_by_id(*lsp_id)?.offset_encoding();
-                        doc.lsp_diagnostic_to_diagnostic(diagnostic, *lsp_id, offset_encoding)
-                    })
-                    .collect::<Vec<_>>()
-            });
-
-        if let Some(diagnostics) = diagnostics {
-            doc.diagnostics = diagnostics;
-        }
-
         Ok(doc)
     }
 
@@ -1712,16 +1694,14 @@ impl Document {
     }
 
     pub fn lsp_diagnostic_to_diagnostic(
-        &self,
+        text: &Rope,
+        language_config: Option<&LanguageConfiguration>,
         diagnostic: &helix_lsp::lsp::Diagnostic,
         language_server_id: usize,
         offset_encoding: helix_lsp::OffsetEncoding,
     ) -> Option<Diagnostic> {
         use helix_core::diagnostic::{Range, Severity::*};
 
-        let lang_conf = self.language_config();
-        let text = self.text();
-
         // TODO: convert inside server
         let start =
             if let Some(start) = lsp_pos_to_pos(text, diagnostic.range.start, offset_encoding) {
@@ -1746,7 +1726,7 @@ impl Document {
             severity => unreachable!("unrecognized diagnostic severity: {:?}", severity),
         });
 
-        if let Some(lang_conf) = lang_conf {
+        if let Some(lang_conf) = language_config {
             if let Some(severity) = severity {
                 if severity < lang_conf.diagnostic_severity {
                     return None;
@@ -1796,24 +1776,20 @@ impl Document {
         &self.diagnostics
     }
 
-    pub fn shown_diagnostics(&self) -> impl Iterator<Item = &Diagnostic> + DoubleEndedIterator {
-        self.diagnostics.iter().filter(|d| {
-            self.language_servers.len() == 0
-                || self
-                    .language_servers_with_feature(LanguageServerFeature::Diagnostics)
-                    .any(|ls| ls.id() == d.language_server_id)
-        })
-    }
-
     pub fn replace_diagnostics(
         &mut self,
-        mut diagnostics: Vec<Diagnostic>,
-        language_server_id: usize,
+        diagnostics: impl Iterator<Item = Diagnostic>,
+        language_server_id: Option<usize>,
     ) {
-        self.clear_diagnostics(language_server_id);
-        self.diagnostics.append(&mut diagnostics);
+        if let Some(id) = language_server_id {
+            self.clear_diagnostics(id);
+        } else {
+            self.diagnostics.clear();
+        }
+
+        self.diagnostics.extend(diagnostics);
         self.diagnostics
-            .sort_unstable_by_key(|diagnostic| diagnostic.range);
+            .sort_by_key(|diagnostic| (diagnostic.range.start, diagnostic.severity));
     }
 
     pub fn clear_diagnostics(&mut self, language_server_id: usize) {

helix-view/src/editor.rs

@@ -42,7 +42,7 @@ use anyhow::{anyhow, bail, Error};
 pub use helix_core::diagnostic::Severity;
 use helix_core::{
     auto_pairs::AutoPairs,
-    syntax::{self, AutoPairConfig, IndentationHeuristic, SoftWrap},
+    syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap},
     Change, LineEnding, NATIVE_LINE_ENDING,
 };
 use helix_core::{Position, Selection};
@@ -1449,7 +1449,15 @@ impl Editor {
         let id = if let Some(id) = id {
             id
         } else {
-            let mut doc = Document::open(&path, None, Some(self.syn_loader.clone()), self)?;
+            let mut doc = Document::open(
+                &path,
+                None,
+                Some(self.syn_loader.clone()),
+                self.config.clone(),
+            )?;
+
+            let diagnostics = self.doc_diagnostics(&doc, |_, _| true);
+            doc.replace_diagnostics(diagnostics, None);
 
             if let Some(diff_base) = self.diff_providers.get_diff_base(&path) {
                 doc.set_diff_base(diff_base);
@@ -1680,6 +1688,49 @@ impl Editor {
             .find(|doc| doc.path().map(|p| p == path.as_ref()).unwrap_or(false))
     }
 
+    /// Returns all supported diagnostics for the document
+    /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
+    pub fn doc_diagnostics<'a>(
+        &'a self,
+        document: &Document,
+        filter: impl Fn(&lsp::Diagnostic, usize) -> bool + 'a,
+    ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
+        let text = document.text().clone();
+        let language_config = document.language.clone();
+        document
+            .path()
+            .and_then(|path| url::Url::from_file_path(path).ok()) // TODO log error?
+            .and_then(|uri| self.diagnostics.get(&uri))
+            .map(|diags| {
+                diags.iter().filter_map(move |(diagnostic, lsp_id)| {
+                    let ls = self.language_server_by_id(*lsp_id)?;
+                    language_config
+                        .as_ref()
+                        .and_then(|c| {
+                            c.language_servers.iter().find(|features| {
+                                features.name == ls.name()
+                                    && features.has_feature(LanguageServerFeature::Diagnostics)
+                            })
+                        })
+                        .and_then(|_| {
+                            if filter(diagnostic, *lsp_id) {
+                                Document::lsp_diagnostic_to_diagnostic(
+                                    &text,
+                                    language_config.as_deref(),
+                                    diagnostic,
+                                    *lsp_id,
+                                    ls.offset_encoding(),
+                                )
+                            } else {
+                                None
+                            }
+                        })
+                })
+            })
+            .into_iter()
+            .flatten()
+    }
+
     /// Gets the primary cursor position in screen coordinates,
     /// or `None` if the primary cursor is not visible on screen.
     pub fn cursor(&self) -> (Option<Position>, CursorKind) {