Reuse menu::Item trait in picker (#2814)

* Refactor menu::Item to accomodate external state

Will be useful for storing editor state when reused by pickers.

* Add some type aliases for readability

* Reuse menu::Item trait in picker

This opens the way for merging the menu and picker code in the
future, since a picker is essentially a menu + prompt. More
excitingly, this change will also allow aligning items in the
picker, which would be useful (for example) in the command palette
for aligning the descriptions to the left and the keybinds to
the right in two separate columns.

The item formatting of each picker has been kept as is, even though
there is room for improvement now that we can format the data into
columns, since that is better tackled in a separate PR.

* Rename menu::Item::EditorData to Data

* Call and inline filter_text() in sort_text() completion

* Rename diagnostic picker's Item::Data

2 files changed, 170 insertions, 106 deletions

diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs
index b897b2d5..9f6f4c15 100644
--- a/helix-term/src/commands/dap.rs
+++ b/helix-term/src/commands/dap.rs
@@ -4,13 +4,15 @@ use crate::{
     job::{Callback, Jobs},
     ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent, Text},
 };
-use helix_core::syntax::{DebugArgumentValue, DebugConfigCompletion};
+use dap::{StackFrame, Thread, ThreadStates};
+use helix_core::syntax::{DebugArgumentValue, DebugConfigCompletion, DebugTemplate};
 use helix_dap::{self as dap, Client};
 use helix_lsp::block_on;
 use helix_view::editor::Breakpoint;
 
 use serde_json::{to_value, Value};
 use tokio_stream::wrappers::UnboundedReceiverStream;
+use tui::text::Spans;
 
 use std::collections::HashMap;
 use std::future::Future;
@@ -20,6 +22,38 @@ use anyhow::{anyhow, bail};
 
 use helix_view::handlers::dap::{breakpoints_changed, jump_to_stack_frame, select_thread_id};
 
+impl ui::menu::Item for StackFrame {
+    type Data = ();
+
+    fn label(&self, _data: &Self::Data) -> Spans {
+        self.name.as_str().into() // TODO: include thread_states in the label
+    }
+}
+
+impl ui::menu::Item for DebugTemplate {
+    type Data = ();
+
+    fn label(&self, _data: &Self::Data) -> Spans {
+        self.name.as_str().into()
+    }
+}
+
+impl ui::menu::Item for Thread {
+    type Data = ThreadStates;
+
+    fn label(&self, thread_states: &Self::Data) -> Spans {
+        format!(
+            "{} ({})",
+            self.name,
+            thread_states
+                .get(&self.id)
+                .map(|state| state.as_str())
+                .unwrap_or("unknown")
+        )
+        .into()
+    }
+}
+
 fn thread_picker(
     cx: &mut Context,
     callback_fn: impl Fn(&mut Editor, &dap::Thread) + Send + 'static,
@@ -41,17 +75,7 @@ fn thread_picker(
             let thread_states = debugger.thread_states.clone();
             let picker = FilePicker::new(
                 threads,
-                move |thread| {
-                    format!(
-                        "{} ({})",
-                        thread.name,
-                        thread_states
-                            .get(&thread.id)
-                            .map(|state| state.as_str())
-                            .unwrap_or("unknown")
-                    )
-                    .into()
-                },
+                thread_states,
                 move |cx, thread, _action| callback_fn(cx.editor, thread),
                 move |editor, thread| {
                     let frames = editor.debugger.as_ref()?.stack_frames.get(&thread.id)?;
@@ -243,7 +267,7 @@ pub fn dap_launch(cx: &mut Context) {
 
     cx.push_layer(Box::new(overlayed(Picker::new(
         templates,
-        |template| template.name.as_str().into(),
+        (),
         |cx, template, _action| {
             let completions = template.completion.clone();
             let name = template.name.clone();
@@ -475,7 +499,7 @@ pub fn dap_variables(cx: &mut Context) {
 
     for scope in scopes.iter() {
         // use helix_view::graphics::Style;
-        use tui::text::{Span, Spans};
+        use tui::text::Span;
         let response = block_on(debugger.variables(scope.variables_reference));
 
         variables.push(Spans::from(Span::styled(
@@ -652,7 +676,7 @@ pub fn dap_switch_stack_frame(cx: &mut Context) {
 
     let picker = FilePicker::new(
         frames,
-        |frame| frame.name.as_str().into(), // TODO: include thread_states in the label
+        (),
         move |cx, frame, _action| {
             let debugger = debugger!(cx.editor);
             // TODO: this should be simpler to find
diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs
index d11c44cd..7f82394a 100644
--- a/helix-term/src/commands/lsp.rs
+++ b/helix-term/src/commands/lsp.rs
@@ -19,7 +19,8 @@ use crate::{
     ui::{self, overlay::overlayed, FileLocation, FilePicker, Popup, PromptEvent},
 };
 
-use std::{borrow::Cow, collections::BTreeMap};
+use std::collections::BTreeMap;
+use std::{borrow::Cow, path::PathBuf};
 
 /// Gets the language server that is attached to a document, and
 /// if it's not active displays a status message. Using this macro
@@ -39,6 +40,112 @@ macro_rules! language_server {
     };
 }
 
+impl ui::menu::Item for lsp::Location {
+    /// Current working directory.
+    type Data = PathBuf;
+
+    fn label(&self, cwdir: &Self::Data) -> Spans {
+        let file: Cow<'_, str> = (self.uri.scheme() == "file")
+            .then(|| {
+                self.uri
+                    .to_file_path()
+                    .map(|path| {
+                        // strip root prefix
+                        path.strip_prefix(&cwdir)
+                            .map(|path| path.to_path_buf())
+                            .unwrap_or(path)
+                    })
+                    .map(|path| Cow::from(path.to_string_lossy().into_owned()))
+                    .ok()
+            })
+            .flatten()
+            .unwrap_or_else(|| self.uri.as_str().into());
+        let line = self.range.start.line;
+        format!("{}:{}", file, line).into()
+    }
+}
+
+impl ui::menu::Item for lsp::SymbolInformation {
+    /// Path to currently focussed document
+    type Data = Option<lsp::Url>;
+
+    fn label(&self, current_doc_path: &Self::Data) -> Spans {
+        if current_doc_path.as_ref() == Some(&self.location.uri) {
+            self.name.as_str().into()
+        } else {
+            match self.location.uri.to_file_path() {
+                Ok(path) => {
+                    let relative_path = helix_core::path::get_relative_path(path.as_path())
+                        .to_string_lossy()
+                        .into_owned();
+                    format!("{} ({})", &self.name, relative_path).into()
+                }
+                Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(),
+            }
+        }
+    }
+}
+
+struct DiagnosticStyles {
+    hint: Style,
+    info: Style,
+    warning: Style,
+    error: Style,
+}
+
+struct PickerDiagnostic {
+    url: lsp::Url,
+    diag: lsp::Diagnostic,
+}
+
+impl ui::menu::Item for PickerDiagnostic {
+    type Data = DiagnosticStyles;
+
+    fn label(&self, styles: &Self::Data) -> Spans {
+        let mut style = self
+            .diag
+            .severity
+            .map(|s| match s {
+                DiagnosticSeverity::HINT => styles.hint,
+                DiagnosticSeverity::INFORMATION => styles.info,
+                DiagnosticSeverity::WARNING => styles.warning,
+                DiagnosticSeverity::ERROR => styles.error,
+                _ => Style::default(),
+            })
+            .unwrap_or_default();
+
+        // remove background as it is distracting in the picker list
+        style.bg = None;
+
+        let code = self
+            .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(self.url.path())
+            .to_string_lossy()
+            .into_owned();
+
+        Spans::from(vec![
+            Span::styled(
+                self.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(&self.diag.message, style),
+        ])
+    }
+}
+
 fn location_to_file_location(location: &lsp::Location) -> FileLocation {
     let path = location.uri.to_file_path().unwrap();
     let line = Some((
@@ -93,29 +200,14 @@ fn sym_picker(
     offset_encoding: OffsetEncoding,
 ) -> FilePicker<lsp::SymbolInformation> {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
-    let current_path2 = current_path.clone();
     FilePicker::new(
         symbols,
-        move |symbol| {
-            if current_path.as_ref() == Some(&symbol.location.uri) {
-                symbol.name.as_str().into()
-            } else {
-                match symbol.location.uri.to_file_path() {
-                    Ok(path) => {
-                        let relative_path = helix_core::path::get_relative_path(path.as_path())
-                            .to_string_lossy()
-                            .into_owned();
-                        format!("{} ({})", &symbol.name, relative_path).into()
-                    }
-                    Err(_) => format!("{} ({})", &symbol.name, &symbol.location.uri).into(),
-                }
-            }
-        },
+        current_path.clone(),
         move |cx, symbol, action| {
             let (view, doc) = current!(cx.editor);
             push_jump(view, doc);
 
-            if current_path2.as_ref() != Some(&symbol.location.uri) {
+            if current_path.as_ref() != Some(&symbol.location.uri) {
                 let uri = &symbol.location.uri;
                 let path = match uri.to_file_path() {
                     Ok(path) => path,
@@ -155,7 +247,7 @@ fn diag_picker(
     diagnostics: BTreeMap<lsp::Url, Vec<lsp::Diagnostic>>,
     current_path: Option<lsp::Url>,
     offset_encoding: OffsetEncoding,
-) -> FilePicker<(lsp::Url, lsp::Diagnostic)> {
+) -> FilePicker<PickerDiagnostic> {
     // TODO: drop current_path comparison and instead use workspace: bool flag?
 
     // flatten the map to a vec of (url, diag) pairs
@@ -163,59 +255,24 @@ fn diag_picker(
     for (url, diags) in diagnostics {
         flat_diag.reserve(diags.len());
         for diag in diags {
-            flat_diag.push((url.clone(), diag));
+            flat_diag.push(PickerDiagnostic {
+                url: 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");
+    let styles = DiagnosticStyles {
+        hint: cx.editor.theme.get("hint"),
+        info: cx.editor.theme.get("info"),
+        warning: cx.editor.theme.get("warning"),
+        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| {
+        styles,
+        move |cx, PickerDiagnostic { url, diag }, action| {
             if current_path.as_ref() == Some(url) {
                 let (view, doc) = current!(cx.editor);
                 push_jump(view, doc);
@@ -233,7 +290,7 @@ fn diag_picker(
                 align_view(doc, view, Align::Center);
             }
         },
-        move |_editor, (url, diag)| {
+        move |_editor, PickerDiagnostic { url, diag }| {
             let location = lsp::Location::new(url.clone(), diag.range);
             Some(location_to_file_location(&location))
         },
@@ -343,10 +400,11 @@ pub fn workspace_diagnostics_picker(cx: &mut Context) {
 }
 
 impl ui::menu::Item for lsp::CodeActionOrCommand {
-    fn label(&self) -> &str {
+    type Data = ();
+    fn label(&self, _data: &Self::Data) -> Spans {
         match self {
-            lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str(),
-            lsp::CodeActionOrCommand::Command(command) => command.title.as_str(),
+            lsp::CodeActionOrCommand::CodeAction(action) => action.title.as_str().into(),
+            lsp::CodeActionOrCommand::Command(command) => command.title.as_str().into(),
         }
     }
 }
@@ -391,7 +449,7 @@ pub fn code_action(cx: &mut Context) {
                 return;
             }
 
-            let mut picker = ui::Menu::new(actions, move |editor, code_action, event| {
+            let mut picker = ui::Menu::new(actions, (), move |editor, code_action, event| {
                 if event != PromptEvent::Validate {
                     return;
                 }
@@ -619,6 +677,7 @@ pub fn apply_workspace_edit(
         }
     }
 }
+
 fn goto_impl(
     editor: &mut Editor,
     compositor: &mut Compositor,
@@ -637,26 +696,7 @@ fn goto_impl(
         _locations => {
             let picker = FilePicker::new(
                 locations,
-                move |location| {
-                    let file: Cow<'_, str> = (location.uri.scheme() == "file")
-                        .then(|| {
-                            location
-                                .uri
-                                .to_file_path()
-                                .map(|path| {
-                                    // strip root prefix
-                                    path.strip_prefix(&cwdir)
-                                        .map(|path| path.to_path_buf())
-                                        .unwrap_or(path)
-                                })
-                                .map(|path| Cow::from(path.to_string_lossy().into_owned()))
-                                .ok()
-                        })
-                        .flatten()
-                        .unwrap_or_else(|| location.uri.as_str().into());
-                    let line = location.range.start.line;
-                    format!("{}:{}", file, line).into()
-                },
+                cwdir,
                 move |cx, location, action| {
                     jump_to_location(cx.editor, location, offset_encoding, action)
                 },