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

helix-dap/src/client.rs

@@ -34,7 +34,7 @@ pub struct Client {
     pub caps: Option<DebuggerCapabilities>,
     // thread_id -> frames
     pub stack_frames: HashMap<ThreadId, Vec<StackFrame>>,
-    pub thread_states: HashMap<ThreadId, String>,
+    pub thread_states: ThreadStates,
     pub thread_id: Option<ThreadId>,
     /// Currently active frame for the current thread.
     pub active_frame: Option<usize>,

helix-dap/src/types.rs

@@ -14,6 +14,8 @@ impl std::fmt::Display for ThreadId {
     }
 }
 
+pub type ThreadStates = HashMap<ThreadId, String>;
+
 pub trait Request {
     type Arguments: serde::de::DeserializeOwned + serde::Serialize;
     type Result: serde::de::DeserializeOwned + serde::Serialize;

helix-term/src/commands.rs

@@ -45,6 +45,7 @@ use movement::Movement;
 use crate::{
     args,
     compositor::{self, Component, Compositor},
+    keymap::ReverseKeymap,
     ui::{self, overlay::overlayed, FilePicker, Picker, Popup, Prompt, PromptEvent},
 };
 
@@ -1744,8 +1745,42 @@ fn search_selection(cx: &mut Context) {
 }
 
 fn global_search(cx: &mut Context) {
-    let (all_matches_sx, all_matches_rx) =
-        tokio::sync::mpsc::unbounded_channel::<(usize, PathBuf)>();
+    #[derive(Debug)]
+    struct FileResult {
+        path: PathBuf,
+        /// 0 indexed lines
+        line_num: usize,
+    }
+
+    impl FileResult {
+        fn new(path: &Path, line_num: usize) -> Self {
+            Self {
+                path: path.to_path_buf(),
+                line_num,
+            }
+        }
+    }
+
+    impl ui::menu::Item for FileResult {
+        type Data = Option<PathBuf>;
+
+        fn label(&self, current_path: &Self::Data) -> Spans {
+            let relative_path = helix_core::path::get_relative_path(&self.path)
+                .to_string_lossy()
+                .into_owned();
+            if current_path
+                .as_ref()
+                .map(|p| p == &self.path)
+                .unwrap_or(false)
+            {
+                format!("{} (*)", relative_path).into()
+            } else {
+                relative_path.into()
+            }
+        }
+    }
+
+    let (all_matches_sx, all_matches_rx) = tokio::sync::mpsc::unbounded_channel::<FileResult>();
     let config = cx.editor.config();
     let smart_case = config.search.smart_case;
     let file_picker_config = config.file_picker.clone();
@@ -1809,7 +1844,7 @@ fn global_search(cx: &mut Context) {
                                 entry.path(),
                                 sinks::UTF8(|line_num, _| {
                                     all_matches_sx
-                                        .send((line_num as usize - 1, entry.path().to_path_buf()))
+                                        .send(FileResult::new(entry.path(), line_num as usize - 1))
                                         .unwrap();
 
                                     Ok(true)
@@ -1836,7 +1871,7 @@ fn global_search(cx: &mut Context) {
     let current_path = doc_mut!(cx.editor).path().cloned();
 
     let show_picker = async move {
-        let all_matches: Vec<(usize, PathBuf)> =
+        let all_matches: Vec<FileResult> =
             UnboundedReceiverStream::new(all_matches_rx).collect().await;
         let call: job::Callback =
             Box::new(move |editor: &mut Editor, compositor: &mut Compositor| {
@@ -1847,17 +1882,8 @@ fn global_search(cx: &mut Context) {
 
                 let picker = FilePicker::new(
                     all_matches,
-                    move |(_line_num, path)| {
-                        let relative_path = helix_core::path::get_relative_path(path)
-                            .to_string_lossy()
-                            .into_owned();
-                        if current_path.as_ref().map(|p| p == path).unwrap_or(false) {
-                            format!("{} (*)", relative_path).into()
-                        } else {
-                            relative_path.into()
-                        }
-                    },
-                    move |cx, (line_num, path), action| {
+                    current_path,
+                    move |cx, FileResult { path, line_num }, action| {
                         match cx.editor.open(path, action) {
                             Ok(_) => {}
                             Err(e) => {
@@ -1879,7 +1905,9 @@ fn global_search(cx: &mut Context) {
                         doc.set_selection(view.id, Selection::single(start, end));
                         align_view(doc, view, Align::Center);
                     },
-                    |_editor, (line_num, path)| Some((path.clone(), Some((*line_num, *line_num)))),
+                    |_editor, FileResult { path, line_num }| {
+                        Some((path.clone(), Some((*line_num, *line_num))))
+                    },
                 );
                 compositor.push(Box::new(overlayed(picker)));
             });
@@ -2172,8 +2200,10 @@ fn buffer_picker(cx: &mut Context) {
         is_current: bool,
     }
 
-    impl BufferMeta {
-        fn format(&self) -> Spans {
+    impl ui::menu::Item for BufferMeta {
+        type Data = ();
+
+        fn label(&self, _data: &Self::Data) -> Spans {
             let path = self
                 .path
                 .as_deref()
@@ -2213,7 +2243,7 @@ fn buffer_picker(cx: &mut Context) {
             .iter()
             .map(|(_, doc)| new_meta(doc))
             .collect(),
-        BufferMeta::format,
+        (),
         |cx, meta, action| {
             cx.editor.switch(meta.id, action);
         },
@@ -2230,6 +2260,38 @@ fn buffer_picker(cx: &mut Context) {
     cx.push_layer(Box::new(overlayed(picker)));
 }
 
+impl ui::menu::Item for MappableCommand {
+    type Data = ReverseKeymap;
+
+    fn label(&self, keymap: &Self::Data) -> Spans {
+        // formats key bindings, multiple bindings are comma separated,
+        // individual key presses are joined with `+`
+        let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
+            bindings
+                .iter()
+                .map(|bind| {
+                    bind.iter()
+                        .map(|key| key.to_string())
+                        .collect::<Vec<String>>()
+                        .join("+")
+                })
+                .collect::<Vec<String>>()
+                .join(", ")
+        };
+
+        match self {
+            MappableCommand::Typable { doc, name, .. } => match keymap.get(name as &String) {
+                Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(),
+                None => doc.as_str().into(),
+            },
+            MappableCommand::Static { doc, name, .. } => match keymap.get(*name) {
+                Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(),
+                None => (*doc).into(),
+            },
+        }
+    }
+}
+
 pub fn command_palette(cx: &mut Context) {
     cx.callback = Some(Box::new(
         move |compositor: &mut Compositor, cx: &mut compositor::Context| {
@@ -2246,44 +2308,17 @@ pub fn command_palette(cx: &mut Context) {
                 }
             }));
 
-            // formats key bindings, multiple bindings are comma separated,
-            // individual key presses are joined with `+`
-            let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
-                bindings
-                    .iter()
-                    .map(|bind| {
-                        bind.iter()
-                            .map(|key| key.key_sequence_format())
-                            .collect::<String>()
-                    })
-                    .collect::<Vec<String>>()
-                    .join(", ")
-            };
-
-            let picker = Picker::new(
-                commands,
-                move |command| match command {
-                    MappableCommand::Typable { doc, name, .. } => match keymap.get(name) {
-                        Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(),
-                        None => doc.as_str().into(),
-                    },
-                    MappableCommand::Static { doc, name, .. } => match keymap.get(*name) {
-                        Some(bindings) => format!("{} ({})", doc, fmt_binding(bindings)).into(),
-                        None => (*doc).into(),
-                    },
-                },
-                move |cx, command, _action| {
-                    let mut ctx = Context {
-                        register: None,
-                        count: std::num::NonZeroUsize::new(1),
-                        editor: cx.editor,
-                        callback: None,
-                        on_next_key_callback: None,
-                        jobs: cx.jobs,
-                    };
-                    command.execute(&mut ctx);
-                },
-            );
+            let picker = Picker::new(commands, keymap, move |cx, command, _action| {
+                let mut ctx = Context {
+                    register: None,
+                    count: std::num::NonZeroUsize::new(1),
+                    editor: cx.editor,
+                    callback: None,
+                    on_next_key_callback: None,
+                    jobs: cx.jobs,
+                };
+                command.execute(&mut ctx);
+            });
             compositor.push(Box::new(overlayed(picker)));
         },
     ));

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