From f06e53c4b49c09203cb3fbe669505dcf69a65015 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sat, 17 Jun 2023 16:15:27 -0500 Subject: [PATCH] LSP: Key diagnostics off file path instead of URI URIs need to be normalized to be comparable. For example a language server could send a URI for a path containing '+' as '%2B' but we might encode this in something like 'Document::url' as just '+'. We can normalize the URI straight into a PathBuf though since this is the only value we compare these diagnostics URIs against. This also covers edge-cases like windows drive letter capitalization. --- helix-term/src/application.rs | 2 +- helix-term/src/commands/lsp.rs | 41 ++++++++++++++-------------------- helix-view/src/editor.rs | 2 +- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 7e9684827a9cd..91bc0881fc8b2 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -867,7 +867,7 @@ impl Application { // 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. - match self.editor.diagnostics.entry(params.uri) { + match self.editor.diagnostics.entry(path) { 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 diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 8c3fd13b558cf..66dacc932133b 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -138,7 +138,7 @@ struct DiagnosticStyles { } struct PickerDiagnostic { - url: lsp::Url, + path: PathBuf, diag: lsp::Diagnostic, offset_encoding: OffsetEncoding, } @@ -171,8 +171,7 @@ impl ui::menu::Item for PickerDiagnostic { let path = match format { DiagnosticsFormat::HideSourcePath => String::new(), DiagnosticsFormat::ShowSourcePath => { - let file_path = self.url.to_file_path().unwrap(); - let path = path::get_truncated_path(file_path); + let path = path::get_truncated_path(&self.path); format!("{}: ", path.to_string_lossy()) } }; @@ -289,21 +288,21 @@ enum DiagnosticsFormat { fn diag_picker( cx: &Context, - diagnostics: BTreeMap>, - current_path: Option, + diagnostics: BTreeMap>, + current_path: Option, format: DiagnosticsFormat, ) -> FilePicker { // TODO: drop current_path comparison and instead use workspace: bool flag? // flatten the map to a vec of (url, diag) pairs let mut flat_diag = Vec::new(); - for (url, diags) in diagnostics { + for (path, diags) in diagnostics { flat_diag.reserve(diags.len()); for (diag, ls) in diags { if let Some(ls) = cx.editor.language_server_by_id(ls) { flat_diag.push(PickerDiagnostic { - url: url.clone(), + path: path.clone(), diag, offset_encoding: ls.offset_encoding(), }); @@ -323,16 +322,15 @@ fn diag_picker( (styles, format), move |cx, PickerDiagnostic { - url, + path, diag, offset_encoding, }, action| { - if current_path.as_ref() == Some(url) { + if current_path.as_ref() == Some(path) { let (view, doc) = current!(cx.editor); push_jump(view, doc); } else { - let path = url.to_file_path().unwrap(); cx.editor.open(&path, action).expect("editor.open failed"); } @@ -345,9 +343,9 @@ fn diag_picker( align_view(doc, view, Align::Center); } }, - move |_editor, PickerDiagnostic { url, diag, .. }| { - let location = lsp::Location::new(url.clone(), diag.range); - Some(location_to_file_location(&location)) + move |_editor, PickerDiagnostic { path, diag, .. }| { + let line = Some((diag.range.start.line as usize, diag.range.end.line as usize)); + Some((path.clone().into(), line)) }, ) .truncate_start(false) @@ -511,17 +509,12 @@ pub fn workspace_symbol_picker(cx: &mut Context) { pub fn diagnostics_picker(cx: &mut Context) { let doc = doc!(cx.editor); - if let Some(current_url) = doc.url() { - let diagnostics = cx - .editor - .diagnostics - .get(¤t_url) - .cloned() - .unwrap_or_default(); + if let Some(path) = doc.path() { + let diagnostics = cx.editor.diagnostics.get(path).cloned().unwrap_or_default(); let picker = diag_picker( cx, - [(current_url.clone(), diagnostics)].into(), - Some(current_url), + [(path.clone(), diagnostics)].into(), + Some(path.clone()), DiagnosticsFormat::HideSourcePath, ); cx.push_layer(Box::new(overlaid(picker))); @@ -530,13 +523,13 @@ pub fn diagnostics_picker(cx: &mut Context) { pub fn workspace_diagnostics_picker(cx: &mut Context) { let doc = doc!(cx.editor); - let current_url = doc.url(); + let current_path = doc.path(); // TODO not yet filtered by LanguageServerFeature, need to do something similar as Document::shown_diagnostics here for all open documents let diagnostics = cx.editor.diagnostics.clone(); let picker = diag_picker( cx, diagnostics, - current_url, + current_path.cloned(), DiagnosticsFormat::ShowSourcePath, ); cx.push_layer(Box::new(overlaid(picker))); diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 1a884c324e18d..4d125dbdb90c3 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -878,7 +878,7 @@ pub struct Editor { pub macro_recording: Option<(char, Vec)>, pub macro_replaying: Vec, pub language_servers: helix_lsp::Registry, - pub diagnostics: BTreeMap>, + pub diagnostics: BTreeMap>, pub diff_providers: DiffProviderRegistry, pub debugger: Option,