TudbuT/helix-mods
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

LSP: Key diagnostics off file path instead of URI

Browse source 
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.
This commit is contained in:
Michael Davis 2023-08-27 13:32:17 -05:00 • committed by Blaž Hrastnik
parent dfa5382c51
commit 8141a4a1ab
3 changed files with 39 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
helix-term/src/application.rs
View file

@ -753,9 +753,7 @@ impl Application {
let lang_conf = doc.language.clone(); let lang_conf = doc.language.clone();
if let Some(lang_conf) = &lang_conf { if let Some(lang_conf) = &lang_conf {
if let Some(old_diagnostics) = if let Some(old_diagnostics) = self.editor.diagnostics.get(&path) {
self.editor.diagnostics.get(&params.uri)
{
if !lang_conf.persistent_diagnostic_sources.is_empty() { if !lang_conf.persistent_diagnostic_sources.is_empty() {
// Sort diagnostics first by severity and then by line numbers. // Sort diagnostics first by severity and then by line numbers.
// Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order
@ -788,7 +786,7 @@ impl Application {
// Insert the original lsp::Diagnostics here because we may have no open document // 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. // 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. // When using them later in the diagnostics picker, we calculate them on-demand.
let diagnostics = match self.editor.diagnostics.entry(params.uri) { let diagnostics = match self.editor.diagnostics.entry(path) {
Entry::Occupied(o) => { Entry::Occupied(o) => {
let current_diagnostics = o.into_mut(); let current_diagnostics = o.into_mut();
// there may entries of other language servers, which is why we can't overwrite the whole entry // there may entries of other language servers, which is why we can't overwrite the whole entry

64
helix-term/src/commands/lsp.rs
View file

@ -38,7 +38,7 @@ use std::{
collections::{BTreeMap, HashSet}, collections::{BTreeMap, HashSet},
fmt::Write, fmt::Write,
future::Future, future::Future,
path::PathBuf, path::{Path, PathBuf},
}; };
/// Gets the first language server that is attached to a document which supports a specific feature. /// Gets the first language server that is attached to a document which supports a specific feature.
@ -134,7 +134,7 @@ struct DiagnosticStyles {
} }
struct PickerDiagnostic { struct PickerDiagnostic {
url: lsp::Url, path: PathBuf,
diag: lsp::Diagnostic, diag: lsp::Diagnostic,
offset_encoding: OffsetEncoding, offset_encoding: OffsetEncoding,
} }
@ -167,8 +167,7 @@ impl ui::menu::Item for PickerDiagnostic {
let path = match format { let path = match format {
DiagnosticsFormat::HideSourcePath => String::new(), DiagnosticsFormat::HideSourcePath => String::new(),
DiagnosticsFormat::ShowSourcePath => { DiagnosticsFormat::ShowSourcePath => {
let file_path = self.url.to_file_path().unwrap(); let path = path::get_truncated_path(&self.path);
let path = path::get_truncated_path(file_path);
format!("{}: ", path.to_string_lossy()) format!("{}: ", path.to_string_lossy())
} }
}; };
@ -208,22 +207,31 @@ fn jump_to_location(
return; return;
} }
}; };
jump_to_position(editor, &path, location.range, offset_encoding, action);
}
let doc = match editor.open(&path, action) { fn jump_to_position(
editor: &mut Editor,
path: &Path,
range: lsp::Range,
offset_encoding: OffsetEncoding,
action: Action,
) {
let doc = match editor.open(path, action) {
Ok(id) => doc_mut!(editor, &id), Ok(id) => doc_mut!(editor, &id),
Err(err) => { Err(err) => {
let err = format!("failed to open path: {:?}: {:?}", location.uri, err); let err = format!("failed to open path: {:?}: {:?}", path, err);
editor.set_error(err); editor.set_error(err);
return; return;
} }
}; };
let view = view_mut!(editor); let view = view_mut!(editor);
// TODO: convert inside server // TODO: convert inside server
let new_range = let new_range = if let Some(new_range) = lsp_range_to_range(doc.text(), range, offset_encoding)
if let Some(new_range) = lsp_range_to_range(doc.text(), location.range, offset_encoding) { {
new_range new_range
} else { } else {
log::warn!("lsp position out of bounds - {:?}", location.range); log::warn!("lsp position out of bounds - {:?}", range);
return; return;
}; };
// we flip the range so that the cursor sits on the start of the symbol // we flip the range so that the cursor sits on the start of the symbol
@ -258,21 +266,20 @@ enum DiagnosticsFormat {
fn diag_picker( fn diag_picker(
cx: &Context, cx: &Context,
diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>, diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
_current_path: Option<lsp::Url>,
format: DiagnosticsFormat, format: DiagnosticsFormat,
) -> Picker<PickerDiagnostic> { ) -> Picker<PickerDiagnostic> {
// TODO: drop current_path comparison and instead use workspace: bool flag? // TODO: drop current_path comparison and instead use workspace: bool flag?
// flatten the map to a vec of (url, diag) pairs // flatten the map to a vec of (url, diag) pairs
let mut flat_diag = Vec::new(); let mut flat_diag = Vec::new();
for (url, diags) in diagnostics { for (path, diags) in diagnostics {
flat_diag.reserve(diags.len()); flat_diag.reserve(diags.len());
for (diag, ls) in diags { for (diag, ls) in diags {
if let Some(ls) = cx.editor.language_server_by_id(ls) { if let Some(ls) = cx.editor.language_server_by_id(ls) {
flat_diag.push(PickerDiagnostic { flat_diag.push(PickerDiagnostic {
url: url.clone(), path: path.clone(),
diag, diag,
offset_encoding: ls.offset_encoding(), offset_encoding: ls.offset_encoding(),
}); });
@ -292,22 +299,17 @@ fn diag_picker(
(styles, format), (styles, format),
move |cx, move |cx,
PickerDiagnostic { PickerDiagnostic {
url, path,
diag, diag,
offset_encoding, offset_encoding,
}, },
action| { action| {
jump_to_location( jump_to_position(cx.editor, path, diag.range, *offset_encoding, action)
cx.editor,
&lsp::Location::new(url.clone(), diag.range),
*offset_encoding,
action,
)
}, },
) )
.with_preview(move |_editor, PickerDiagnostic { url, diag, .. }| { .with_preview(move |_editor, PickerDiagnostic { path, diag, .. }| {
let location = lsp::Location::new(url.clone(), diag.range); let line = Some((diag.range.start.line as usize, diag.range.end.line as usize));
Some(location_to_file_location(&location)) Some((path.clone().into(), line))
}) })
.truncate_start(false) .truncate_start(false)
} }
@ -470,17 +472,16 @@ pub fn workspace_symbol_picker(cx: &mut Context) {
pub fn diagnostics_picker(cx: &mut Context) { pub fn diagnostics_picker(cx: &mut Context) {
let doc = doc!(cx.editor); let doc = doc!(cx.editor);
if let Some(current_url) = doc.url() { if let Some(current_path) = doc.path() {
let diagnostics = cx let diagnostics = cx
.editor .editor
.diagnostics .diagnostics
.get(&current_url) .get(current_path)
.cloned() .cloned()
.unwrap_or_default(); .unwrap_or_default();
let picker = diag_picker( let picker = diag_picker(
cx, cx,
[(current_url.clone(), diagnostics)].into(), [(current_path.clone(), diagnostics)].into(),
Some(current_url),
DiagnosticsFormat::HideSourcePath, DiagnosticsFormat::HideSourcePath,
); );
cx.push_layer(Box::new(overlaid(picker))); cx.push_layer(Box::new(overlaid(picker)));
@ -488,16 +489,9 @@ pub fn diagnostics_picker(cx: &mut Context) {
} }
pub fn workspace_diagnostics_picker(cx: &mut Context) { pub fn workspace_diagnostics_picker(cx: &mut Context) {
let doc = doc!(cx.editor);
let current_url = doc.url();
// TODO not yet filtered by LanguageServerFeature, need to do something similar as Document::shown_diagnostics here for all open documents // 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 diagnostics = cx.editor.diagnostics.clone();
let picker = diag_picker( let picker = diag_picker(cx, diagnostics, DiagnosticsFormat::ShowSourcePath);
cx,
diagnostics,
current_url,
DiagnosticsFormat::ShowSourcePath,
);
cx.push_layer(Box::new(overlaid(picker))); cx.push_layer(Box::new(overlaid(picker)));
} }

9
helix-view/src/editor.rs
View file

@ -914,7 +914,7 @@ pub struct Editor {
pub macro_recording: Option<(char, Vec<KeyEvent>)>, pub macro_recording: Option<(char, Vec<KeyEvent>)>,
pub macro_replaying: Vec<char>, pub macro_replaying: Vec<char>,
pub language_servers: helix_lsp::Registry, pub language_servers: helix_lsp::Registry,
pub diagnostics: BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>, pub diagnostics: BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
pub diff_providers: DiffProviderRegistry, pub diff_providers: DiffProviderRegistry,
pub debugger: Option<dap::Client>, pub debugger: Option<dap::Client>,
@ -1815,7 +1815,7 @@ impl Editor {
/// Returns all supported diagnostics for the document /// Returns all supported diagnostics for the document
pub fn doc_diagnostics<'a>( pub fn doc_diagnostics<'a>(
language_servers: &'a helix_lsp::Registry, language_servers: &'a helix_lsp::Registry,
diagnostics: &'a BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>, diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
document: &Document, document: &Document,
) -> impl Iterator<Item = helix_core::Diagnostic> + 'a { ) -> impl Iterator<Item = helix_core::Diagnostic> + 'a {
Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true) Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true)
@ -1825,7 +1825,7 @@ impl Editor {
/// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from
pub fn doc_diagnostics_with_filter<'a>( pub fn doc_diagnostics_with_filter<'a>(
language_servers: &'a helix_lsp::Registry, language_servers: &'a helix_lsp::Registry,
diagnostics: &'a BTreeMap<lsp::Url, Vec<(lsp::Diagnostic, usize)>>, diagnostics: &'a BTreeMap<PathBuf, Vec<(lsp::Diagnostic, usize)>>,
document: &Document, document: &Document,
filter: impl Fn(&lsp::Diagnostic, usize) -> bool + 'a, filter: impl Fn(&lsp::Diagnostic, usize) -> bool + 'a,
@ -1834,8 +1834,7 @@ impl Editor {
let language_config = document.language.clone(); let language_config = document.language.clone();
document document
.path() .path()
.and_then(|path| url::Url::from_file_path(path).ok()) // TODO log error? .and_then(|path| diagnostics.get(path))
.and_then(|uri| diagnostics.get(&uri))
.map(|diags| { .map(|diags| {
diags.iter().filter_map(move |(diagnostic, lsp_id)| { diags.iter().filter_map(move |(diagnostic, lsp_id)| {
let ls = language_servers.get_by_id(*lsp_id)?; let ls = language_servers.get_by_id(*lsp_id)?;