Add workspace and document diagnostics picker (#2013)

* Add workspace and document diagnostics picker

fixes #1891

* Fix some of @archseer's annotations

* Add From<&Spans> impl for String

* More descriptive parameter names.

* Adding From<Cow<str>> impls for Span and Spans

* Add new keymap entries to docs

* Avoid some clones

* Fix api change

* Update helix-term/src/application.rs

Co-authored-by: Bjorn Ove Hay Andersen <bjrnove@gmail.com>

* Fix a clippy hint

* Sort diagnostics first by URL and then by severity.

* Sort diagnostics first by URL and then by severity.

* Ignore missing lsp severity entries

* Add truncated filepath

* Typo

* Strip cwd from paths and use url-path without schema

* Make tests a doctest

* Better variable names

Co-authored-by: Falco Hirschenberger <falco.hirschenberger@itwm.fraunhofer.de>
Co-authored-by: Bjorn Ove Hay Andersen <bjrnove@gmail.com>

book/src/keymap.md

@@ -241,6 +241,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`                       |

helix-core/src/path.rs

@@ -90,3 +90,54 @@ 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.
+///
+/// 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<P: AsRef<Path>>(path: P) -> PathBuf {
+    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(
+            d.to_string_lossy()
+                .chars()
+                .next()
+                .unwrap_or_default()
+                .to_string(),
+        );
+    }
+    ret.push(file);
+    ret
+}

helix-term/src/application.rs

@@ -495,7 +495,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);
 
@@ -505,12 +505,9 @@ impl Application {
 
                             let diagnostics = params
                                 .diagnostics
-                                .into_iter()
+                                .iter()
                                 .filter_map(|diagnostic| {
-                                    use helix_core::{
+                                    use helix_core::diagnostic::{Diagnostic, Range, Severity::*};
-                                        diagnostic::{Range, Severity::*},
-                                        Diagnostic,
-                                    };
                                     use lsp::DiagnosticSeverity;
 
                                     let language_server = doc.language_server().unwrap();
@@ -561,7 +558,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
@@ -571,6 +568,23 @@ 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 {
+                                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.
+                        self.editor
+                            .diagnostics
+                            .insert(params.uri, params.diagnostics);
                     }
                     Notification::ShowMessage(params) => {
                         log::warn!("unhandled window/showMessage: {:?}", params);

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::{
@@ -265,6 +266,8 @@ impl MappableCommand {
         symbol_picker, "Open symbol picker",
         select_references_to_symbol_under_cursor, "Select symbol references",
         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",
@@ -2170,7 +2173,7 @@ fn buffer_picker(cx: &mut Context) {
     }
 
     impl BufferMeta {
-        fn format(&self) -> Cow<str> {
+        fn format(&self) -> Spans {
             let path = self
                 .path
                 .as_deref()
@@ -2193,7 +2196,7 @@ fn buffer_picker(cx: &mut Context) {
             } else {
                 format!(" ({})", flags.join(""))
             };
-            Cow::Owned(format!("{} {}{}", self.id, path, flag))
+            format!("{} {}{}", self.id, path, flag).into()
         }
     }
 
@@ -2260,10 +2263,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.into(),
+                        None => doc.as_str().into(),
                     },
                     MappableCommand::Static { doc, name, .. } => match keymap.get(*name) {
                         Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(),

helix-term/src/commands/lsp.rs

@@ -1,20 +1,25 @@
 use helix_lsp::{
-    block_on, lsp,
+    block_on,
+    lsp::{self, DiagnosticSeverity, NumberOrString},
     util::{diagnostic_to_lsp_diagnostic, 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_core::{path, 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};
 
 /// Gets the language server that is attached to a document, and
 /// if it's not active displays a status message. Using this macro
@@ -145,6 +150,97 @@ fn sym_picker(
     .truncate_start(false)
 }
 
+fn diag_picker(
+    cx: &Context,
+    diagnostics: BTreeMap<lsp::Url, Vec<lsp::Diagnostic>>,
+    current_path: Option<lsp::Url>,
+    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 (url, diag) pairs
+    let mut flat_diag = Vec::new();
+    for (url, diags) in diagnostics {
+        flat_diag.reserve(diags.len());
+        for diag in diags {
+            flat_diag.push((url.clone(), diag));
+        }
+    }
+
+    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, diag)| {
+            let mut style = diag
+                .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;
+
+            let code = diag
+                .code
+                .as_ref()
+                .map(|c| match c {
+                    NumberOrString::Number(n) => n.to_string(),
+                    NumberOrString::String(s) => s.to_string(),
+                })
+                .unwrap_or_default();
+
+            let truncated_path = path::get_truncated_path(url.path())
+                .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(truncated_path, style),
+                Span::raw(" - "),
+                Span::styled(code, style.add_modifier(Modifier::BOLD)),
+                Span::raw(": "),
+                Span::styled(&diag.message, style),
+            ])
+        },
+        move |cx, (url, diag), action| {
+            if current_path.as_ref() == Some(url) {
+                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");
+            }
+
+            let (view, doc) = current!(cx.editor);
+
+            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));
+                align_view(doc, view, Align::Center);
+            }
+        },
+        move |_editor, (url, diag)| {
+            let location = lsp::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<lsp::SymbolInformation>,
@@ -215,6 +311,37 @@ 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
+            .get(&current_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)));
+    }
+}
+
+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 {

helix-term/src/keymap/default.rs

@@ -207,6 +207,8 @@ pub fn default() -> HashMap<Mode, Keymap> {
             "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

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;
 
@@ -172,7 +174,7 @@ 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())
         },
         move |cx, path: &PathBuf, action| {
             if let Err(e) = cx.editor.open(path, action) {

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<T> FilePicker<T> {
     pub fn new(
         options: Vec<T>,
-        format_fn: impl Fn(&T) -> Cow<str> + 'static,
+        format_fn: impl Fn(&T) -> Spans + 'static,
         callback_fn: impl Fn(&mut Context, &T, Action) + 'static,
         preview_fn: impl Fn(&Editor, &T) -> Option<FileLocation> + 'static,
     ) -> Self {
@@ -299,14 +299,14 @@ pub struct Picker<T> {
     /// Whether to truncate the start (default true)
     pub truncate_start: bool,
 
-    format_fn: Box<dyn Fn(&T) -> Cow<str>>,
+    format_fn: Box<dyn Fn(&T) -> Spans>,
     callback_fn: Box<dyn Fn(&mut Context, &T, Action)>,
 }
 
 impl<T> Picker<T> {
     pub fn new(
         options: Vec<T>,
-        format_fn: impl Fn(&T) -> Cow<str> + 'static,
+        format_fn: impl Fn(&T) -> Spans + 'static,
         callback_fn: impl Fn(&mut Context, &T, Action) + 'static,
     ) -> Self {
         let prompt = Prompt::new(
@@ -372,9 +372,8 @@ impl<T> Picker<T> {
             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);
+                let line: String = (self.format_fn)(option).into();
-
+                match self.matcher.fuzzy_match(&line, pattern) {
-                match self.matcher.fuzzy_match(&text, pattern) {
                     Some(s) => {
                         // Update the score
                         *score = s;
@@ -401,10 +400,10 @@ impl<T> Picker<T> {
                         }
 
                         // 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))
                     }),
             );
@@ -611,30 +610,34 @@ impl<T: 'static> Component for Picker<T> {
                 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), self.prompt.line())
                 .unwrap_or_default();
 
-            surface.set_string_truncated(
+            spans.0.into_iter().fold(inner, |pos, span| {
-                inner.x,
+                let new_x = surface
-                inner.y + i as u16,
+                    .set_string_truncated(
-                &formatted,
+                        pos.x,
-                inner.width as usize,
+                        pos.y + i as u16,
-                |idx| {
+                        &span.content,
-                    if highlights.contains(&idx) {
+                        pos.width as usize,
-                        highlighted
+                        |idx| {
-                    } else if is_active {
+                            if highlights.contains(&idx) {
-                        selected
+                                highlighted.patch(span.style)
-                    } else {
+                            } else if is_active {
-                        text_style
+                                selected.patch(span.style)
-                    }
+                            } else {
-                },
+                                text_style.patch(span.style)
-                true,
+                            }
-                self.truncate_start,
+                        },
-            );
+                        true,
+                        self.truncate_start,
+                    )
+                    .0;
+                pos.clip_left(new_x - pos.x)
+            });
         }
     }
 

helix-tui/src/text.rs

@@ -194,6 +194,12 @@ impl<'a> From<&'a str> for Span<'a> {
     }
 }
 
+impl<'a> From<Cow<'a, str>> 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<Span<'a>>);
@@ -229,6 +235,12 @@ impl<'a> From<&'a str> for Spans<'a> {
     }
 }
 
+impl<'a> From<Cow<'a, str>> for Spans<'a> {
+    fn from(s: Cow<'a, str>) -> Spans<'a> {
+        Spans(vec![Span::raw(s)])
+    }
+}
+
 impl<'a> From<Vec<Span<'a>>> for Spans<'a> {
     fn from(spans: Vec<Span<'a>>) -> Spans<'a> {
         Spans(spans)
@@ -243,10 +255,13 @@ impl<'a> From<Span<'a>> for Spans<'a> {
 
 impl<'a> From<Spans<'a>> for String {
     fn from(line: Spans<'a>) -> String {
-        line.0.iter().fold(String::new(), |mut acc, s| {
+        line.0.iter().map(|s| &*s.content).collect()
-            acc.push_str(s.content.as_ref());
+    }
-            acc
+}
-        })
+
+impl<'a> From<&Spans<'a>> for String {
+    fn from(line: &Spans<'a>) -> String {
+        line.0.iter().map(|s| &*s.content).collect()
     }
 }
 

helix-tui/src/widgets/block.rs

@@ -77,7 +77,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

helix-view/src/editor.rs

@@ -39,6 +39,7 @@ use helix_core::{
     Change,
 };
 use helix_dap as dap;
+use helix_lsp::lsp;
 
 use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer};
 
@@ -462,6 +463,7 @@ pub struct Editor {
     pub macro_replaying: Vec<char>,
     pub theme: Theme,
     pub language_servers: helix_lsp::Registry,
+    pub diagnostics: BTreeMap<lsp::Url, Vec<lsp::Diagnostic>>,
 
     pub debugger: Option<dap::Client>,
     pub debugger_events: SelectAll<UnboundedReceiverStream<dap::Payload>>,
@@ -533,6 +535,7 @@ impl Editor {
             macro_replaying: Vec::new(),
             theme: theme_loader.default(),
             language_servers,
+            diagnostics: BTreeMap::new(),
             debugger: None,
             debugger_events: SelectAll::new(),
             breakpoints: HashMap::new(),

xtask/src/main.rs

@@ -106,7 +106,7 @@ pub mod md_gen {
                 .collect::<Vec<_>>()
                 .join(", ");
 
-            let doc = cmd.doc.replace("\n", "<br>");
+            let doc = cmd.doc.replace('\n', "<br>");
 
             md.push_str(&md_table_row(&[names.to_owned(), doc.to_owned()]));
         }