From e0fc9c02ad876dd184c61a880a2b7f24071e5b7f Mon Sep 17 00:00:00 2001 From: Sofus Addington Date: Wed, 24 Jul 2024 15:16:25 +0200 Subject: [PATCH 1/3] Parse unchanged sources --- helix-term/src/application.rs | 76 +++++++++++++++++++--------------- helix-term/src/commands/lsp.rs | 41 +++++++++++++----- 2 files changed, 74 insertions(+), 43 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 60bc5b7cf..4b7c03141 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -15,7 +15,7 @@ use helix_view::{ graphics::Rect, theme, tree::Layout, - Align, Editor, + Align, Document, Editor, }; use serde_json::json; use tui::backend::Backend; @@ -735,7 +735,7 @@ impl Application { )); } } - Notification::PublishDiagnostics(mut params) => { + Notification::PublishDiagnostics(params) => { let uri = match helix_core::Uri::try_from(params.uri) { Ok(uri) => uri, Err(err) => { @@ -761,41 +761,24 @@ impl Application { true }); + let diagnostics = params + .diagnostics + .into_iter() + .map(|d| (d, server_id)) + .collect(); + let mut unchanged_diag_sources = Vec::new(); if let Some(doc) = &doc { - let lang_conf = doc.language.clone(); - - if let Some(lang_conf) = &lang_conf { - if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) { - if !lang_conf.persistent_diagnostic_sources.is_empty() { - // Sort diagnostics first by severity and then by line numbers. - // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order - params - .diagnostics - .sort_by_key(|d| (d.severity, d.range.start)); - } - for source in &lang_conf.persistent_diagnostic_sources { - let new_diagnostics = params - .diagnostics - .iter() - .filter(|d| d.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.eq(old_diagnostics) { - unchanged_diag_sources.push(source.clone()) - } - } - } + if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) { + unchanged_diag_sources = get_unchanged_diagnostic_sources( + doc, + &diagnostics, + old_diagnostics, + server_id, + ); } } - let diagnostics = params.diagnostics.into_iter().map(|d| (d, 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. @@ -808,7 +791,7 @@ impl Application { current_diagnostics // Sort diagnostics first by severity and then by line numbers. } - Entry::Vacant(v) => v.insert(diagnostics.collect()), + Entry::Vacant(v) => v.insert(diagnostics), }; // Sort diagnostics first by severity and then by line numbers. @@ -1255,3 +1238,30 @@ impl Application { errs } } + +pub fn get_unchanged_diagnostic_sources( + doc: &Document, + diagnostics: &Vec<(lsp::Diagnostic, LanguageServerId)>, + old_diagnostics: &Vec<(lsp::Diagnostic, LanguageServerId)>, + server_id: LanguageServerId, +) -> Vec { + 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 +} diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index c443021ad..84ad1364a 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -2,8 +2,8 @@ use futures_util::{stream::FuturesOrdered, FutureExt}; use helix_lsp::{ block_on, lsp::{ - self, CodeAction, CodeActionOrCommand, CodeActionTriggerKind, DiagnosticSeverity, - NumberOrString, + self, CodeAction, CodeActionOrCommand, CodeActionTriggerKind, Diagnostic, + DiagnosticSeverity, NumberOrString, }, util::{diagnostic_to_lsp_diagnostic, lsp_range_to_range, range_to_lsp_range}, Client, LanguageServerId, OffsetEncoding, @@ -26,6 +26,7 @@ use helix_view::{ }; use crate::{ + application::get_unchanged_diagnostic_sources, compositor::{self, Compositor}, job::Callback, ui::{self, overlay::overlaid, FileLocation, Picker, Popup, PromptEvent}, @@ -36,7 +37,7 @@ use std::{ collections::{btree_map::Entry, BTreeMap, HashMap, HashSet}, fmt::Write, future::Future, - path::Path, + path::{Path, PathBuf}, }; /// Gets the first language server that is attached to a document which supports a specific feature. @@ -1459,17 +1460,25 @@ pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::J let server_id = language_server.id(); let parse_diagnostic = |editor: &mut Editor, - path, + path: PathBuf, report: Vec, result_id: Option| { + // TODO: No clone and no unwrap + let uri = helix_core::Uri::try_from(path.clone()).unwrap(); + let mut diagnostics: Vec<(Diagnostic, LanguageServerId)> = + report.into_iter().map(|d| (d, server_id)).collect(); + + // TODO: Not clone? + let old_diagnostics = editor.diagnostics.get(&uri).cloned(); + if let Some(doc) = editor.document_by_path_mut(&path) { - let diagnostics: Vec = report + let new_diagnostics: Vec = diagnostics .iter() .map(|d| { Document::lsp_diagnostic_to_diagnostic( doc.text(), doc.language_config(), - d, + &d.0, server_id, offset_encoding, ) @@ -1478,13 +1487,25 @@ pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::J .collect(); doc.previous_diagnostic_id = result_id; - // TODO: Should i get unchanged_sources? - doc.replace_diagnostics(diagnostics, &[], Some(server_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), + ); } - let uri = helix_core::Uri::try_from(path).unwrap(); // TODO: Maybe share code with application.rs:802 - let mut diagnostics = report.into_iter().map(|d| (d, server_id)).collect(); match editor.diagnostics.entry(uri) { Entry::Occupied(o) => { let current_diagnostics = o.into_mut(); From 6541153cb4826d12084acb7035e5a27dd85d191d Mon Sep 17 00:00:00 2001 From: Sofus Addington Date: Thu, 25 Jul 2024 12:50:10 +0200 Subject: [PATCH 2/3] Remove comments --- helix-term/src/commands/lsp.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 84ad1364a..da7315f5d 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1463,12 +1463,10 @@ pub fn pull_diagnostic_for_current_doc(editor: &Editor, jobs: &mut crate::job::J path: PathBuf, report: Vec, result_id: Option| { - // TODO: No clone and no unwrap let uri = helix_core::Uri::try_from(path.clone()).unwrap(); let mut diagnostics: Vec<(Diagnostic, LanguageServerId)> = report.into_iter().map(|d| (d, server_id)).collect(); - // TODO: Not clone? let old_diagnostics = editor.diagnostics.get(&uri).cloned(); if let Some(doc) = editor.document_by_path_mut(&path) { From 009f414c62cabf91b8882dabe47f86e1adf5fa62 Mon Sep 17 00:00:00 2001 From: Sofus Addington Date: Fri, 26 Jul 2024 09:03:11 +0200 Subject: [PATCH 3/3] Fix lint issue --- helix-term/src/application.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 4b7c03141..cfc2db21b 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -761,7 +761,7 @@ impl Application { true }); - let diagnostics = params + let diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)> = params .diagnostics .into_iter() .map(|d| (d, server_id)) @@ -1241,8 +1241,8 @@ impl Application { pub fn get_unchanged_diagnostic_sources( doc: &Document, - diagnostics: &Vec<(lsp::Diagnostic, LanguageServerId)>, - old_diagnostics: &Vec<(lsp::Diagnostic, LanguageServerId)>, + diagnostics: &[(lsp::Diagnostic, LanguageServerId)], + old_diagnostics: &[(lsp::Diagnostic, LanguageServerId)], server_id: LanguageServerId, ) -> Vec { let mut unchanged_diag_sources = Vec::new();