From 7245c7fb4681140ac056ad1f65a7cb850f094dda Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Thu, 7 Apr 2022 23:59:42 +0200 Subject: [PATCH 01/18] Add workspace and document diagnostics picker fixes #1891 --- helix-term/src/application.rs | 12 ++-- helix-term/src/commands.rs | 9 ++- helix-term/src/commands/lsp.rs | 115 ++++++++++++++++++++++++++++++- helix-term/src/keymap/default.rs | 2 + helix-term/src/ui/mod.rs | 9 ++- helix-term/src/ui/picker.rs | 61 ++++++++-------- helix-tui/src/text.rs | 5 +- helix-view/src/editor.rs | 3 + 8 files changed, 172 insertions(+), 44 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index bc5f3bd774f2..6819cf323de8 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -420,6 +420,13 @@ impl Application { } } Notification::PublishDiagnostics(params) => { + // 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. + self.editor + .diagnostics + .insert(params.uri.clone(), params.diagnostics.clone()); + let path = params.uri.to_file_path().unwrap(); let doc = self.editor.document_by_path_mut(&path); @@ -431,10 +438,7 @@ impl Application { .diagnostics .into_iter() .filter_map(|diagnostic| { - use helix_core::{ - diagnostic::{Range, Severity::*}, - Diagnostic, - }; + use helix_core::diagnostic::{Diagnostic, Range, Severity::*}; use lsp::DiagnosticSeverity; let language_server = doc.language_server().unwrap(); diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index c7eb46ab10cb..47599576d6f5 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -4,6 +4,7 @@ pub(crate) mod typed; pub use dap::*; pub use lsp::*; +use tui::text::Spans; pub use typed::*; use helix_core::{ @@ -263,6 +264,8 @@ impl MappableCommand { buffer_picker, "Open buffer picker", symbol_picker, "Open symbol picker", workspace_symbol_picker, "Open workspace symbol picker", + diagnostics_picker, "Open diagnostic picker", + workspace_diagnostics_picker, "Open workspace diagnostic picker", last_picker, "Open last picker", prepend_to_line, "Insert at start of line", append_to_line, "Insert at end of line", @@ -2058,7 +2061,7 @@ fn buffer_picker(cx: &mut Context) { } impl BufferMeta { - fn format(&self) -> Cow { + fn format(&self) -> Spans { let path = self .path .as_deref() @@ -2081,7 +2084,7 @@ fn buffer_picker(cx: &mut Context) { } else { format!(" ({})", flags.join("")) }; - Cow::Owned(format!("{} {}{}", self.id, path, flag)) + format!("{} {}{}", self.id, path, flag).into() } } @@ -2152,7 +2155,7 @@ pub fn command_palette(cx: &mut Context) { MappableCommand::Typable { doc, name, .. } => match keymap.get(name as &String) { Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(), - None => doc.into(), + None => doc.clone().into(), }, MappableCommand::Static { doc, name, .. } => match keymap.get(*name) { Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(), diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 1db57ecf264d..38e1c09483dd 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1,20 +1,25 @@ use helix_lsp::{ - block_on, lsp, + block_on, + lsp::{self, DiagnosticSeverity, Location, NumberOrString}, util::{lsp_pos_to_pos, lsp_range_to_range, range_to_lsp_range}, OffsetEncoding, }; +use tui::text::{Span, Spans}; use super::{align_view, push_jump, Align, Context, Editor}; use helix_core::Selection; -use helix_view::editor::Action; +use helix_view::{ + editor::Action, + theme::{Modifier, Style}, +}; use crate::{ compositor::{self, Compositor}, ui::{self, overlay::overlayed, FileLocation, FilePicker, Popup, PromptEvent}, }; -use std::borrow::Cow; +use std::{borrow::Cow, collections::BTreeMap}; #[macro_export] macro_rules! language_server { @@ -108,6 +113,83 @@ fn sym_picker( .truncate_start(false) } +fn diag_picker( + cx: &Context, + diagnostics: BTreeMap>, + current_path: Option, + offset_encoding: OffsetEncoding, +) -> FilePicker<(lsp::Url, lsp::Diagnostic)> { + // TODO: drop current_path comparison and instead use workspace: bool flag? + + // flatten the map to a vec of (uri, diag) pairs + let mut flat_diag = Vec::new(); + for (u, diags) in diagnostics { + for d in diags { + flat_diag.push((u.clone(), d)); + } + } + + let theme = cx.editor.theme.clone(); + + FilePicker::new( + flat_diag, + move |(_url, d)| { + let mut style = d.severity.map_or(Style::default(), |s| match s { + DiagnosticSeverity::HINT => theme.get("hint"), + DiagnosticSeverity::INFORMATION => theme.get("info"), + DiagnosticSeverity::WARNING => theme.get("warning"), + DiagnosticSeverity::ERROR => theme.get("error"), + _ => Style::default(), + }); + + // remove background as it is distracting in the picker list + style.bg = None; + + let code = d + .code + .as_ref() + .map(|c| match c { + NumberOrString::Number(n) => n.to_string(), + NumberOrString::String(s) => s.to_string(), + }) + .unwrap_or_default(); + + Spans::from(vec![ + Span::styled( + d.source.clone().unwrap_or_default(), + style.add_modifier(Modifier::BOLD), + ), + Span::raw(": "), + Span::styled(code, style), + Span::raw(" - "), + Span::styled(&d.message, style), + ]) + }, + move |cx, (url, d), action| { + if current_path.as_ref() == Some(url) { + push_jump(cx.editor); + } else { + let path = url.to_file_path().unwrap(); + cx.editor.open(path, action).expect("editor.open failed"); + } + + let (view, doc) = current!(cx.editor); + + if let Some(range) = lsp_range_to_range(doc.text(), d.range, offset_encoding) { + // we flip the range so that the cursor sits on the start of the symbol + // (for example start of the function). + doc.set_selection(view.id, Selection::single(range.head, range.anchor)); + align_view(doc, view, Align::Center); + } + }, + move |_editor, (url, diag)| { + let location = Location::new(url.clone(), diag.range); + Some(location_to_file_location(&location)) + }, + ) + .truncate_start(false) +} + pub fn symbol_picker(cx: &mut Context) { fn nested_to_flat( list: &mut Vec, @@ -178,6 +260,33 @@ pub fn workspace_symbol_picker(cx: &mut Context) { ) } +pub fn diagnostics_picker(cx: &mut Context) { + let doc = doc!(cx.editor); + let language_server = language_server!(cx.editor, doc); + if let Some(current_url) = doc.url() { + let offset_encoding = language_server.offset_encoding(); + let diagnostics = cx + .editor + .diagnostics + .clone() + .into_iter() + .filter(|(u, _)| *u == current_url) + .collect(); + let picker = diag_picker(cx, diagnostics, Some(current_url), offset_encoding); + cx.push_layer(Box::new(overlayed(picker))); + } +} + +pub fn workspace_diagnostics_picker(cx: &mut Context) { + let doc = doc!(cx.editor); + let language_server = language_server!(cx.editor, doc); + let current_url = doc.url(); + let offset_encoding = language_server.offset_encoding(); + let diagnostics = cx.editor.diagnostics.clone(); + let picker = diag_picker(cx, diagnostics, current_url, offset_encoding); + cx.push_layer(Box::new(overlayed(picker))); +} + impl ui::menu::Item for lsp::CodeActionOrCommand { fn label(&self) -> &str { match self { diff --git a/helix-term/src/keymap/default.rs b/helix-term/src/keymap/default.rs index a8144ebc2655..ef69e14f75b1 100644 --- a/helix-term/src/keymap/default.rs +++ b/helix-term/src/keymap/default.rs @@ -199,6 +199,8 @@ pub fn default() -> HashMap { "b" => buffer_picker, "s" => symbol_picker, "S" => workspace_symbol_picker, + "g" => diagnostics_picker, + "G" => workspace_diagnostics_picker, "a" => code_action, "'" => last_picker, "d" => { "Debug (experimental)" sticky=true diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 2dca870baa42..8b9f391e5f12 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -23,6 +23,8 @@ pub use text::Text; use helix_core::regex::Regex; use helix_core::regex::RegexBuilder; use helix_view::{Document, Editor, View}; +use tui; +use tui::text::Spans; use std::path::PathBuf; @@ -177,7 +179,12 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi files, move |path: &PathBuf| { // format_fn - path.strip_prefix(&root).unwrap_or(path).to_string_lossy() + Spans::from( + path.strip_prefix(&root) + .unwrap_or(path) + .to_string_lossy() + .into_owned(), + ) }, move |cx, path: &PathBuf, action| { cx.editor diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index dec59c892478..b3bba0151a66 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -6,6 +6,7 @@ use crate::{ use crossterm::event::Event; use tui::{ buffer::Buffer as Surface, + text::Spans, widgets::{Block, BorderType, Borders}, }; @@ -15,7 +16,6 @@ use tui::widgets::Widget; use std::time::Instant; use std::{ - borrow::Cow, cmp::Reverse, collections::HashMap, io::Read, @@ -87,7 +87,7 @@ impl Preview<'_, '_> { impl FilePicker { pub fn new( options: Vec, - format_fn: impl Fn(&T) -> Cow + 'static, + format_fn: impl Fn(&T) -> Spans + 'static, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, preview_fn: impl Fn(&Editor, &T) -> Option + 'static, ) -> Self { @@ -301,14 +301,14 @@ pub struct Picker { /// Whether to truncate the start (default true) pub truncate_start: bool, - format_fn: Box Cow>, + format_fn: Box Spans>, callback_fn: Box, } impl Picker { pub fn new( options: Vec, - format_fn: impl Fn(&T) -> Cow + 'static, + format_fn: impl Fn(&T) -> Spans + 'static, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { let prompt = Prompt::new( @@ -373,9 +373,8 @@ impl Picker { self.matches.retain_mut(|(index, score)| { let option = &self.options[*index]; // TODO: maybe using format_fn isn't the best idea here - let text = (self.format_fn)(option); - - match self.matcher.fuzzy_match(&text, pattern) { + let line: String = (self.format_fn)(option).into(); + match self.matcher.fuzzy_match(&line, pattern) { Some(s) => { // Update the score *score = s; @@ -402,10 +401,10 @@ impl Picker { } // TODO: maybe using format_fn isn't the best idea here - let text = (self.format_fn)(option); + let line: String = (self.format_fn)(option).into(); self.matcher - .fuzzy_match(&text, pattern) + .fuzzy_match(&line, pattern) .map(|score| (index, score)) }), ); @@ -612,30 +611,34 @@ impl Component for Picker { surface.set_string(inner.x.saturating_sub(2), inner.y + i as u16, ">", selected); } - let formatted = (self.format_fn)(option); - + let spans = (self.format_fn)(option); let (_score, highlights) = self .matcher - .fuzzy_indices(&formatted, self.prompt.line()) + .fuzzy_indices(&String::from(spans.clone()), self.prompt.line()) .unwrap_or_default(); - surface.set_string_truncated( - inner.x, - inner.y + i as u16, - &formatted, - inner.width as usize, - |idx| { - if highlights.contains(&idx) { - highlighted - } else if is_active { - selected - } else { - text_style - } - }, - true, - self.truncate_start, - ); + spans.0.into_iter().fold(inner, |pos, span| { + let new_x = surface + .set_string_truncated( + pos.x, + pos.y + i as u16, + &span.content, + pos.width as usize, + |idx| { + if highlights.contains(&idx) { + highlighted.patch(span.style) + } else if is_active { + selected.patch(span.style) + } else { + text_style.patch(span.style) + } + }, + true, + self.truncate_start, + ) + .0; + pos.clip_left(new_x - pos.x) + }); } } diff --git a/helix-tui/src/text.rs b/helix-tui/src/text.rs index 8a974ddba465..56440ecea8a9 100644 --- a/helix-tui/src/text.rs +++ b/helix-tui/src/text.rs @@ -243,10 +243,7 @@ impl<'a> From> for Spans<'a> { impl<'a> From> for String { fn from(line: Spans<'a>) -> String { - line.0.iter().fold(String::new(), |mut acc, s| { - acc.push_str(s.content.as_ref()); - acc - }) + line.0.iter().map(|s| &*s.content).collect() } } diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index c4e9ec283f71..66c240bd2f3f 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -39,6 +39,7 @@ use helix_core::{ }; use helix_core::{Position, Selection}; use helix_dap as dap; +use helix_lsp::lsp; use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer}; @@ -309,6 +310,7 @@ pub struct Editor { pub macro_recording: Option<(char, Vec)>, pub theme: Theme, pub language_servers: helix_lsp::Registry, + pub diagnostics: BTreeMap>, pub debugger: Option, pub debugger_events: SelectAll>, @@ -379,6 +381,7 @@ impl Editor { macro_recording: None, theme: theme_loader.default(), language_servers, + diagnostics: BTreeMap::new(), debugger: None, debugger_events: SelectAll::new(), breakpoints: HashMap::new(), From b4c5fdd41a9a86c738b4641b6209d7a7ba17a990 Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Thu, 14 Apr 2022 10:19:52 +0200 Subject: [PATCH 02/18] Fix some of @archseer's annotations --- helix-term/src/commands.rs | 5 ++--- helix-term/src/commands/lsp.rs | 37 ++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b8dfaf59004a..6c0e0092c867 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2159,10 +2159,9 @@ pub fn command_palette(cx: &mut Context) { let picker = Picker::new( commands, move |command| match command { - MappableCommand::Typable { doc, name, .. } => match keymap.get(name as &String) - { + MappableCommand::Typable { doc, name, .. } => match keymap.get(name) { Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(), - None => doc.clone().into(), + None => doc.as_str().into(), }, MappableCommand::Static { doc, name, .. } => match keymap.get(*name) { Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(), diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 38e1c09483dd..deb70b570da9 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -124,23 +124,30 @@ fn diag_picker( // flatten the map to a vec of (uri, diag) pairs let mut flat_diag = Vec::new(); for (u, diags) in diagnostics { + flat_diag.reserve(diags.len()); for d in diags { flat_diag.push((u.clone(), d)); } } - let theme = cx.editor.theme.clone(); + let hint = cx.editor.theme.get("hint"); + let info = cx.editor.theme.get("info"); + let warning = cx.editor.theme.get("warning"); + let error = cx.editor.theme.get("error"); FilePicker::new( flat_diag, move |(_url, d)| { - let mut style = d.severity.map_or(Style::default(), |s| match s { - DiagnosticSeverity::HINT => theme.get("hint"), - DiagnosticSeverity::INFORMATION => theme.get("info"), - DiagnosticSeverity::WARNING => theme.get("warning"), - DiagnosticSeverity::ERROR => theme.get("error"), - _ => Style::default(), - }); + let mut style = d + .severity + .map(|s| match s { + DiagnosticSeverity::HINT => hint, + DiagnosticSeverity::INFORMATION => info, + DiagnosticSeverity::WARNING => warning, + DiagnosticSeverity::ERROR => error, + _ => Style::default(), + }) + .unwrap_or_default(); // remove background as it is distracting in the picker list style.bg = None; @@ -268,11 +275,15 @@ pub fn diagnostics_picker(cx: &mut Context) { let diagnostics = cx .editor .diagnostics - .clone() - .into_iter() - .filter(|(u, _)| *u == current_url) - .collect(); - let picker = diag_picker(cx, diagnostics, Some(current_url), offset_encoding); + .get(¤t_url) + .cloned() + .unwrap_or_default(); + let picker = diag_picker( + cx, + [(current_url.clone(), diagnostics)].into(), + Some(current_url), + offset_encoding, + ); cx.push_layer(Box::new(overlayed(picker))); } } From e164409c3cb851939dc00106ee99f91b0c645d67 Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Thu, 14 Apr 2022 15:54:05 +0200 Subject: [PATCH 03/18] Add From<&Spans> impl for String --- helix-term/src/ui/picker.rs | 2 +- helix-tui/src/text.rs | 6 ++++++ helix-tui/src/widgets/block.rs | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index b3bba0151a66..d197c6381e93 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -614,7 +614,7 @@ impl Component for Picker { let spans = (self.format_fn)(option); let (_score, highlights) = self .matcher - .fuzzy_indices(&String::from(spans.clone()), self.prompt.line()) + .fuzzy_indices(&String::from(&spans), self.prompt.line()) .unwrap_or_default(); spans.0.into_iter().fold(inner, |pos, span| { diff --git a/helix-tui/src/text.rs b/helix-tui/src/text.rs index 56440ecea8a9..edb806ba0676 100644 --- a/helix-tui/src/text.rs +++ b/helix-tui/src/text.rs @@ -247,6 +247,12 @@ impl<'a> From> for String { } } +impl<'a> From<&Spans<'a>> for String { + fn from(line: &Spans<'a>) -> String { + line.0.iter().map(|s| &*s.content).collect() + } +} + /// A string split over multiple lines where each line is composed of several clusters, each with /// their own style. /// diff --git a/helix-tui/src/widgets/block.rs b/helix-tui/src/widgets/block.rs index 26223c3ebbb1..a3ab40e15ab2 100644 --- a/helix-tui/src/widgets/block.rs +++ b/helix-tui/src/widgets/block.rs @@ -82,7 +82,7 @@ impl<'a> Block<'a> { )] pub fn title_style(mut self, style: Style) -> Block<'a> { if let Some(t) = self.title { - let title = String::from(t); + let title = String::from(&t); self.title = Some(Spans::from(Span::styled(title, style))); } self From 29208081653b8e7e0b91b40987a91c08414097ff Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Thu, 14 Apr 2022 18:36:42 +0200 Subject: [PATCH 04/18] More descriptive parameter names. --- helix-term/src/commands/lsp.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index deb70b570da9..61d5b5d8f64e 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -137,8 +137,8 @@ fn diag_picker( FilePicker::new( flat_diag, - move |(_url, d)| { - let mut style = d + move |(_, diag)| { + let mut style = diag .severity .map(|s| match s { DiagnosticSeverity::HINT => hint, @@ -152,7 +152,7 @@ fn diag_picker( // remove background as it is distracting in the picker list style.bg = None; - let code = d + let code = diag .code .as_ref() .map(|c| match c { @@ -163,16 +163,16 @@ fn diag_picker( Spans::from(vec![ Span::styled( - d.source.clone().unwrap_or_default(), + diag.source.clone().unwrap_or_default(), style.add_modifier(Modifier::BOLD), ), Span::raw(": "), Span::styled(code, style), Span::raw(" - "), - Span::styled(&d.message, style), + Span::styled(&diag.message, style), ]) }, - move |cx, (url, d), action| { + move |cx, (url, diag), action| { if current_path.as_ref() == Some(url) { push_jump(cx.editor); } else { @@ -182,7 +182,7 @@ fn diag_picker( let (view, doc) = current!(cx.editor); - if let Some(range) = lsp_range_to_range(doc.text(), d.range, offset_encoding) { + if let Some(range) = lsp_range_to_range(doc.text(), diag.range, offset_encoding) { // we flip the range so that the cursor sits on the start of the symbol // (for example start of the function). doc.set_selection(view.id, Selection::single(range.head, range.anchor)); From 6f87babf165f0450f2d6b4da722e5c72d9897107 Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Tue, 19 Apr 2022 09:41:50 +0200 Subject: [PATCH 05/18] Adding From> impls for Span and Spans --- helix-term/src/ui/mod.rs | 7 +------ helix-tui/src/text.rs | 12 ++++++++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 8b9f391e5f12..0c46a4cad890 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -179,12 +179,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi files, move |path: &PathBuf| { // format_fn - Spans::from( - path.strip_prefix(&root) - .unwrap_or(path) - .to_string_lossy() - .into_owned(), - ) + Spans::from(path.strip_prefix(&root).unwrap_or(path).to_string_lossy()) }, move |cx, path: &PathBuf, action| { cx.editor diff --git a/helix-tui/src/text.rs b/helix-tui/src/text.rs index edb806ba0676..b4278c864b30 100644 --- a/helix-tui/src/text.rs +++ b/helix-tui/src/text.rs @@ -194,6 +194,12 @@ impl<'a> From<&'a str> for Span<'a> { } } +impl<'a> From> for Span<'a> { + fn from(s: Cow<'a, str>) -> Span<'a> { + Span::raw(s) + } +} + /// A string composed of clusters of graphemes, each with their own style. #[derive(Debug, Default, Clone, PartialEq)] pub struct Spans<'a>(pub Vec>); @@ -229,6 +235,12 @@ impl<'a> From<&'a str> for Spans<'a> { } } +impl<'a> From> for Spans<'a> { + fn from(s: Cow<'a, str>) -> Spans<'a> { + Spans(vec![Span::raw(s)]) + } +} + impl<'a> From>> for Spans<'a> { fn from(spans: Vec>) -> Spans<'a> { Spans(spans) From 4819226349c93e568edd1d88aba8010181bff87f Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Thu, 28 Apr 2022 13:48:18 +0200 Subject: [PATCH 06/18] Add new keymap entries to docs --- book/src/keymap.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/book/src/keymap.md b/book/src/keymap.md index 2abe4f74a653..7bbbbe1beab1 100644 --- a/book/src/keymap.md +++ b/book/src/keymap.md @@ -236,6 +236,8 @@ This layer is a kludge of mappings, mostly pickers. | `k` | Show documentation for item under cursor in a [popup](#popup) (**LSP**) | `hover` | | `s` | Open document symbol picker (**LSP**) | `symbol_picker` | | `S` | Open workspace symbol picker (**LSP**) | `workspace_symbol_picker` | +| `g` | Open document diagnosics picker (**LSP**) | `diagnostics_picker` | +| `G` | Open workspace diagnostics picker (**LSP**) | `workspace_diagnosics_picker` | `r` | Rename symbol (**LSP**) | `rename_symbol` | | `a` | Apply code action (**LSP**) | `code_action` | | `'` | Open last fuzzy picker | `last_picker` | From 2bfc8f96af9dc405c5eebbc11b4908cb0548b843 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 3 May 2022 17:02:29 +0200 Subject: [PATCH 07/18] Avoid some clones --- helix-term/src/application.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index d5d3d0ad9864..0bf4153f07a9 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -439,13 +439,6 @@ impl Application { } } Notification::PublishDiagnostics(params) => { - // 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. - self.editor - .diagnostics - .insert(params.uri.clone(), params.diagnostics.clone()); - let path = params.uri.to_file_path().unwrap(); let doc = self.editor.document_by_path_mut(&path); @@ -455,7 +448,7 @@ impl Application { let diagnostics = params .diagnostics - .into_iter() + .iter() .filter_map(|diagnostic| { use helix_core::diagnostic::{Diagnostic, Range, Severity::*}; use lsp::DiagnosticSeverity; @@ -508,7 +501,7 @@ impl Application { Some(Diagnostic { range: Range { start, end }, line: diagnostic.range.start.line as usize, - message: diagnostic.message, + message: diagnostic.message.clone(), severity, // code // source @@ -517,6 +510,13 @@ impl Application { .collect(); doc.set_diagnostics(diagnostics); + + // 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. + self.editor + .diagnostics + .insert(params.uri, params.diagnostics); } } Notification::ShowMessage(params) => { From 19aa61581c6f74c7ef1ebb6b4bddfaa981ae155c Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Tue, 14 Jun 2022 01:58:40 +0200 Subject: [PATCH 08/18] Fix api change --- helix-term/src/commands/lsp.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 64ef4f972912..470a9914f526 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -204,7 +204,8 @@ fn diag_picker( }, move |cx, (url, diag), action| { if current_path.as_ref() == Some(url) { - push_jump(cx.editor); + 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"); From 9d9aeaf8ab65903ed707560dc321da01f61c1e83 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 14 Jun 2022 11:13:48 +0200 Subject: [PATCH 09/18] Update helix-term/src/application.rs Co-authored-by: Bjorn Ove Hay Andersen --- helix-term/src/application.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index ace995dbdb38..86848fee7ca4 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -516,14 +516,13 @@ impl Application { .collect(); doc.set_diagnostics(diagnostics); - - // 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. - self.editor - .diagnostics - .insert(params.uri, params.diagnostics); } + // 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. + self.editor + .diagnostics + .insert(params.uri, params.diagnostics); } Notification::ShowMessage(params) => { log::warn!("unhandled window/showMessage: {:?}", params); From 77405473a1578a8720bb1b7a15390527264604a5 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 14 Jun 2022 11:25:18 +0200 Subject: [PATCH 10/18] Fix a clippy hint --- xtask/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xtask/src/main.rs b/xtask/src/main.rs index a4c69d701166..f66fb4f4237a 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -106,7 +106,7 @@ pub mod md_gen { .collect::>() .join(", "); - let doc = cmd.doc.replace("\n", "
"); + let doc = cmd.doc.replace('\n', "
"); md.push_str(&md_table_row(&[names.to_owned(), doc.to_owned()])); } From 38ccf7e2ab58b59967dc1f2649ba15e048439e97 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 14 Jun 2022 15:34:49 +0200 Subject: [PATCH 11/18] Sort diagnostics first by URL and then by severity. --- helix-term/src/application.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 86848fee7ca4..d0ed222b0bea 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -444,7 +444,7 @@ impl Application { )); } } - Notification::PublishDiagnostics(params) => { + Notification::PublishDiagnostics(mut params) => { let path = params.uri.to_file_path().unwrap(); let doc = self.editor.document_by_path_mut(&path); @@ -520,6 +520,14 @@ 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. + params.diagnostics.sort_unstable_by(|a, b| { + if let (Some(a), Some(b)) = (a.severity, b.severity) { + a.partial_cmp(&b).unwrap() + } else { + unreachable!("unrecognized diagnostic severity") + } + }); + self.editor .diagnostics .insert(params.uri, params.diagnostics); From 7301d05675d1495de4aa83fef18d5e934bec3680 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 14 Jun 2022 15:34:49 +0200 Subject: [PATCH 12/18] Sort diagnostics first by URL and then by severity. --- helix-term/src/application.rs | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 86848fee7ca4..8dacf8d69552 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -444,7 +444,7 @@ impl Application { )); } } - Notification::PublishDiagnostics(params) => { + Notification::PublishDiagnostics(mut params) => { let path = params.uri.to_file_path().unwrap(); let doc = self.editor.document_by_path_mut(&path); @@ -517,6 +517,17 @@ impl Application { doc.set_diagnostics(diagnostics); } + + // Sort diagnostics first by URL and then by severity. + // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order + params.diagnostics.sort_unstable_by(|a, b| { + if let (Some(a), Some(b)) = (a.severity, b.severity) { + a.partial_cmp(&b).unwrap() + } else { + unreachable!("unrecognized diagnostic severity") + } + }); + // 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. From 154e985570cdfe04e6a0e0418aa58f4ba11ffd6d Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Wed, 15 Jun 2022 07:57:09 +0200 Subject: [PATCH 13/18] Ignore missing lsp severity entries --- helix-term/src/application.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 89c1d37aa0d5..51bca7c2cdde 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -524,21 +524,13 @@ impl Application { if let (Some(a), Some(b)) = (a.severity, b.severity) { a.partial_cmp(&b).unwrap() } else { - unreachable!("unrecognized diagnostic severity") + std::cmp::Ordering::Equal } }); // 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. - params.diagnostics.sort_unstable_by(|a, b| { - if let (Some(a), Some(b)) = (a.severity, b.severity) { - a.partial_cmp(&b).unwrap() - } else { - unreachable!("unrecognized diagnostic severity") - } - }); - self.editor .diagnostics .insert(params.uri, params.diagnostics); From 627bb095e90b3837e51b5090c6e98cbf93b762e3 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 21 Jun 2022 17:11:29 +0200 Subject: [PATCH 14/18] Add truncated filepath --- helix-core/src/path.rs | 42 ++++++++++++++++++++++++++++++++++ helix-term/src/commands/lsp.rs | 12 +++++++--- 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index cb50e136c23e..2da7ec54b3de 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -90,3 +90,45 @@ pub fn get_relative_path(path: &Path) -> PathBuf { }; fold_home_dir(path) } + +/// Returns a truncated filepath where the basepart of the path is reduced to the first +/// char of the folder and the whole filename appended. +/// +/// Note that this function does not check if the truncated path if unambiguous. +pub fn get_truncated_path>(path: P) -> PathBuf { + let file = path.as_ref().file_name().unwrap_or_default(); + let base = path.as_ref().parent().unwrap_or_else(|| Path::new("")); + let mut ret = PathBuf::new(); + for d in base { + ret.push( + d.to_string_lossy() + .chars() + .next() + .unwrap_or_default() + .to_string(), + ); + } + ret.push(file); + ret +} + +#[test] +fn get_truncated_path_test() { + assert_eq!( + get_truncated_path("/home/cnorris/documents/jokes.txt").as_path(), + Path::new("/h/c/d/jokes.txt") + ); + assert_eq!( + get_truncated_path("jokes.txt").as_path(), + Path::new("jokes.txt") + ); + assert_eq!( + get_truncated_path("/jokes.txt").as_path(), + Path::new("/jokes.txt") + ); + assert_eq!( + get_truncated_path("/h/c/d/jokes.txt").as_path(), + Path::new("/h/c/d/jokes.txt") + ); + assert_eq!(get_truncated_path("").as_path(), Path::new("")); +} diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 470a9914f526..fb881f00368c 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -8,7 +8,7 @@ use tui::text::{Span, Spans}; use super::{align_view, push_jump, Align, Context, Editor}; -use helix_core::Selection; +use helix_core::{path, Selection}; use helix_view::{ editor::Action, theme::{Modifier, Style}, @@ -167,7 +167,7 @@ fn diag_picker( FilePicker::new( flat_diag, - move |(_, diag)| { + move |(url, diag)| { let mut style = diag .severity .map(|s| match s { @@ -191,14 +191,20 @@ fn diag_picker( }) .unwrap_or_default(); + let truncated_path = path::get_truncated_path(url.as_str()) + .to_string_lossy() + .into_owned(); + Spans::from(vec![ Span::styled( diag.source.clone().unwrap_or_default(), style.add_modifier(Modifier::BOLD), ), Span::raw(": "), - Span::styled(code, style), + Span::styled(truncated_path, style), Span::raw(" - "), + Span::styled(code, style.add_modifier(Modifier::BOLD)), + Span::raw(": "), Span::styled(&diag.message, style), ]) }, From 45ff7fd44d3e438980a54ccaca1653b02d8ed695 Mon Sep 17 00:00:00 2001 From: Falco Hirschenberger Date: Tue, 21 Jun 2022 17:15:52 +0200 Subject: [PATCH 15/18] Typo --- helix-core/src/path.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index 2da7ec54b3de..586d141d6618 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -94,7 +94,7 @@ pub fn get_relative_path(path: &Path) -> PathBuf { /// Returns a truncated filepath where the basepart of the path is reduced to the first /// char of the folder and the whole filename appended. /// -/// Note that this function does not check if the truncated path if unambiguous. +/// Note that this function does not check if the truncated path is unambiguous. pub fn get_truncated_path>(path: P) -> PathBuf { let file = path.as_ref().file_name().unwrap_or_default(); let base = path.as_ref().parent().unwrap_or_else(|| Path::new("")); From 50e67c9aa029208f2369d58270175da92f8b9e3f Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Wed, 22 Jun 2022 08:59:00 +0200 Subject: [PATCH 16/18] Strip cwd from paths and use url-path without schema --- helix-core/src/path.rs | 10 ++++++++-- helix-term/src/commands/lsp.rs | 4 ++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index 586d141d6618..449604c0d76e 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -94,10 +94,16 @@ pub fn get_relative_path(path: &Path) -> PathBuf { /// Returns a truncated filepath where the basepart of the path is reduced to the first /// char of the folder and the whole filename appended. /// +/// Also strip the current working directory from the beginning of the path. /// Note that this function does not check if the truncated path is unambiguous. pub fn get_truncated_path>(path: P) -> PathBuf { - let file = path.as_ref().file_name().unwrap_or_default(); - let base = path.as_ref().parent().unwrap_or_else(|| Path::new("")); + let cwd = std::env::current_dir().unwrap_or_default(); + let path = path + .as_ref() + .strip_prefix(cwd) + .unwrap_or_else(|_| path.as_ref()); + let file = path.file_name().unwrap_or_default(); + let base = path.parent().unwrap_or_else(|| Path::new("")); let mut ret = PathBuf::new(); for d in base { ret.push( diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 3ea5955d057a..e3e512323193 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -198,7 +198,7 @@ fn diag_picker( }) .unwrap_or_default(); - let truncated_path = path::get_truncated_path(url.as_str()) + let truncated_path = path::get_truncated_path(url.path()) .to_string_lossy() .into_owned(); @@ -221,7 +221,7 @@ fn diag_picker( push_jump(view, doc); } else { let path = url.to_file_path().unwrap(); - cx.editor.open(path, action).expect("editor.open failed"); + cx.editor.open(&path, action).expect("editor.open failed"); } let (view, doc) = current!(cx.editor); From 00be70bcd3c4cbb7bc2323be90ed5195a41e97a2 Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Wed, 22 Jun 2022 11:45:36 +0200 Subject: [PATCH 17/18] Make tests a doctest --- helix-core/src/path.rs | 45 ++++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/helix-core/src/path.rs b/helix-core/src/path.rs index 449604c0d76e..d59a6baad604 100644 --- a/helix-core/src/path.rs +++ b/helix-core/src/path.rs @@ -96,6 +96,30 @@ pub fn get_relative_path(path: &Path) -> PathBuf { /// /// Also strip the current working directory from the beginning of the path. /// Note that this function does not check if the truncated path is unambiguous. +/// +/// ``` +/// use helix_core::path::get_truncated_path; +/// use std::path::Path; +/// +/// assert_eq!( +/// get_truncated_path("/home/cnorris/documents/jokes.txt").as_path(), +/// Path::new("/h/c/d/jokes.txt") +/// ); +/// assert_eq!( +/// get_truncated_path("jokes.txt").as_path(), +/// Path::new("jokes.txt") +/// ); +/// assert_eq!( +/// get_truncated_path("/jokes.txt").as_path(), +/// Path::new("/jokes.txt") +/// ); +/// assert_eq!( +/// get_truncated_path("/h/c/d/jokes.txt").as_path(), +/// Path::new("/h/c/d/jokes.txt") +/// ); +/// assert_eq!(get_truncated_path("").as_path(), Path::new("")); +/// ``` +/// pub fn get_truncated_path>(path: P) -> PathBuf { let cwd = std::env::current_dir().unwrap_or_default(); let path = path @@ -117,24 +141,3 @@ pub fn get_truncated_path>(path: P) -> PathBuf { ret.push(file); ret } - -#[test] -fn get_truncated_path_test() { - assert_eq!( - get_truncated_path("/home/cnorris/documents/jokes.txt").as_path(), - Path::new("/h/c/d/jokes.txt") - ); - assert_eq!( - get_truncated_path("jokes.txt").as_path(), - Path::new("jokes.txt") - ); - assert_eq!( - get_truncated_path("/jokes.txt").as_path(), - Path::new("/jokes.txt") - ); - assert_eq!( - get_truncated_path("/h/c/d/jokes.txt").as_path(), - Path::new("/h/c/d/jokes.txt") - ); - assert_eq!(get_truncated_path("").as_path(), Path::new("")); -} From 21607e0f973f530e24caf22cec793fb0745fdd6a Mon Sep 17 00:00:00 2001 From: hirschenberger Date: Sun, 26 Jun 2022 15:14:45 +0200 Subject: [PATCH 18/18] Better variable names --- helix-term/src/commands/lsp.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index e3e512323193..7ff7894653ce 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -158,12 +158,12 @@ fn diag_picker( ) -> FilePicker<(lsp::Url, lsp::Diagnostic)> { // TODO: drop current_path comparison and instead use workspace: bool flag? - // flatten the map to a vec of (uri, diag) pairs + // flatten the map to a vec of (url, diag) pairs let mut flat_diag = Vec::new(); - for (u, diags) in diagnostics { + for (url, diags) in diagnostics { flat_diag.reserve(diags.len()); - for d in diags { - flat_diag.push((u.clone(), d)); + for diag in diags { + flat_diag.push((url.clone(), diag)); } }