From 759972459cf7b873f8e0439f061e838b99588036 Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Tue, 19 Jul 2022 21:28:14 +0530 Subject: [PATCH 1/3] Add force_score() for scoring picker items without optimizations --- helix-term/src/ui/picker.rs | 51 +++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 9707c81ee2b2..36a3d3a1cb93 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -390,37 +390,44 @@ impl Picker { self.matches .sort_unstable_by_key(|(_, score)| Reverse(*score)); } else { - self.matches.clear(); - self.matches.extend( - self.options - .iter() - .enumerate() - .filter_map(|(index, option)| { - // filter options first before matching - if !self.filters.is_empty() { - // TODO: this filters functionality seems inefficient, - // instead store and operate on filters if any - self.filters.binary_search(&index).ok()?; - } - - let text = option.filter_text(&self.editor_data); - - self.matcher - .fuzzy_match(&text, pattern) - .map(|score| (index, score)) - }), - ); - self.matches - .sort_unstable_by_key(|(_, score)| Reverse(*score)); + self.force_score(); } log::debug!("picker score {:?}", Instant::now().duration_since(now)); // reset cursor position self.cursor = 0; + let pattern = self.prompt.line(); self.previous_pattern.clone_from(pattern); } + pub fn force_score(&mut self) { + let pattern = self.prompt.line(); + + self.matches.clear(); + self.matches.extend( + self.options + .iter() + .enumerate() + .filter_map(|(index, option)| { + // filter options first before matching + if !self.filters.is_empty() { + // TODO: this filters functionality seems inefficient, + // instead store and operate on filters if any + self.filters.binary_search(&index).ok()?; + } + + let text = option.filter_text(&self.editor_data); + + self.matcher + .fuzzy_match(&text, pattern) + .map(|score| (index, score)) + }), + ); + self.matches + .sort_unstable_by_key(|(_, score)| Reverse(*score)); + } + /// Move the cursor by a number of lines, either down (`Forward`) or up (`Backward`) pub fn move_by(&mut self, amount: usize, direction: Direction) { let len = self.matches.len(); From fbaca6ed7b5f34f70c857d3422e3a522accd8e5e Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Tue, 19 Jul 2022 21:49:02 +0530 Subject: [PATCH 2/3] Add DynamicPicker for updating options on every key --- helix-term/src/ui/mod.rs | 2 +- helix-term/src/ui/overlay.rs | 4 ++ helix-term/src/ui/picker.rs | 73 +++++++++++++++++++++++++++++++++++- 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 257608f00dac..fca3b1d805b3 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -16,7 +16,7 @@ pub use completion::Completion; pub use editor::EditorView; pub use markdown::Markdown; pub use menu::Menu; -pub use picker::{FileLocation, FilePicker, Picker}; +pub use picker::{DynamicPicker, FileLocation, FilePicker, Picker}; pub use popup::Popup; pub use prompt::{Prompt, PromptEvent}; pub use spinner::{ProgressSpinners, Spinner}; diff --git a/helix-term/src/ui/overlay.rs b/helix-term/src/ui/overlay.rs index 9f522e355547..fef6cd7d3447 100644 --- a/helix-term/src/ui/overlay.rs +++ b/helix-term/src/ui/overlay.rs @@ -70,4 +70,8 @@ impl Component for Overlay { let dimensions = (self.calc_child_size)(area); self.content.cursor(dimensions, ctx) } + + fn id(&self) -> Option<&'static str> { + self.content.id() + } } diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 36a3d3a1cb93..a6f5bca96f9f 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -4,6 +4,7 @@ use crate::{ ui::{self, EditorView}, }; use crossterm::event::Event; +use futures_util::future::BoxFuture; use tui::{ buffer::Buffer as Surface, widgets::{Block, BorderType, Borders}, @@ -29,7 +30,7 @@ use helix_view::{ Document, Editor, }; -use super::menu::Item; +use super::{menu::Item, overlay::Overlay}; pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72; /// Biggest file size to preview in bytes @@ -678,3 +679,73 @@ impl Component for Picker { self.prompt.cursor(area, editor) } } + +/// Returns a new list of options to replace the contents of the picker +/// when called with the current picker query, +pub type DynQueryCallback = + Box BoxFuture<'static, anyhow::Result>>>; + +/// A picker that updates its contents via a callback whenever the +/// query string changes. Useful for live grep, workspace symbols, etc. +pub struct DynamicPicker { + file_picker: FilePicker, + query_callback: DynQueryCallback, +} + +impl DynamicPicker { + pub const ID: &'static str = "dynamic-picker"; + + pub fn new(file_picker: FilePicker, query_callback: DynQueryCallback) -> Self { + Self { + file_picker, + query_callback, + } + } +} + +impl Component for DynamicPicker { + fn render(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { + self.file_picker.render(area, surface, cx); + } + + fn handle_event(&mut self, event: Event, cx: &mut Context) -> EventResult { + let prev_query = self.file_picker.picker.prompt.line().to_owned(); + let event_result = self.file_picker.handle_event(event, cx); + let current_query = self.file_picker.picker.prompt.line(); + + if *current_query == prev_query || matches!(event_result, EventResult::Ignored(_)) { + return event_result; + } + + let new_options = (self.query_callback)(current_query.to_owned(), cx.editor); + + cx.jobs.callback(async move { + let new_options = new_options.await?; + let callback: crate::job::Callback = Box::new(move |_editor, compositor| { + // Wrapping of pickers in overlay is done outside the picker code, + // so this is fragile and will break if wrapped in some other widget. + let picker = match compositor.find_id::>>(Self::ID) { + Some(overlay) => &mut overlay.content.file_picker.picker, + None => return, + }; + picker.options = new_options; + picker.cursor = 0; + picker.force_score(); + }); + anyhow::Ok(callback) + }); + EventResult::Consumed(None) + } + + fn cursor(&self, area: Rect, ctx: &Editor) -> (Option, CursorKind) { + self.file_picker.cursor(area, ctx) + } + + fn required_size(&mut self, viewport: (u16, u16)) -> Option<(u16, u16)> { + self.file_picker.required_size(viewport) + } + + fn id(&self) -> Option<&'static str> { + Some(Self::ID) + } +} From 2d86d101183fae2157dac1ebd16cf997133ab99a Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Tue, 19 Jul 2022 22:15:03 +0530 Subject: [PATCH 3/3] Re-request workspace symbols on keypress in picker Most language servers limit the number of workspace symbols that are returned with an empty query even though all symbols are supposed to be returned, according to the spec (for perfomance reasons). This patch adds a workspace symbol picker based on a dynamic picker that allows re-requesting the symbols on every keypress (i.e. when the picker query text changes). The old behavior has been completely replaced, and I have only tested with rust-analyzer so far. --- helix-term/src/commands/lsp.rs | 38 +++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 1785a50c29dd..0705e9fdf9fc 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -1,3 +1,4 @@ +use futures_util::FutureExt; use helix_lsp::{ block_on, lsp::{self, DiagnosticSeverity, NumberOrString}, @@ -14,7 +15,8 @@ use helix_view::{editor::Action, theme::Style}; use crate::{ compositor::{self, Compositor}, ui::{ - self, lsp::SignatureHelp, overlay::overlayed, FileLocation, FilePicker, Popup, PromptEvent, + self, lsp::SignatureHelp, overlay::overlayed, DynamicPicker, FileLocation, FilePicker, + Popup, PromptEvent, }, }; @@ -365,10 +367,36 @@ pub fn workspace_symbol_picker(cx: &mut Context) { cx.callback( future, move |_editor, compositor, response: Option>| { - if let Some(symbols) = response { - let picker = sym_picker(symbols, current_url, offset_encoding); - compositor.push(Box::new(overlayed(picker))) - } + let symbols = match response { + Some(s) => s, + None => return, + }; + let picker = sym_picker(symbols, current_url, offset_encoding); + let get_symbols = |query: String, editor: &mut Editor| { + let doc = doc!(editor); + let language_server = match doc.language_server() { + Some(s) => s, + None => { + // This should not generally happen since the picker will not + // even open in the first place if there is no server. + return async move { Err(anyhow::anyhow!("LSP not active")) }.boxed(); + } + }; + let symbol_request = language_server.workspace_symbols(query); + + let future = async move { + let json = symbol_request.await?; + let response: Option> = + serde_json::from_value(json)?; + + response.ok_or_else(|| { + anyhow::anyhow!("No response for workspace symbols from language server") + }) + }; + future.boxed() + }; + let dyn_picker = DynamicPicker::new(picker, Box::new(get_symbols)); + compositor.push(Box::new(overlayed(dyn_picker))) }, ) }