From 9e31ba547574ca77b1cf63a62237a51eeea2e222 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 11:19:00 -0500 Subject: [PATCH] Consolidate DynamicPicker into Picker DynamicPicker is a thin wrapper over Picker that holds some additional state, similar to the old FilePicker type. Like with FilePicker, we want to fold the two types together, having Picker optionally hold that extra state. The DynamicPicker is a little more complicated than FilePicker was though - it holds a query callback and current query string in state and provides some debounce for queries using the IdleTimeout event. We can move all of that state and debounce logic into an AsyncHook implementation, introduced here as `DynamicQueryHandler`. The hook receives updates to the primary query and debounces those events so that once a query has been idle for a short time (275ms) we re-run the query. A standard Picker created through `new` for example can be promoted into a Dynamic picker by chaining the new `with_dynamic_query` function, very similar to FilePicker's replacement `with_preview`. The workspace symbol picker has been migrated to the new way of writing dynamic pickers as an example. The child commit will promote global search into a dynamic Picker as well. --- helix-term/src/commands/lsp.rs | 69 ++++++++++++----- helix-term/src/ui/mod.rs | 2 +- helix-term/src/ui/picker.rs | 112 +++++++-------------------- helix-term/src/ui/picker/handlers.rs | 99 +++++++++++++++++++---- 4 files changed, 163 insertions(+), 119 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 2292569bf..3dfe46ad7 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -26,7 +26,7 @@ use helix_view::{ use crate::{ compositor::{self, Compositor}, job::Callback, - ui::{self, overlay::overlaid, DynamicPicker, FileLocation, Picker, Popup, PromptEvent}, + ui::{self, overlay::overlaid, FileLocation, Picker, Popup, PromptEvent}, }; use std::{ @@ -413,6 +413,8 @@ pub fn symbol_picker(cx: &mut Context) { } pub fn workspace_symbol_picker(cx: &mut Context) { + use crate::ui::picker::Injector; + let doc = doc!(cx.editor); if doc .language_servers_with_feature(LanguageServerFeature::WorkspaceSymbols) @@ -424,19 +426,21 @@ pub fn workspace_symbol_picker(cx: &mut Context) { return; } - let get_symbols = move |pattern: String, editor: &mut Editor| { + let get_symbols = |pattern: &str, editor: &mut Editor, _data, injector: &Injector<_, _>| { let doc = doc!(editor); let mut seen_language_servers = HashSet::new(); let mut futures: FuturesOrdered<_> = doc .language_servers_with_feature(LanguageServerFeature::WorkspaceSymbols) .filter(|ls| seen_language_servers.insert(ls.id())) .map(|language_server| { - let request = language_server.workspace_symbols(pattern.clone()).unwrap(); + let request = language_server + .workspace_symbols(pattern.to_string()) + .unwrap(); let offset_encoding = language_server.offset_encoding(); async move { let json = request.await?; - let response = + let response: Vec<_> = serde_json::from_value::>>(json)? .unwrap_or_default() .into_iter() @@ -455,29 +459,56 @@ pub fn workspace_symbol_picker(cx: &mut Context) { editor.set_error("No configured language server supports workspace symbols"); } + let injector = injector.clone(); async move { - let mut symbols = Vec::new(); // TODO if one symbol request errors, all other requests are discarded (even if they're valid) - while let Some(mut lsp_items) = futures.try_next().await? { - symbols.append(&mut lsp_items); + while let Some(lsp_items) = futures.try_next().await? { + for item in lsp_items { + injector.push(item)?; + } } - anyhow::Ok(symbols) + Ok(()) } .boxed() }; + let columns = vec![ + ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { + display_symbol_kind(item.symbol.kind).into() + }), + ui::PickerColumn::new("name", |item: &SymbolInformationItem, _| { + item.symbol.name.as_str().into() + }) + .without_filtering(), + ui::PickerColumn::new("path", |item: &SymbolInformationItem, _| { + match item.symbol.location.uri.to_file_path() { + Ok(path) => path::get_relative_path(path.as_path()) + .to_string_lossy() + .to_string() + .into(), + Err(_) => item.symbol.location.uri.to_string().into(), + } + }), + ]; - let initial_symbols = get_symbols("".to_owned(), cx.editor); - - cx.jobs.callback(async move { - let symbols = initial_symbols.await?; - let call = move |_editor: &mut Editor, compositor: &mut Compositor| { - let picker = sym_picker(symbols, true); - let dyn_picker = DynamicPicker::new(picker, Box::new(get_symbols)); - compositor.push(Box::new(overlaid(dyn_picker))) - }; + let picker = Picker::new( + columns, + 1, // name column + vec![], + (), + move |cx, item, action| { + jump_to_location( + cx.editor, + &item.symbol.location, + item.offset_encoding, + action, + ); + }, + ) + .with_preview(|_editor, item| Some(location_to_file_location(&item.symbol.location))) + .with_dynamic_query(get_symbols, None) + .truncate_start(false); - Ok(Callback::EditorCompositor(Box::new(call))) - }); + cx.push_layer(Box::new(overlaid(picker))); } pub fn diagnostics_picker(cx: &mut Context) { diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 4f6b031d4..93ac2e651 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -21,7 +21,7 @@ pub use editor::EditorView; use helix_stdx::rope; pub use markdown::Markdown; pub use menu::Menu; -pub use picker::{Column as PickerColumn, DynamicPicker, FileLocation, Picker}; +pub use picker::{Column as PickerColumn, FileLocation, Picker}; pub use popup::Popup; pub use prompt::{Prompt, PromptEvent}; pub use spinner::{ProgressSpinners, Spinner}; diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index fc1eba6e7..bfec9ddb4 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -4,9 +4,7 @@ mod query; use crate::{ alt, compositor::{self, Component, Compositor, Context, Event, EventResult}, - ctrl, - job::Callback, - key, shift, + ctrl, key, shift, ui::{ self, document::{render_document, LineDecoration, LinePos, TextRenderer}, @@ -53,9 +51,7 @@ use helix_view::{ Document, DocumentId, Editor, }; -use super::overlay::Overlay; - -use self::handlers::PreviewHighlightHandler; +use self::handlers::{DynamicQueryHandler, PreviewHighlightHandler}; pub const ID: &str = "picker"; @@ -221,6 +217,11 @@ impl Column { } } +/// Returns a new list of options to replace the contents of the picker +/// when called with the current picker query, +type DynQueryCallback = + fn(&str, &mut Editor, Arc, &Injector) -> BoxFuture<'static, anyhow::Result<()>>; + pub struct Picker { columns: Arc<[Column]>, primary_column: usize, @@ -250,6 +251,7 @@ pub struct Picker { file_fn: Option>, /// An event handler for syntax highlighting the currently previewed file. preview_highlight_handler: Sender>, + dynamic_query_handler: Option>>, } impl Picker { @@ -359,6 +361,7 @@ impl Picker { read_buffer: Vec::with_capacity(1024), file_fn: None, preview_highlight_handler: PreviewHighlightHandler::::default().spawn(), + dynamic_query_handler: None, } } @@ -394,12 +397,15 @@ impl Picker { self } - pub fn set_options(&mut self, new_options: Vec) { - self.matcher.restart(false); - let injector = self.matcher.injector(); - for item in new_options { - inject_nucleo_item(&injector, &self.columns, item, &self.editor_data); - } + pub fn with_dynamic_query( + mut self, + callback: DynQueryCallback, + debounce_ms: Option, + ) -> Self { + let handler = DynamicQueryHandler::new(callback, debounce_ms).spawn(); + helix_event::send_blocking(&handler, self.primary_query()); + self.dynamic_query_handler = Some(handler); + self } /// Move the cursor by a number of lines, either down (`Forward`) or up (`Backward`) @@ -514,6 +520,11 @@ impl Picker { is_append, ); } + // If this is a dynamic picker, notify the query hook that the primary + // query might have been updated. + if let Some(handler) = &self.dynamic_query_handler { + helix_event::send_blocking(handler, self.primary_query()); + } } fn current_file(&self, editor: &Editor) -> Option { @@ -621,7 +632,11 @@ impl Picker { let count = format!( "{}{}/{}", - if status.running { "(running) " } else { "" }, + if status.running || self.matcher.active_injectors() > 0 { + "(running) " + } else { + "" + }, snapshot.matched_item_count(), snapshot.item_count(), ); @@ -1018,74 +1033,3 @@ impl Drop for Picker { } type PickerCallback = Box; - -/// 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: Picker, - query_callback: DynQueryCallback, - query: String, -} - -impl DynamicPicker { - pub fn new(file_picker: Picker, query_callback: DynQueryCallback) -> Self { - Self { - file_picker, - query_callback, - query: String::new(), - } - } -} - -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 event_result = self.file_picker.handle_event(event, cx); - let Some(current_query) = self.file_picker.primary_query() else { - return event_result; - }; - - if !matches!(event, Event::IdleTimeout) || self.query == *current_query { - return event_result; - } - - self.query = current_query.to_string(); - - 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 = Callback::EditorCompositor(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::>(ID) { - Some(overlay) => &mut overlay.content.file_picker, - None => return, - }; - picker.set_options(new_options); - })); - 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(ID) - } -} diff --git a/helix-term/src/ui/picker/handlers.rs b/helix-term/src/ui/picker/handlers.rs index f01c982a7..e426adda7 100644 --- a/helix-term/src/ui/picker/handlers.rs +++ b/helix-term/src/ui/picker/handlers.rs @@ -1,11 +1,15 @@ -use std::{path::Path, sync::Arc, time::Duration}; +use std::{ + path::Path, + sync::{atomic, Arc}, + time::Duration, +}; use helix_event::AsyncHook; use tokio::time::Instant; use crate::{job, ui::overlay::Overlay}; -use super::{CachedPreview, DynamicPicker, Picker}; +use super::{CachedPreview, DynQueryCallback, Picker}; pub(super) struct PreviewHighlightHandler { trigger: Option>, @@ -50,12 +54,11 @@ impl AsyncHook }; job::dispatch_blocking(move |editor, compositor| { - let picker = match compositor.find::>>() { - Some(Overlay { content, .. }) => content, - None => match compositor.find::>>() { - Some(Overlay { content, .. }) => &mut content.file_picker, - None => return, - }, + let Some(Overlay { + content: picker, .. + }) = compositor.find::>>() + else { + return; }; let Some(CachedPreview::Document(ref mut doc)) = picker.preview_cache.get_mut(&path) @@ -87,13 +90,10 @@ impl AsyncHook }; job::dispatch_blocking(move |editor, compositor| { - let picker = match compositor.find::>>() { - Some(Overlay { content, .. }) => Some(content), - None => compositor - .find::>>() - .map(|overlay| &mut overlay.content.file_picker), - }; - let Some(picker) = picker else { + let Some(Overlay { + content: picker, .. + }) = compositor.find::>>() + else { log::info!("picker closed before syntax highlighting finished"); return; }; @@ -114,3 +114,72 @@ impl AsyncHook }); } } + +pub(super) struct DynamicQueryHandler { + callback: Arc>, + // Duration used as a debounce. + // Defaults to 100ms if not provided via `Picker::with_dynamic_query`. Callers may want to set + // this higher if the dynamic query is expensive - for example global search. + debounce: Duration, + last_query: Arc, + query: Option>, +} + +impl DynamicQueryHandler { + pub(super) fn new(callback: DynQueryCallback, duration_ms: Option) -> Self { + Self { + callback: Arc::new(callback), + debounce: Duration::from_millis(duration_ms.unwrap_or(100)), + last_query: "".into(), + query: None, + } + } +} + +impl AsyncHook for DynamicQueryHandler { + type Event = Arc; + + fn handle_event(&mut self, query: Self::Event, _timeout: Option) -> Option { + if query == self.last_query { + // If the search query reverts to the last one we requested, no need to + // make a new request. + self.query = None; + None + } else { + self.query = Some(query); + Some(Instant::now() + self.debounce) + } + } + + fn finish_debounce(&mut self) { + let Some(query) = self.query.take() else { + return; + }; + self.last_query = query.clone(); + let callback = self.callback.clone(); + + job::dispatch_blocking(move |editor, compositor| { + let Some(Overlay { + content: picker, .. + }) = compositor.find::>>() + else { + return; + }; + // Increment the version number to cancel any ongoing requests. + picker.version.fetch_add(1, atomic::Ordering::Relaxed); + picker.matcher.restart(false); + let injector = picker.injector(); + let get_options = (callback)(&query, editor, picker.editor_data.clone(), &injector); + tokio::spawn(async move { + if let Err(err) = get_options.await { + log::info!("Dynamic request failed: {err}"); + } + // The picker's shows its running indicator when there are any active + // injectors. When we're done injecting new options, drop the injector + // and request a redraw to remove the running indicator. + drop(injector); + helix_event::request_redraw(); + }); + }) + } +}