From 13f048b3d797c1f43d881f7d5db7495459332863 Mon Sep 17 00:00:00 2001 From: Sofus Addington Date: Sat, 3 Aug 2024 22:40:09 +0200 Subject: [PATCH] Refactor and share code with publish diagnostics --- helix-term/src/application.rs | 109 +++------------------------------ helix-term/src/commands/lsp.rs | 77 +++-------------------- helix-view/src/editor.rs | 86 ++++++++++++++++++++++++++ 3 files changed, 104 insertions(+), 168 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index a46c0089f666..646a3397b59a 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -11,11 +11,10 @@ use helix_view::{ 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 crate::{ 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 @@ impl Application { 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 @@ impl Application { .map(|d| (d, server_id)) .collect(); - let mut unchanged_diag_sources = Vec::new(); - if let Some(doc) = &doc { - if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) { - unchanged_diag_sources = get_unchanged_diagnostic_sources( - doc, - &diagnostics, - 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, - }); - } + self.editor.add_diagnostics( + diagnostics, + server_id, + uri, + params.version, + None, + ); } Notification::ShowMessage(params) => { log::warn!("unhandled window/showMessage: {:?}", params); @@ -1240,30 +1176,3 @@ impl Application { errs } } - -pub fn get_unchanged_diagnostic_sources( - doc: &Document, - diagnostics: &[(lsp::Diagnostic, LanguageServerId)], - old_diagnostics: &[(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 86bdc40498ae..74cbd935bef8 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -26,7 +26,6 @@ use helix_view::{ }; 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 crate::{ 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| { - 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, result_id: Option| { - let uri = helix_core::Uri::try_from(path.clone()).unwrap(); - let mut diagnostics: Vec<(Diagnostic, LanguageServerId)> = + let uri = helix_core::Uri::try_from(path); + let diagnostics: Vec<(Diagnostic, LanguageServerId)> = report.into_iter().map(|d| (d, server_id)).collect(); - let old_diagnostics = editor.diagnostics.get(&uri).cloned(); - - if let Some(doc) = editor.document_by_path_mut(&path) { - let new_diagnostics: Vec = 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), - ); + if let Ok(uri) = uri { + editor.add_diagnostics(diagnostics, server_id, uri, None, result_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 diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 1708b3b4e053..691c762b59b5 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -2154,6 +2154,92 @@ impl Editor { current_view.id } } + + pub fn add_diagnostics( + &mut self, + diagnostics: Vec<(lsp::Diagnostic, LanguageServerId)>, + server_id: LanguageServerId, + uri: helix_core::Uri, + document_version: Option, + result_id: Option, + ) { + 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) {