From dae3841a75fc521c9e5d63ff8f3ffa32be4e41e1 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Thu, 15 Feb 2024 15:13:44 -0500 Subject: [PATCH 01/20] Use an AsyncHook for picker preview highlighting The picker previously used the IdleTimeout event as a trigger for syntax-highlighting the currently selected document in the preview pane. This is a bit ad-hoc now that the event system has landed and we can refactor towards an AsyncHook (like those used for LSP completion and signature-help). This should resolve some odd scenarios where the preview did not highlight because of a race between the idle timeout and items appearing in the picker. --- helix-term/src/ui/picker.rs | 147 ++++++++------------------- helix-term/src/ui/picker/handlers.rs | 117 +++++++++++++++++++++ 2 files changed, 161 insertions(+), 103 deletions(-) create mode 100644 helix-term/src/ui/picker/handlers.rs diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index b8ec57d51447..cc86a4faddab 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -1,3 +1,5 @@ +mod handlers; + use crate::{ alt, compositor::{self, Component, Compositor, Context, Event, EventResult}, @@ -10,9 +12,11 @@ use crate::{ EditorView, }, }; -use futures_util::{future::BoxFuture, FutureExt}; +use futures_util::future::BoxFuture; +use helix_event::AsyncHook; use nucleo::pattern::CaseMatching; use nucleo::{Config, Nucleo, Utf32String}; +use tokio::sync::mpsc::Sender; use tui::{ buffer::Buffer as Surface, layout::Constraint, @@ -25,7 +29,7 @@ use tui::widgets::Widget; use std::{ collections::HashMap, io::Read, - path::PathBuf, + path::{Path, PathBuf}, sync::{ atomic::{self, AtomicBool}, Arc, @@ -36,7 +40,6 @@ use crate::ui::{Prompt, PromptEvent}; use helix_core::{ char_idx_at_visual_offset, fuzzy::MATCHER, movement::Direction, text_annotations::TextAnnotations, unicode::segmentation::UnicodeSegmentation, Position, - Syntax, }; use helix_view::{ editor::Action, @@ -46,9 +49,12 @@ use helix_view::{ Document, DocumentId, Editor, }; -pub const ID: &str = "picker"; +use self::handlers::PreviewHighlightHandler; + use super::{menu::Item, overlay::Overlay}; +pub const ID: &str = "picker"; + pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72; /// Biggest file size to preview in bytes pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024; @@ -56,14 +62,14 @@ pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024; #[derive(PartialEq, Eq, Hash)] pub enum PathOrId { Id(DocumentId), - Path(PathBuf), + Path(Arc), } impl PathOrId { fn get_canonicalized(self) -> Self { use PathOrId::*; match self { - Path(path) => Path(helix_stdx::path::canonicalize(path)), + Path(path) => Path(helix_stdx::path::canonicalize(&path).into()), Id(id) => Id(id), } } @@ -71,7 +77,7 @@ impl PathOrId { impl From for PathOrId { fn from(v: PathBuf) -> Self { - Self::Path(v) + Self::Path(v.as_path().into()) } } @@ -197,10 +203,12 @@ pub struct Picker { pub truncate_start: bool, /// Caches paths to documents - preview_cache: HashMap, + preview_cache: HashMap, CachedPreview>, read_buffer: Vec, /// Given an item in the picker, return the file path and line number to display. file_fn: Option>, + /// An event handler for syntax highlighting the currently previewed file. + preview_highlight_handler: Sender>, } impl Picker { @@ -280,6 +288,7 @@ impl Picker { preview_cache: HashMap::new(), read_buffer: Vec::with_capacity(1024), file_fn: None, + preview_highlight_handler: PreviewHighlightHandler::::default().spawn(), } } @@ -403,16 +412,26 @@ impl Picker { ) -> Preview<'picker, 'editor> { match path_or_id { PathOrId::Path(path) => { - let path = &path; - if let Some(doc) = editor.document_by_path(path) { + if let Some(doc) = editor.document_by_path(&path) { return Preview::EditorDocument(doc); } - if self.preview_cache.contains_key(path) { - return Preview::Cached(&self.preview_cache[path]); + if self.preview_cache.contains_key(&path) { + let preview = &self.preview_cache[&path]; + match preview { + // If the document isn't highlighted yet, attempt to highlight it. + CachedPreview::Document(doc) if doc.language_config().is_none() => { + helix_event::send_blocking( + &self.preview_highlight_handler, + path.clone(), + ); + } + _ => (), + } + return Preview::Cached(preview); } - let data = std::fs::File::open(path).and_then(|file| { + let data = std::fs::File::open(&path).and_then(|file| { let metadata = file.metadata()?; // Read up to 1kb to detect the content type let n = file.take(1024).read_to_end(&mut self.read_buffer)?; @@ -427,14 +446,21 @@ impl Picker { (size, _) if size > MAX_FILE_SIZE_FOR_PREVIEW => { CachedPreview::LargeFile } - _ => Document::open(path, None, None, editor.config.clone()) - .map(|doc| CachedPreview::Document(Box::new(doc))) + _ => Document::open(&path, None, None, editor.config.clone()) + .map(|doc| { + // Asynchronously highlight the new document + helix_event::send_blocking( + &self.preview_highlight_handler, + path.clone(), + ); + CachedPreview::Document(Box::new(doc)) + }) .unwrap_or(CachedPreview::NotFound), }, ) .unwrap_or(CachedPreview::NotFound); - self.preview_cache.insert(path.to_owned(), preview); - Preview::Cached(&self.preview_cache[path]) + self.preview_cache.insert(path.clone(), preview); + Preview::Cached(&self.preview_cache[&path]) } PathOrId::Id(id) => { let doc = editor.documents.get(&id).unwrap(); @@ -443,84 +469,6 @@ impl Picker { } } - fn handle_idle_timeout(&mut self, cx: &mut Context) -> EventResult { - let Some((current_file, _)) = self.current_file(cx.editor) else { - return EventResult::Consumed(None); - }; - - // Try to find a document in the cache - let doc = match ¤t_file { - PathOrId::Id(doc_id) => doc_mut!(cx.editor, doc_id), - PathOrId::Path(path) => match self.preview_cache.get_mut(path) { - Some(CachedPreview::Document(ref mut doc)) => doc, - _ => return EventResult::Consumed(None), - }, - }; - - let mut callback: Option = None; - - // Then attempt to highlight it if it has no language set - if doc.language_config().is_none() { - if let Some(language_config) = doc.detect_language_config(&cx.editor.syn_loader.load()) - { - doc.language = Some(language_config.clone()); - let text = doc.text().clone(); - let loader = cx.editor.syn_loader.clone(); - let job = tokio::task::spawn_blocking(move || { - let syntax = language_config - .highlight_config(&loader.load().scopes()) - .and_then(|highlight_config| { - Syntax::new(text.slice(..), highlight_config, loader) - }); - let callback = move |editor: &mut Editor, compositor: &mut Compositor| { - let Some(syntax) = syntax else { - log::info!("highlighting picker item failed"); - return; - }; - 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 { - log::info!("picker closed before syntax highlighting finished"); - return; - }; - // Try to find a document in the cache - let doc = match current_file { - PathOrId::Id(doc_id) => doc_mut!(editor, &doc_id), - PathOrId::Path(path) => match picker.preview_cache.get_mut(&path) { - Some(CachedPreview::Document(ref mut doc)) => { - let diagnostics = Editor::doc_diagnostics( - &editor.language_servers, - &editor.diagnostics, - doc, - ); - doc.replace_diagnostics(diagnostics, &[], None); - doc - } - _ => return, - }, - }; - doc.syntax = Some(syntax); - }; - Callback::EditorCompositor(Box::new(callback)) - }); - let tmp: compositor::Callback = Box::new(move |_, ctx| { - ctx.jobs - .callback(job.map(|res| res.map_err(anyhow::Error::from))) - }); - callback = Some(Box::new(tmp)) - } - } - - // QUESTION: do we want to compute inlay hints in pickers too ? Probably not for now - // but it could be interesting in the future - - EventResult::Consumed(callback) - } - fn render_picker(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { let status = self.matcher.tick(10); let snapshot = self.matcher.snapshot(); @@ -826,9 +774,6 @@ impl Component for Picker { } fn handle_event(&mut self, event: &Event, ctx: &mut Context) -> EventResult { - if let Event::IdleTimeout = event { - return self.handle_idle_timeout(ctx); - } // TODO: keybinds for scrolling preview let key_event = match event { @@ -861,9 +806,6 @@ impl Component for Picker { EventResult::Consumed(Some(callback)) }; - // So that idle timeout retriggers - ctx.editor.reset_idle_timer(); - match key_event { shift!(Tab) | key!(Up) | ctrl!('p') => { self.move_by(1, Direction::Backward); @@ -989,7 +931,7 @@ impl Component for DynamicPicker { cx.jobs.callback(async move { let new_options = new_options.await?; - let callback = Callback::EditorCompositor(Box::new(move |editor, compositor| { + 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) { @@ -997,7 +939,6 @@ impl Component for DynamicPicker { None => return, }; picker.set_options(new_options); - editor.reset_idle_timer(); })); anyhow::Ok(callback) }); diff --git a/helix-term/src/ui/picker/handlers.rs b/helix-term/src/ui/picker/handlers.rs new file mode 100644 index 000000000000..7a77efa44aee --- /dev/null +++ b/helix-term/src/ui/picker/handlers.rs @@ -0,0 +1,117 @@ +use std::{path::Path, sync::Arc, time::Duration}; + +use helix_event::AsyncHook; +use tokio::time::Instant; + +use crate::{ + job, + ui::{menu::Item, overlay::Overlay}, +}; + +use super::{CachedPreview, DynamicPicker, Picker}; + +pub(super) struct PreviewHighlightHandler { + trigger: Option>, + phantom_data: std::marker::PhantomData, +} + +impl Default for PreviewHighlightHandler { + fn default() -> Self { + Self { + trigger: None, + phantom_data: Default::default(), + } + } +} + +impl AsyncHook for PreviewHighlightHandler { + type Event = Arc; + + fn handle_event( + &mut self, + path: Self::Event, + timeout: Option, + ) -> Option { + if self + .trigger + .as_ref() + .is_some_and(|trigger| trigger == &path) + { + // If the path hasn't changed, don't reset the debounce + timeout + } else { + self.trigger = Some(path); + Some(Instant::now() + Duration::from_millis(150)) + } + } + + fn finish_debounce(&mut self) { + let Some(path) = self.trigger.take() else { + return; + }; + + 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(CachedPreview::Document(ref mut doc)) = picker.preview_cache.get_mut(&path) + else { + return; + }; + + if doc.language_config().is_some() { + return; + } + + let Some(language_config) = doc.detect_language_config(&editor.syn_loader.load()) + else { + return; + }; + doc.language = Some(language_config.clone()); + let text = doc.text().clone(); + let loader = editor.syn_loader.clone(); + + tokio::task::spawn_blocking(move || { + let Some(syntax) = language_config + .highlight_config(&loader.load().scopes()) + .and_then(|highlight_config| { + helix_core::Syntax::new(text.slice(..), highlight_config, loader) + }) + else { + log::info!("highlighting picker item failed"); + return; + }; + + 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 { + log::info!("picker closed before syntax highlighting finished"); + return; + }; + let Some(CachedPreview::Document(ref mut doc)) = + picker.preview_cache.get_mut(&path) + else { + return; + }; + let diagnostics = helix_view::Editor::doc_diagnostics( + &editor.language_servers, + &editor.diagnostics, + doc, + ); + doc.replace_diagnostics(diagnostics, &[], None); + doc.syntax = Some(syntax); + }); + }); + }); + } +} From f40fca88e00968a74ae17b8ae83f9ff9680671f1 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 10:55:02 -0500 Subject: [PATCH 02/20] Refactor Picker in terms of columns `menu::Item` is replaced with column configurations for each picker which control how a column is displayed and whether it is passed to nucleo for filtering. (This is used for dynamic pickers so that we can filter those items with the dynamic picker callback rather than nucleo.) The picker has a new lucene-like syntax that can be used to filter the picker only on certain criteria. If a filter is not specified, the text in the prompt applies to the picker's configured "primary" column. Adding column configurations for each picker is left for the child commit. --- book/src/themes.md | 1 + helix-term/src/commands.rs | 14 +- helix-term/src/commands/dap.rs | 18 +- helix-term/src/commands/lsp.rs | 35 ++- helix-term/src/commands/typed.rs | 18 +- helix-term/src/ui/mod.rs | 33 ++- helix-term/src/ui/picker.rs | 346 +++++++++++++++++---------- helix-term/src/ui/picker/handlers.rs | 23 +- helix-term/src/ui/prompt.rs | 6 +- 9 files changed, 310 insertions(+), 184 deletions(-) diff --git a/book/src/themes.md b/book/src/themes.md index e3b95c0a7af0..a59df2fd72e1 100644 --- a/book/src/themes.md +++ b/book/src/themes.md @@ -297,6 +297,7 @@ These scopes are used for theming the editor interface: | `ui.bufferline.background` | Style for bufferline background | | `ui.popup` | Documentation popups (e.g. Space + k) | | `ui.popup.info` | Prompt for multiple key options | +| `ui.picker.header` | Column names in pickers with multiple columns | | `ui.window` | Borderlines separating splits | | `ui.help` | Description box for commands | | `ui.text` | Default text style, command prompts, popup text, etc. | diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 69496eb61619..5600a1e49ffc 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2317,7 +2317,9 @@ fn global_search(cx: &mut Context) { return; } - let (picker, injector) = Picker::stream(current_path); + // TODO + let columns = vec![]; + let (picker, injector) = Picker::stream(columns, current_path); let dedup_symlinks = file_picker_config.deduplicate_links; let absolute_root = search_root @@ -2420,6 +2422,7 @@ fn global_search(cx: &mut Context) { let call = move |_: &mut Editor, compositor: &mut Compositor| { let picker = Picker::with_stream( picker, + 0, injector, move |cx, FileResult { path, line_num }, action| { let doc = match cx.editor.open(path, action) { @@ -2937,7 +2940,8 @@ fn buffer_picker(cx: &mut Context) { // mru items.sort_unstable_by_key(|item| std::cmp::Reverse(item.focused_at)); - let picker = Picker::new(items, (), |cx, meta, action| { + let columns = vec![]; + let picker = Picker::new(columns, 0, items, (), |cx, meta, action| { cx.editor.switch(meta.id, action); }) .with_preview(|editor, meta| { @@ -3014,7 +3018,10 @@ fn jumplist_picker(cx: &mut Context) { } }; + let columns = vec![]; let picker = Picker::new( + columns, + 0, cx.editor .tree .views() @@ -3180,7 +3187,8 @@ pub fn command_palette(cx: &mut Context) { } })); - let picker = Picker::new(commands, keymap, move |cx, command, _action| { + let columns = vec![]; + let picker = Picker::new(columns, 0, commands, keymap, move |cx, command, _action| { let mut ctx = Context { register, count, diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 0e50377ac1a3..da2b60dae4b3 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -73,9 +73,14 @@ fn thread_picker( let debugger = debugger!(editor); let thread_states = debugger.thread_states.clone(); - let picker = Picker::new(threads, thread_states, move |cx, thread, _action| { - callback_fn(cx.editor, thread) - }) + let columns = vec![]; + let picker = Picker::new( + columns, + 0, + threads, + thread_states, + move |cx, thread, _action| callback_fn(cx.editor, thread), + ) .with_preview(move |editor, thread| { let frames = editor.debugger.as_ref()?.stack_frames.get(&thread.id)?; let frame = frames.first()?; @@ -268,7 +273,11 @@ pub fn dap_launch(cx: &mut Context) { let templates = config.templates.clone(); + let columns = vec![]; + cx.push_layer(Box::new(overlaid(Picker::new( + columns, + 0, templates, (), |cx, template, _action| { @@ -736,7 +745,8 @@ pub fn dap_switch_stack_frame(cx: &mut Context) { let frames = debugger.stack_frames[&thread_id].clone(); - let picker = Picker::new(frames, (), move |cx, frame, _action| { + let columns = vec![]; + let picker = Picker::new(columns, 0, frames, (), move |cx, frame, _action| { let debugger = debugger!(cx.editor); // TODO: this should be simpler to find let pos = debugger.stack_frames[&thread_id] diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index d585e1bedd6d..bf9747a4cc9b 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -241,18 +241,25 @@ fn jump_to_position( } } -type SymbolPicker = Picker; +type SymbolPicker = Picker>; fn sym_picker(symbols: Vec, current_path: Option) -> SymbolPicker { // TODO: drop current_path comparison and instead use workspace: bool flag? - Picker::new(symbols, current_path, move |cx, item, action| { - jump_to_location( - cx.editor, - &item.symbol.location, - item.offset_encoding, - action, - ); - }) + let columns = vec![]; + Picker::new( + columns, + 0, + symbols, + current_path, + move |cx, item, action| { + jump_to_location( + cx.editor, + &item.symbol.location, + item.offset_encoding, + action, + ); + }, + ) .with_preview(move |_editor, item| Some(location_to_file_location(&item.symbol.location))) .truncate_start(false) } @@ -263,11 +270,13 @@ enum DiagnosticsFormat { HideSourcePath, } +type DiagnosticsPicker = Picker; + fn diag_picker( cx: &Context, diagnostics: BTreeMap>, format: DiagnosticsFormat, -) -> Picker { +) -> DiagnosticsPicker { // TODO: drop current_path comparison and instead use workspace: bool flag? // flatten the map to a vec of (url, diag) pairs @@ -293,7 +302,10 @@ fn diag_picker( error: cx.editor.theme.get("error"), }; + let columns = vec![]; Picker::new( + columns, + 0, flat_diag, (styles, format), move |cx, @@ -817,7 +829,8 @@ fn goto_impl( } [] => unreachable!("`locations` should be non-empty for `goto_impl`"), _locations => { - let picker = Picker::new(locations, cwdir, move |cx, location, action| { + let columns = vec![]; + let picker = Picker::new(columns, 0, locations, cwdir, move |cx, location, action| { jump_to_location(cx.editor, location, offset_encoding, action) }) .with_preview(move |_editor, location| Some(location_to_file_location(location))); diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index ed1547f1f5ad..232e5846b830 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -9,7 +9,6 @@ use super::*; use helix_core::fuzzy::fuzzy_match; use helix_core::indent::MAX_INDENT; use helix_core::{line_ending, shellwords::Shellwords}; -use helix_lsp::LanguageServerId; use helix_view::document::{read_to_string, DEFAULT_LANGUAGE_NAME}; use helix_view::editor::{CloseError, ConfigEvent}; use serde_json::Value; @@ -1378,16 +1377,6 @@ fn lsp_workspace_command( return Ok(()); } - struct LsIdCommand(LanguageServerId, helix_lsp::lsp::Command); - - impl ui::menu::Item for LsIdCommand { - type Data = (); - - fn format(&self, _data: &Self::Data) -> Row { - self.1.title.as_str().into() - } - } - let doc = doc!(cx.editor); let ls_id_commands = doc .language_servers_with_feature(LanguageServerFeature::WorkspaceCommand) @@ -1402,7 +1391,7 @@ fn lsp_workspace_command( if args.is_empty() { let commands = ls_id_commands .map(|(ls_id, command)| { - LsIdCommand( + ( ls_id, helix_lsp::lsp::Command { title: command.clone(), @@ -1415,10 +1404,13 @@ fn lsp_workspace_command( let callback = async move { let call: job::Callback = Callback::EditorCompositor(Box::new( move |_editor: &mut Editor, compositor: &mut Compositor| { + let columns = vec![]; let picker = ui::Picker::new( + columns, + 0, commands, (), - move |cx, LsIdCommand(ls_id, command), _action| { + move |cx, (ls_id, command), _action| { execute_lsp_command(cx.editor, *ls_id, command.clone()); }, ); diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 0a65b12b5fa9..01b718d45da9 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::{DynamicPicker, FileLocation, Picker}; +pub use picker::{Column as PickerColumn, DynamicPicker, FileLocation, Picker}; pub use popup::Popup; pub use prompt::{Prompt, PromptEvent}; pub use spinner::{ProgressSpinners, Spinner}; @@ -170,7 +170,9 @@ pub fn raw_regex_prompt( cx.push_layer(Box::new(prompt)); } -pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> Picker { +type FilePicker = Picker; + +pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePicker { use ignore::{types::TypesBuilder, WalkBuilder}; use std::time::Instant; @@ -217,16 +219,23 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> Picker }); log::debug!("file_picker init {:?}", Instant::now().duration_since(now)); - let picker = Picker::new(Vec::new(), root, move |cx, path: &PathBuf, action| { - if let Err(e) = cx.editor.open(path, action) { - let err = if let Some(err) = e.source() { - format!("{}", err) - } else { - format!("unable to open \"{}\"", path.display()) - }; - cx.editor.set_error(err); - } - }) + let columns = vec![]; + let picker = Picker::new( + columns, + 0, + Vec::new(), + root, + move |cx, path: &PathBuf, action| { + if let Err(e) = cx.editor.open(path, action) { + let err = if let Some(err) = e.source() { + format!("{}", err) + } else { + format!("unable to open \"{}\"", path.display()) + }; + cx.editor.set_error(err); + } + }, + ) .with_preview(|_editor, path| Some((path.clone().into(), None))); let injector = picker.injector(); let timeout = std::time::Instant::now() + std::time::Duration::from_millis(30); diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index cc86a4faddab..ab8e4e1553d0 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -21,12 +21,13 @@ use tui::{ buffer::Buffer as Surface, layout::Constraint, text::{Span, Spans}, - widgets::{Block, BorderType, Cell, Table}, + widgets::{Block, BorderType, Cell, Row, Table}, }; use tui::widgets::Widget; use std::{ + borrow::Cow, collections::HashMap, io::Read, path::{Path, PathBuf}, @@ -49,9 +50,9 @@ use helix_view::{ Document, DocumentId, Editor, }; -use self::handlers::PreviewHighlightHandler; +use super::overlay::Overlay; -use super::{menu::Item, overlay::Overlay}; +use self::handlers::PreviewHighlightHandler; pub const ID: &str = "picker"; @@ -129,38 +130,36 @@ impl Preview<'_, '_> { } } -fn item_to_nucleo(item: T, editor_data: &T::Data) -> Option<(T, Utf32String)> { - let row = item.format(editor_data); - let mut cells = row.cells.iter(); - let mut text = String::with_capacity(row.cell_text().map(|cell| cell.len()).sum()); - let cell = cells.next()?; - if let Some(cell) = cell.content.lines.first() { - for span in &cell.0 { - text.push_str(&span.content); - } - } - - for cell in cells { - text.push(' '); - if let Some(cell) = cell.content.lines.first() { - for span in &cell.0 { - text.push_str(&span.content); - } +fn inject_nucleo_item( + injector: &nucleo::Injector, + columns: &[Column], + item: T, + editor_data: &D, +) { + let column_texts: Vec = columns + .iter() + .filter(|column| column.filter) + .map(|column| column.format_text(&item, editor_data).into()) + .collect(); + injector.push(item, |dst| { + for (i, text) in column_texts.into_iter().enumerate() { + dst[i] = text; } - } - Some((item, text.into())) + }); } -pub struct Injector { +pub struct Injector { dst: nucleo::Injector, - editor_data: Arc, + columns: Arc<[Column]>, + editor_data: Arc, shutown: Arc, } -impl Clone for Injector { +impl Clone for Injector { fn clone(&self) -> Self { Injector { dst: self.dst.clone(), + columns: self.columns.clone(), editor_data: self.editor_data.clone(), shutown: self.shutown.clone(), } @@ -169,21 +168,56 @@ impl Clone for Injector { pub struct InjectorShutdown; -impl Injector { +impl Injector { pub fn push(&self, item: T) -> Result<(), InjectorShutdown> { if self.shutown.load(atomic::Ordering::Relaxed) { return Err(InjectorShutdown); } - if let Some((item, matcher_text)) = item_to_nucleo(item, &self.editor_data) { - self.dst.push(item, |dst| dst[0] = matcher_text); - } + inject_nucleo_item(&self.dst, &self.columns, item, &self.editor_data); Ok(()) } } -pub struct Picker { - editor_data: Arc, +type ColumnFormatFn = for<'a> fn(&'a T, &'a D) -> Cell<'a>; + +pub struct Column { + name: Arc, + format: ColumnFormatFn, + /// Whether the column should be passed to nucleo for matching and filtering. + /// `DynamicPicker` uses this so that the dynamic column (for example regex in + /// global search) is not used for filtering twice. + filter: bool, +} + +impl Column { + pub fn new(name: impl Into>, format: ColumnFormatFn) -> Self { + Self { + name: name.into(), + format, + filter: true, + } + } + + pub fn without_filtering(mut self) -> Self { + self.filter = false; + self + } + + fn format<'a>(&self, item: &'a T, data: &'a D) -> Cell<'a> { + (self.format)(item, data) + } + + fn format_text<'a>(&self, item: &'a T, data: &'a D) -> Cow<'a, str> { + let text: String = self.format(item, data).content.into(); + text.into() + } +} + +pub struct Picker { + columns: Arc<[Column]>, + primary_column: usize, + editor_data: Arc, shutdown: Arc, matcher: Nucleo, @@ -211,16 +245,19 @@ pub struct Picker { preview_highlight_handler: Sender>, } -impl Picker { - pub fn stream(editor_data: T::Data) -> (Nucleo, Injector) { +impl Picker { + pub fn stream(columns: Vec>, editor_data: D) -> (Nucleo, Injector) { + let matcher_columns = columns.iter().filter(|col| col.filter).count() as u32; + assert!(matcher_columns > 0); let matcher = Nucleo::new( Config::DEFAULT, Arc::new(helix_event::request_redraw), None, - 1, + matcher_columns, ); let streamer = Injector { dst: matcher.injector(), + columns: columns.into(), editor_data: Arc::new(editor_data), shutown: Arc::new(AtomicBool::new(false)), }; @@ -228,24 +265,28 @@ impl Picker { } pub fn new( + columns: Vec>, + primary_column: usize, options: Vec, - editor_data: T::Data, + editor_data: D, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { + let matcher_columns = columns.iter().filter(|col| col.filter).count() as u32; + assert!(matcher_columns > 0); let matcher = Nucleo::new( Config::DEFAULT, Arc::new(helix_event::request_redraw), None, - 1, + matcher_columns, ); let injector = matcher.injector(); for item in options { - if let Some((item, matcher_text)) = item_to_nucleo(item, &editor_data) { - injector.push(item, |dst| dst[0] = matcher_text); - } + inject_nucleo_item(&injector, &columns, item, &editor_data); } Self::with( matcher, + columns.into(), + primary_column, Arc::new(editor_data), Arc::new(AtomicBool::new(false)), callback_fn, @@ -254,18 +295,30 @@ impl Picker { pub fn with_stream( matcher: Nucleo, - injector: Injector, + primary_column: usize, + injector: Injector, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { - Self::with(matcher, injector.editor_data, injector.shutown, callback_fn) + Self::with( + matcher, + injector.columns, + primary_column, + injector.editor_data, + injector.shutown, + callback_fn, + ) } fn with( matcher: Nucleo, - editor_data: Arc, + columns: Arc<[Column]>, + default_column: usize, + editor_data: Arc, shutdown: Arc, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { + assert!(!columns.is_empty()); + let prompt = Prompt::new( "".into(), None, @@ -273,7 +326,14 @@ impl Picker { |_editor: &mut Context, _pattern: &str, _event: PromptEvent| {}, ); + let widths = columns + .iter() + .map(|column| Constraint::Length(column.name.chars().count() as u16)) + .collect(); + Self { + columns, + primary_column: default_column, matcher, editor_data, shutdown, @@ -284,17 +344,18 @@ impl Picker { show_preview: true, callback_fn: Box::new(callback_fn), completion_height: 0, - widths: Vec::new(), + widths, preview_cache: HashMap::new(), read_buffer: Vec::with_capacity(1024), file_fn: None, - preview_highlight_handler: PreviewHighlightHandler::::default().spawn(), + preview_highlight_handler: PreviewHighlightHandler::::default().spawn(), } } - pub fn injector(&self) -> Injector { + pub fn injector(&self) -> Injector { Injector { dst: self.matcher.injector(), + columns: self.columns.clone(), editor_data: self.editor_data.clone(), shutown: self.shutdown.clone(), } @@ -316,13 +377,17 @@ impl Picker { self } + pub fn with_line(mut self, line: String, editor: &Editor) -> Self { + self.prompt.set_line(line, editor); + self.handle_prompt_change(); + self + } + pub fn set_options(&mut self, new_options: Vec) { self.matcher.restart(false); let injector = self.matcher.injector(); for item in new_options { - if let Some((item, matcher_text)) = item_to_nucleo(item, &self.editor_data) { - injector.push(item, |dst| dst[0] = matcher_text); - } + inject_nucleo_item(&injector, &self.columns, item, &self.editor_data); } } @@ -376,27 +441,39 @@ impl Picker { .map(|item| item.data) } + fn header_height(&self) -> u16 { + if self.columns.len() > 1 { + 1 + } else { + 0 + } + } + pub fn toggle_preview(&mut self) { self.show_preview = !self.show_preview; } fn prompt_handle_event(&mut self, event: &Event, cx: &mut Context) -> EventResult { if let EventResult::Consumed(_) = self.prompt.handle_event(event, cx) { - let pattern = self.prompt.line(); - // TODO: better track how the pattern has changed - if pattern != &self.previous_pattern { - self.matcher.pattern.reparse( - 0, - pattern, - CaseMatching::Smart, - pattern.starts_with(&self.previous_pattern), - ); - self.previous_pattern = pattern.clone(); - } + self.handle_prompt_change(); } EventResult::Consumed(None) } + fn handle_prompt_change(&mut self) { + let pattern = self.prompt.line(); + // TODO: better track how the pattern has changed + if pattern != &self.previous_pattern { + self.matcher.pattern.reparse( + 0, + pattern, + CaseMatching::Smart, + pattern.starts_with(&self.previous_pattern), + ); + self.previous_pattern = pattern.clone(); + } + } + fn current_file(&self, editor: &Editor) -> Option { self.selection() .and_then(|current| (self.file_fn.as_ref()?)(editor, current)) @@ -526,7 +603,7 @@ impl Picker { // -- Render the contents: // subtract area of prompt from top let inner = inner.clip_top(2); - let rows = inner.height as u32; + let rows = inner.height.saturating_sub(self.header_height()) as u32; let offset = self.cursor - (self.cursor % std::cmp::max(1, rows)); let cursor = self.cursor.saturating_sub(offset); let end = offset @@ -540,83 +617,94 @@ impl Picker { } let options = snapshot.matched_items(offset..end).map(|item| { - snapshot.pattern().column_pattern(0).indices( - item.matcher_columns[0].slice(..), - &mut matcher, - &mut indices, - ); - indices.sort_unstable(); - indices.dedup(); - let mut row = item.data.format(&self.editor_data); - - let mut grapheme_idx = 0u32; - let mut indices = indices.drain(..); - let mut next_highlight_idx = indices.next().unwrap_or(u32::MAX); - if self.widths.len() < row.cells.len() { - self.widths.resize(row.cells.len(), Constraint::Length(0)); - } let mut widths = self.widths.iter_mut(); - for cell in &mut row.cells { + let mut matcher_index = 0; + + Row::new(self.columns.iter().map(|column| { let Some(Constraint::Length(max_width)) = widths.next() else { unreachable!(); }; - - // merge index highlights on top of existing hightlights - let mut span_list = Vec::new(); - let mut current_span = String::new(); - let mut current_style = Style::default(); - let mut width = 0; - - let spans: &[Span] = cell.content.lines.first().map_or(&[], |it| it.0.as_slice()); - for span in spans { - // this looks like a bug on first glance, we are iterating - // graphemes but treating them as char indices. The reason that - // this is correct is that nucleo will only ever consider the first char - // of a grapheme (and discard the rest of the grapheme) so the indices - // returned by nucleo are essentially grapheme indecies - for grapheme in span.content.graphemes(true) { - let style = if grapheme_idx == next_highlight_idx { - next_highlight_idx = indices.next().unwrap_or(u32::MAX); - span.style.patch(highlight_style) - } else { - span.style - }; - if style != current_style { - if !current_span.is_empty() { - span_list.push(Span::styled(current_span, current_style)) + let mut cell = column.format(item.data, &self.editor_data); + let width = if column.filter { + snapshot.pattern().column_pattern(matcher_index).indices( + item.matcher_columns[matcher_index].slice(..), + &mut matcher, + &mut indices, + ); + indices.sort_unstable(); + indices.dedup(); + let mut indices = indices.drain(..); + let mut next_highlight_idx = indices.next().unwrap_or(u32::MAX); + let mut span_list = Vec::new(); + let mut current_span = String::new(); + let mut current_style = Style::default(); + let mut grapheme_idx = 0u32; + let mut width = 0; + + let spans: &[Span] = + cell.content.lines.first().map_or(&[], |it| it.0.as_slice()); + for span in spans { + // this looks like a bug on first glance, we are iterating + // graphemes but treating them as char indices. The reason that + // this is correct is that nucleo will only ever consider the first char + // of a grapheme (and discard the rest of the grapheme) so the indices + // returned by nucleo are essentially grapheme indecies + for grapheme in span.content.graphemes(true) { + let style = if grapheme_idx == next_highlight_idx { + next_highlight_idx = indices.next().unwrap_or(u32::MAX); + span.style.patch(highlight_style) + } else { + span.style + }; + if style != current_style { + if !current_span.is_empty() { + span_list.push(Span::styled(current_span, current_style)) + } + current_span = String::new(); + current_style = style; } - current_span = String::new(); - current_style = style; + current_span.push_str(grapheme); + grapheme_idx += 1; } - current_span.push_str(grapheme); - grapheme_idx += 1; + width += span.width(); } - width += span.width(); - } - span_list.push(Span::styled(current_span, current_style)); + span_list.push(Span::styled(current_span, current_style)); + cell = Cell::from(Spans::from(span_list)); + matcher_index += 1; + width + } else { + cell.content + .lines + .first() + .map(|line| line.width()) + .unwrap_or_default() + }; + if width as u16 > *max_width { *max_width = width as u16; } - *cell = Cell::from(Spans::from(span_list)); - // spacer - if grapheme_idx == next_highlight_idx { - next_highlight_idx = indices.next().unwrap_or(u32::MAX); - } - grapheme_idx += 1; - } - - row + cell + })) }); - let table = Table::new(options) + let mut table = Table::new(options) .style(text_style) .highlight_style(selected) .highlight_symbol(" > ") .column_spacing(1) .widths(&self.widths); + // -- Header + if self.columns.len() > 1 { + let header_style = cx.editor.theme.get("ui.picker.header"); + + table = table.header(Row::new(self.columns.iter().map(|column| { + Cell::from(Span::styled(Cow::from(&*column.name), header_style)) + }))); + } + use tui::widgets::TableState; table.render_table( @@ -746,7 +834,7 @@ impl Picker { } } -impl Component for Picker { +impl Component for Picker { fn render(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { // +---------+ +---------+ // |prompt | |preview | @@ -872,7 +960,7 @@ impl Component for Picker { } fn required_size(&mut self, (width, height): (u16, u16)) -> Option<(u16, u16)> { - self.completion_height = height.saturating_sub(4); + self.completion_height = height.saturating_sub(4 + self.header_height()); Some((width, height)) } @@ -880,7 +968,7 @@ impl Component for Picker { Some(ID) } } -impl Drop for Picker { +impl Drop for Picker { fn drop(&mut self) { // ensure we cancel any ongoing background threads streaming into the picker self.shutdown.store(true, atomic::Ordering::Relaxed) @@ -896,14 +984,14 @@ pub type DynQueryCallback = /// 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, +pub struct DynamicPicker { + file_picker: Picker, query_callback: DynQueryCallback, query: String, } -impl DynamicPicker { - pub fn new(file_picker: Picker, query_callback: DynQueryCallback) -> Self { +impl DynamicPicker { + pub fn new(file_picker: Picker, query_callback: DynQueryCallback) -> Self { Self { file_picker, query_callback, @@ -912,20 +1000,22 @@ impl DynamicPicker { } } -impl Component for DynamicPicker { +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 current_query = self.file_picker.prompt.line(); + 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.clone_from(current_query); + self.query = current_query.to_string(); let new_options = (self.query_callback)(current_query.to_owned(), cx.editor); @@ -934,7 +1024,7 @@ impl Component for DynamicPicker { 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) { + let picker = match compositor.find_id::>(ID) { Some(overlay) => &mut overlay.content.file_picker, None => return, }; diff --git a/helix-term/src/ui/picker/handlers.rs b/helix-term/src/ui/picker/handlers.rs index 7a77efa44aee..f01c982a7ce0 100644 --- a/helix-term/src/ui/picker/handlers.rs +++ b/helix-term/src/ui/picker/handlers.rs @@ -3,19 +3,16 @@ use std::{path::Path, sync::Arc, time::Duration}; use helix_event::AsyncHook; use tokio::time::Instant; -use crate::{ - job, - ui::{menu::Item, overlay::Overlay}, -}; +use crate::{job, ui::overlay::Overlay}; use super::{CachedPreview, DynamicPicker, Picker}; -pub(super) struct PreviewHighlightHandler { +pub(super) struct PreviewHighlightHandler { trigger: Option>, - phantom_data: std::marker::PhantomData, + phantom_data: std::marker::PhantomData<(T, D)>, } -impl Default for PreviewHighlightHandler { +impl Default for PreviewHighlightHandler { fn default() -> Self { Self { trigger: None, @@ -24,7 +21,9 @@ impl Default for PreviewHighlightHandler { } } -impl AsyncHook for PreviewHighlightHandler { +impl AsyncHook + for PreviewHighlightHandler +{ type Event = Arc; fn handle_event( @@ -51,9 +50,9 @@ impl AsyncHook for PreviewHighlightHandler { }; job::dispatch_blocking(move |editor, compositor| { - let picker = match compositor.find::>>() { + let picker = match compositor.find::>>() { Some(Overlay { content, .. }) => content, - None => match compositor.find::>>() { + None => match compositor.find::>>() { Some(Overlay { content, .. }) => &mut content.file_picker, None => return, }, @@ -88,10 +87,10 @@ impl AsyncHook for PreviewHighlightHandler { }; job::dispatch_blocking(move |editor, compositor| { - let picker = match compositor.find::>>() { + let picker = match compositor.find::>>() { Some(Overlay { content, .. }) => Some(content), None => compositor - .find::>>() + .find::>>() .map(|overlay| &mut overlay.content.file_picker), }; let Some(picker) = picker else { diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 14b242df6672..19183470c7a6 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -93,11 +93,15 @@ impl Prompt { } pub fn with_line(mut self, line: String, editor: &Editor) -> Self { + self.set_line(line, editor); + self + } + + pub fn set_line(&mut self, line: String, editor: &Editor) { let cursor = line.len(); self.line = line; self.cursor = cursor; self.recalculate_completion(editor); - self } pub fn with_language( From c4c17c693de839ab48fc7c43f46aa25c5473c583 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 10:57:38 -0500 Subject: [PATCH 03/20] Add a special query syntax for Pickers to select columns Now that the picker is defined as a table, we need a way to provide input for each field in the picker. We introduce a small query syntax that supports multiple columns without being too verbose. Fields are specified as `%field pattern`. The default column for a picker doesn't need the `%field` prefix. The field name may be selected by a prefix of the field, for example `%p foo.rs` rather than `%path foo.rs`. Co-authored-by: ItsEthra <107059409+ItsEthra@users.noreply.github.com> --- helix-term/src/ui/picker.rs | 53 ++++-- helix-term/src/ui/picker/query.rs | 282 ++++++++++++++++++++++++++++++ 2 files changed, 324 insertions(+), 11 deletions(-) create mode 100644 helix-term/src/ui/picker/query.rs diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index ab8e4e1553d0..5e7bda21bcf4 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -1,4 +1,5 @@ mod handlers; +mod query; use crate::{ alt, @@ -9,6 +10,7 @@ use crate::{ ui::{ self, document::{render_document, LineDecoration, LinePos, TextRenderer}, + picker::query::PickerQuery, EditorView, }, }; @@ -226,7 +228,7 @@ pub struct Picker { cursor: u32, prompt: Prompt, - previous_pattern: String, + query: PickerQuery, /// Whether to show the preview panel (default true) show_preview: bool, @@ -331,6 +333,8 @@ impl Picker { .map(|column| Constraint::Length(column.name.chars().count() as u16)) .collect(); + let query = PickerQuery::new(columns.iter().map(|col| &col.name).cloned(), default_column); + Self { columns, primary_column: default_column, @@ -339,7 +343,7 @@ impl Picker { shutdown, cursor: 0, prompt, - previous_pattern: String::new(), + query, truncate_start: true, show_preview: true, callback_fn: Box::new(callback_fn), @@ -441,6 +445,13 @@ impl Picker { .map(|item| item.data) } + fn primary_query(&self) -> Arc { + self.query + .get(&self.columns[self.primary_column].name) + .cloned() + .unwrap_or_else(|| "".into()) + } + fn header_height(&self) -> u16 { if self.columns.len() > 1 { 1 @@ -461,16 +472,36 @@ impl Picker { } fn handle_prompt_change(&mut self) { - let pattern = self.prompt.line(); // TODO: better track how the pattern has changed - if pattern != &self.previous_pattern { - self.matcher.pattern.reparse( - 0, - pattern, - CaseMatching::Smart, - pattern.starts_with(&self.previous_pattern), - ); - self.previous_pattern = pattern.clone(); + let line = self.prompt.line(); + let old_query = self.query.parse(line); + if self.query == old_query { + return; + } + // Have nucleo reparse each changed column. + for (i, column) in self + .columns + .iter() + .filter(|column| column.filter) + .enumerate() + { + let pattern = self + .query + .get(&column.name) + .map(|f| &**f) + .unwrap_or_default(); + let old_pattern = old_query + .get(&column.name) + .map(|f| &**f) + .unwrap_or_default(); + // Fastlane: most columns will remain unchanged after each edit. + if pattern == old_pattern { + continue; + } + let is_append = pattern.starts_with(old_pattern); + self.matcher + .pattern + .reparse(i, pattern, CaseMatching::Smart, is_append); } } diff --git a/helix-term/src/ui/picker/query.rs b/helix-term/src/ui/picker/query.rs new file mode 100644 index 000000000000..89ade95ff4a1 --- /dev/null +++ b/helix-term/src/ui/picker/query.rs @@ -0,0 +1,282 @@ +use std::{collections::HashMap, mem, sync::Arc}; + +#[derive(Debug)] +pub(super) struct PickerQuery { + /// The column names of the picker. + column_names: Box<[Arc]>, + /// The index of the primary column in `column_names`. + /// The primary column is selected by default unless another + /// field is specified explicitly with `%fieldname`. + primary_column: usize, + /// The mapping between column names and input in the query + /// for those columns. + inner: HashMap, Arc>, +} + +impl PartialEq, Arc>> for PickerQuery { + fn eq(&self, other: &HashMap, Arc>) -> bool { + self.inner.eq(other) + } +} + +impl PickerQuery { + pub(super) fn new>>( + column_names: I, + primary_column: usize, + ) -> Self { + let column_names: Box<[_]> = column_names.collect(); + let inner = HashMap::with_capacity(column_names.len()); + Self { + column_names, + primary_column, + inner, + } + } + + pub(super) fn get(&self, column: &str) -> Option<&Arc> { + self.inner.get(column) + } + + pub(super) fn parse(&mut self, input: &str) -> HashMap, Arc> { + let mut fields: HashMap, String> = HashMap::new(); + let primary_field = &self.column_names[self.primary_column]; + let mut escaped = false; + let mut in_field = false; + let mut field = None; + let mut text = String::new(); + + macro_rules! finish_field { + () => { + let key = field.take().unwrap_or(primary_field); + + if let Some(pattern) = fields.get_mut(key) { + pattern.push(' '); + pattern.push_str(text.trim()); + } else { + fields.insert(key.clone(), text.trim().to_string()); + } + text.clear(); + }; + } + + for ch in input.chars() { + match ch { + // Backslash escaping + _ if escaped => { + // '%' is the only character that is special cased. + // You can escape it to prevent parsing the text that + // follows it as a field name. + if ch != '%' { + text.push('\\'); + } + text.push(ch); + escaped = false; + } + '\\' => escaped = !escaped, + '%' => { + if !text.is_empty() { + finish_field!(); + } + in_field = true; + } + ' ' if in_field => { + // Go over all columns and their indices, find all that starts with field key, + // select a column that fits key the most. + field = self + .column_names + .iter() + .filter(|col| col.starts_with(&text)) + // select "fittest" column + .min_by_key(|col| col.len()); + text.clear(); + in_field = false; + } + _ => text.push(ch), + } + } + + if !in_field && !text.is_empty() { + finish_field!(); + } + + let new_inner: HashMap<_, _> = fields + .into_iter() + .map(|(field, query)| (field, query.as_str().into())) + .collect(); + + mem::replace(&mut self.inner, new_inner) + } +} + +#[cfg(test)] +mod test { + use helix_core::hashmap; + + use super::*; + + #[test] + fn parse_query_test() { + let mut query = PickerQuery::new( + [ + "primary".into(), + "field1".into(), + "field2".into(), + "another".into(), + "anode".into(), + ] + .into_iter(), + 0, + ); + + // Basic field splitting + query.parse("hello world"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello world".into(), + ) + ); + query.parse("hello %field1 world %field2 !"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "field1".into() => "world".into(), + "field2".into() => "!".into(), + ) + ); + query.parse("%field1 abc %field2 def xyz"); + assert_eq!( + query, + hashmap!( + "field1".into() => "abc".into(), + "field2".into() => "def xyz".into(), + ) + ); + + // Trailing space is trimmed + query.parse("hello "); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + ) + ); + + // Unknown fields are trimmed. + query.parse("hello %foo"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + ) + ); + + // Multiple words in a field + query.parse("hello %field1 a b c"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "field1".into() => "a b c".into(), + ) + ); + + // Escaping + query.parse(r#"hello\ world"#); + assert_eq!( + query, + hashmap!( + "primary".into() => r#"hello\ world"#.into(), + ) + ); + query.parse(r#"hello \%field1 world"#); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello %field1 world".into(), + ) + ); + query.parse(r#"%field1 hello\ world"#); + assert_eq!( + query, + hashmap!( + "field1".into() => r#"hello\ world"#.into(), + ) + ); + query.parse(r#"hello %field1 a\"b"#); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "field1".into() => r#"a\"b"#.into(), + ) + ); + query.parse(r#"%field1 hello\ world"#); + assert_eq!( + query, + hashmap!( + "field1".into() => r#"hello\ world"#.into(), + ) + ); + query.parse(r#"\bfoo\b"#); + assert_eq!( + query, + hashmap!( + "primary".into() => r#"\bfoo\b"#.into(), + ) + ); + query.parse(r#"\\n"#); + assert_eq!( + query, + hashmap!( + "primary".into() => r#"\\n"#.into(), + ) + ); + + // Only the prefix of a field is required. + query.parse("hello %anot abc"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "another".into() => "abc".into(), + ) + ); + // The shortest matching the prefix is selected. + query.parse("hello %ano abc"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "anode".into() => "abc".into() + ) + ); + // Multiple uses of a column are concatenated with space separators. + query.parse("hello %field1 xyz %fie abc"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "field1".into() => "xyz abc".into() + ) + ); + query.parse("hello %fie abc"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello".into(), + "field1".into() => "abc".into() + ) + ); + // The primary column can be explicitly qualified. + query.parse("hello %fie abc %prim world"); + assert_eq!( + query, + hashmap!( + "primary".into() => "hello world".into(), + "field1".into() => "abc".into() + ) + ); + } +} From 385b398808b38dc7837e7e4516a9137c5b9feed0 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 3 Sep 2023 14:47:17 -0500 Subject: [PATCH 04/20] Add column configurations for existing pickers This removes the menu::Item implementations for picker item types and adds `Vec>` configurations. --- helix-term/src/commands.rs | 425 ++++++++++++++++--------------- helix-term/src/commands/dap.rs | 54 ++-- helix-term/src/commands/lsp.rs | 274 +++++++++++--------- helix-term/src/commands/typed.rs | 7 +- helix-term/src/ui/menu.rs | 14 +- helix-term/src/ui/mod.rs | 10 +- 6 files changed, 396 insertions(+), 388 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 5600a1e49ffc..f4d2c634ce80 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -10,10 +10,7 @@ use helix_stdx::{ }; use helix_vcs::{FileChange, Hunk}; pub use lsp::*; -use tui::{ - text::Span, - widgets::{Cell, Row}, -}; +use tui::text::Span; pub use typed::*; use helix_core::{ @@ -61,8 +58,7 @@ use crate::{ compositor::{self, Component, Compositor}, filter_picker_entry, job::Callback, - keymap::ReverseKeymap, - ui::{self, menu::Item, overlay::overlaid, Picker, Popup, Prompt, PromptEvent}, + ui::{self, overlay::overlaid, Picker, PickerColumn, Popup, Prompt, PromptEvent}, }; use crate::job::{self, Jobs}; @@ -2246,32 +2242,15 @@ fn global_search(cx: &mut Context) { path: PathBuf, /// 0 indexed lines line_num: usize, + line_content: String, } impl FileResult { - fn new(path: &Path, line_num: usize) -> Self { + fn new(path: &Path, line_num: usize, line_content: String) -> Self { Self { path: path.to_path_buf(), line_num, - } - } - } - - impl ui::menu::Item for FileResult { - type Data = Option; - - fn format(&self, current_path: &Self::Data) -> Row { - let relative_path = helix_stdx::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() + line_content, } } } @@ -2317,8 +2296,28 @@ fn global_search(cx: &mut Context) { return; } - // TODO - let columns = vec![]; + let columns = vec![ + PickerColumn::new( + "path", + |item: &FileResult, current_path: &Option| { + let relative_path = helix_stdx::path::get_relative_path(&item.path) + .to_string_lossy() + .into_owned(); + if current_path + .as_ref() + .map(|p| p == &item.path) + .unwrap_or(false) + { + format!("{} (*)", relative_path).into() + } else { + relative_path.into() + } + }, + ), + PickerColumn::new("contents", |item: &FileResult, _| { + item.line_content.as_str().into() + }), + ]; let (picker, injector) = Picker::stream(columns, current_path); let dedup_symlinks = file_picker_config.deduplicate_links; @@ -2345,77 +2344,79 @@ fn global_search(cx: &mut Context) { .max_depth(file_picker_config.max_depth) .filter_entry(move |entry| { filter_picker_entry(entry, &absolute_root, dedup_symlinks) - }); + }) + .add_custom_ignore_filename(helix_loader::config_dir().join("ignore")) + .add_custom_ignore_filename(".helix/ignore") + .build_parallel() + .run(|| { + let mut searcher = searcher.clone(); + let matcher = matcher.clone(); + let injector = injector_.clone(); + let documents = &documents; + Box::new(move |entry: Result| -> WalkState { + let entry = match entry { + Ok(entry) => entry, + Err(_) => return WalkState::Continue, + }; - walk_builder - .add_custom_ignore_filename(helix_loader::config_dir().join("ignore")); - walk_builder.add_custom_ignore_filename(".helix/ignore"); - - walk_builder.build_parallel().run(|| { - let mut searcher = searcher.clone(); - let matcher = matcher.clone(); - let injector = injector_.clone(); - let documents = &documents; - Box::new(move |entry: Result| -> WalkState { - let entry = match entry { - Ok(entry) => entry, - Err(_) => return WalkState::Continue, - }; - - match entry.file_type() { - Some(entry) if entry.is_file() => {} - // skip everything else - _ => return WalkState::Continue, - }; - - let mut stop = false; - let sink = sinks::UTF8(|line_num, _| { - stop = injector - .push(FileResult::new(entry.path(), line_num as usize - 1)) - .is_err(); - - Ok(!stop) - }); - let doc = documents.iter().find(|&(doc_path, _)| { - doc_path - .as_ref() - .map_or(false, |doc_path| doc_path == entry.path()) - }); - - let result = if let Some((_, doc)) = doc { - // there is already a buffer for this file - // search the buffer instead of the file because it's faster - // and captures new edits without requiring a save - if searcher.multi_line_with_matcher(&matcher) { - // in this case a continous buffer is required - // convert the rope to a string - let text = doc.to_string(); - searcher.search_slice(&matcher, text.as_bytes(), sink) + match entry.file_type() { + Some(entry) if entry.is_file() => {} + // skip everything else + _ => return WalkState::Continue, + }; + + let mut stop = false; + let sink = sinks::UTF8(|line_num, line_content| { + stop = injector + .push(FileResult::new( + entry.path(), + line_num as usize - 1, + line_content.to_string(), + )) + .is_err(); + + Ok(!stop) + }); + let doc = documents.iter().find(|&(doc_path, _)| { + doc_path + .as_ref() + .map_or(false, |doc_path| doc_path == entry.path()) + }); + + let result = if let Some((_, doc)) = doc { + // there is already a buffer for this file + // search the buffer instead of the file because it's faster + // and captures new edits without requiring a save + if searcher.multi_line_with_matcher(&matcher) { + // in this case a continous buffer is required + // convert the rope to a string + let text = doc.to_string(); + searcher.search_slice(&matcher, text.as_bytes(), sink) + } else { + searcher.search_reader( + &matcher, + RopeReader::new(doc.slice(..)), + sink, + ) + } } else { - searcher.search_reader( - &matcher, - RopeReader::new(doc.slice(..)), - sink, - ) + searcher.search_path(&matcher, entry.path(), sink) + }; + + if let Err(err) = result { + log::error!( + "Global search error: {}, {}", + entry.path().display(), + err + ); } - } else { - searcher.search_path(&matcher, entry.path(), sink) - }; - - if let Err(err) = result { - log::error!( - "Global search error: {}, {}", - entry.path().display(), - err - ); - } - if stop { - WalkState::Quit - } else { - WalkState::Continue - } - }) - }); + if stop { + WalkState::Quit + } else { + WalkState::Continue + } + }) + }); }); cx.jobs.callback(async move { @@ -2424,7 +2425,7 @@ fn global_search(cx: &mut Context) { picker, 0, injector, - move |cx, FileResult { path, line_num }, action| { + move |cx, FileResult { path, line_num, .. }, action| { let doc = match cx.editor.open(path, action) { Ok(id) => doc_mut!(cx.editor, &id), Err(e) => { @@ -2456,7 +2457,7 @@ fn global_search(cx: &mut Context) { }, ) .with_preview( - |_editor, FileResult { path, line_num }| { + |_editor, FileResult { path, line_num, .. }| { Some((path.clone().into(), Some((*line_num, *line_num)))) }, ); @@ -2897,31 +2898,6 @@ fn buffer_picker(cx: &mut Context) { focused_at: std::time::Instant, } - impl ui::menu::Item for BufferMeta { - type Data = (); - - fn format(&self, _data: &Self::Data) -> Row { - let path = self - .path - .as_deref() - .map(helix_stdx::path::get_relative_path); - let path = match path.as_deref().and_then(Path::to_str) { - Some(path) => path, - None => SCRATCH_BUFFER_NAME, - }; - - let mut flags = String::new(); - if self.is_modified { - flags.push('+'); - } - if self.is_current { - flags.push('*'); - } - - Row::new([self.id.to_string(), flags, path.to_string()]) - } - } - let new_meta = |doc: &Document| BufferMeta { id: doc.id(), path: doc.path().cloned(), @@ -2940,8 +2916,31 @@ fn buffer_picker(cx: &mut Context) { // mru items.sort_unstable_by_key(|item| std::cmp::Reverse(item.focused_at)); - let columns = vec![]; - let picker = Picker::new(columns, 0, items, (), |cx, meta, action| { + let columns = vec![ + PickerColumn::new("id", |meta: &BufferMeta, _| meta.id.to_string().into()), + PickerColumn::new("flags", |meta: &BufferMeta, _| { + let mut flags = String::new(); + if meta.is_modified { + flags.push('+'); + } + if meta.is_current { + flags.push('*'); + } + flags.into() + }), + PickerColumn::new("path", |meta: &BufferMeta, _| { + let path = meta + .path + .as_deref() + .map(helix_stdx::path::get_relative_path); + path.as_deref() + .and_then(Path::to_str) + .unwrap_or(SCRATCH_BUFFER_NAME) + .to_string() + .into() + }), + ]; + let picker = Picker::new(columns, 2, items, (), |cx, meta, action| { cx.editor.switch(meta.id, action); }) .with_preview(|editor, meta| { @@ -2965,33 +2964,6 @@ fn jumplist_picker(cx: &mut Context) { is_current: bool, } - impl ui::menu::Item for JumpMeta { - type Data = (); - - fn format(&self, _data: &Self::Data) -> Row { - let path = self - .path - .as_deref() - .map(helix_stdx::path::get_relative_path); - let path = match path.as_deref().and_then(Path::to_str) { - Some(path) => path, - None => SCRATCH_BUFFER_NAME, - }; - - let mut flags = Vec::new(); - if self.is_current { - flags.push("*"); - } - - let flag = if flags.is_empty() { - "".into() - } else { - format!(" ({})", flags.join("")) - }; - format!("{} {}{} {}", self.id, path, flag, self.text).into() - } - } - for (view, _) in cx.editor.tree.views_mut() { for doc_id in view.jumps.iter().map(|e| e.0).collect::>().iter() { let doc = doc_mut!(cx.editor, doc_id); @@ -3018,10 +2990,37 @@ fn jumplist_picker(cx: &mut Context) { } }; - let columns = vec![]; + let columns = vec![ + ui::PickerColumn::new("id", |item: &JumpMeta, _| item.id.to_string().into()), + ui::PickerColumn::new("path", |item: &JumpMeta, _| { + let path = item + .path + .as_deref() + .map(helix_stdx::path::get_relative_path); + path.as_deref() + .and_then(Path::to_str) + .unwrap_or(SCRATCH_BUFFER_NAME) + .to_string() + .into() + }), + ui::PickerColumn::new("flags", |item: &JumpMeta, _| { + let mut flags = Vec::new(); + if item.is_current { + flags.push("*"); + } + + if flags.is_empty() { + "".into() + } else { + format!(" ({})", flags.join("")).into() + } + }), + ui::PickerColumn::new("contents", |item: &JumpMeta, _| item.text.as_str().into()), + ]; + let picker = Picker::new( columns, - 0, + 1, // path cx.editor .tree .views() @@ -3061,33 +3060,6 @@ fn changed_file_picker(cx: &mut Context) { style_renamed: Style, } - impl Item for FileChange { - type Data = FileChangeData; - - fn format(&self, data: &Self::Data) -> Row { - let process_path = |path: &PathBuf| { - path.strip_prefix(&data.cwd) - .unwrap_or(path) - .display() - .to_string() - }; - - let (sign, style, content) = match self { - Self::Untracked { path } => ("[+]", data.style_untracked, process_path(path)), - Self::Modified { path } => ("[~]", data.style_modified, process_path(path)), - Self::Conflict { path } => ("[x]", data.style_conflict, process_path(path)), - Self::Deleted { path } => ("[-]", data.style_deleted, process_path(path)), - Self::Renamed { from_path, to_path } => ( - "[>]", - data.style_renamed, - format!("{} -> {}", process_path(from_path), process_path(to_path)), - ), - }; - - Row::new([Cell::from(Span::styled(sign, style)), Cell::from(content)]) - } - } - let cwd = helix_stdx::env::current_working_dir(); if !cwd.exists() { cx.editor @@ -3101,7 +3073,40 @@ fn changed_file_picker(cx: &mut Context) { let deleted = cx.editor.theme.get("diff.minus"); let renamed = cx.editor.theme.get("diff.delta.moved"); + let columns = vec![ + PickerColumn::new("change", |change: &FileChange, data: &FileChangeData| { + match change { + FileChange::Untracked { .. } => Span::styled("+ untracked", data.style_untracked), + FileChange::Modified { .. } => Span::styled("~ modified", data.style_modified), + FileChange::Conflict { .. } => Span::styled("x conflict", data.style_conflict), + FileChange::Deleted { .. } => Span::styled("- deleted", data.style_deleted), + FileChange::Renamed { .. } => Span::styled("> renamed", data.style_renamed), + } + .into() + }), + PickerColumn::new("path", |change: &FileChange, data: &FileChangeData| { + let display_path = |path: &PathBuf| { + path.strip_prefix(&data.cwd) + .unwrap_or(path) + .display() + .to_string() + }; + match change { + FileChange::Untracked { path } => display_path(path), + FileChange::Modified { path } => display_path(path), + FileChange::Conflict { path } => display_path(path), + FileChange::Deleted { path } => display_path(path), + FileChange::Renamed { from_path, to_path } => { + format!("{} -> {}", display_path(from_path), display_path(to_path)) + } + } + .into() + }), + ]; + let picker = Picker::new( + columns, + 1, // path Vec::new(), FileChangeData { cwd: cwd.clone(), @@ -3139,35 +3144,6 @@ fn changed_file_picker(cx: &mut Context) { cx.push_layer(Box::new(overlaid(picker))); } -impl ui::menu::Item for MappableCommand { - type Data = ReverseKeymap; - - fn format(&self, keymap: &Self::Data) -> Row { - let fmt_binding = |bindings: &Vec>| -> String { - bindings.iter().fold(String::new(), |mut acc, bind| { - if !acc.is_empty() { - acc.push(' '); - } - for key in bind { - acc.push_str(&key.key_sequence_format()); - } - acc - }) - }; - - match self { - MappableCommand::Typable { doc, name, .. } => match keymap.get(name as &String) { - Some(bindings) => format!("{} ({}) [:{}]", doc, fmt_binding(bindings), name).into(), - None => format!("{} [:{}]", doc, name).into(), - }, - MappableCommand::Static { doc, name, .. } => match keymap.get(*name) { - Some(bindings) => format!("{} ({}) [{}]", doc, fmt_binding(bindings), name).into(), - None => format!("{} [{}]", doc, name).into(), - }, - } - } -} - pub fn command_palette(cx: &mut Context) { let register = cx.register; let count = cx.count; @@ -3187,7 +3163,34 @@ pub fn command_palette(cx: &mut Context) { } })); - let columns = vec![]; + let columns = vec![ + ui::PickerColumn::new("name", |item, _| match item { + MappableCommand::Typable { name, .. } => format!(":{name}").into(), + MappableCommand::Static { name, .. } => (*name).into(), + }), + ui::PickerColumn::new( + "bindings", + |item: &MappableCommand, keymap: &crate::keymap::ReverseKeymap| { + keymap + .get(item.name()) + .map(|bindings| { + bindings.iter().fold(String::new(), |mut acc, bind| { + if !acc.is_empty() { + acc.push(' '); + } + for key in bind { + acc.push_str(&key.key_sequence_format()); + } + acc + }) + }) + .unwrap_or_default() + .into() + }, + ), + ui::PickerColumn::new("doc", |item: &MappableCommand, _| item.doc().into()), + ]; + let picker = Picker::new(columns, 0, commands, keymap, move |cx, command, _action| { let mut ctx = Context { register, diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index da2b60dae4b3..14cb83d290e9 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -12,7 +12,7 @@ use helix_view::editor::Breakpoint; use serde_json::{to_value, Value}; use tokio_stream::wrappers::UnboundedReceiverStream; -use tui::{text::Spans, widgets::Row}; +use tui::text::Spans; use std::collections::HashMap; use std::future::Future; @@ -22,38 +22,6 @@ 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 format(&self, _data: &Self::Data) -> Row { - self.name.as_str().into() // TODO: include thread_states in the label - } -} - -impl ui::menu::Item for DebugTemplate { - type Data = (); - - fn format(&self, _data: &Self::Data) -> Row { - self.name.as_str().into() - } -} - -impl ui::menu::Item for Thread { - type Data = ThreadStates; - - fn format(&self, thread_states: &Self::Data) -> Row { - 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, @@ -73,7 +41,16 @@ fn thread_picker( let debugger = debugger!(editor); let thread_states = debugger.thread_states.clone(); - let columns = vec![]; + let columns = vec![ + ui::PickerColumn::new("name", |item: &Thread, _| item.name.as_str().into()), + ui::PickerColumn::new("state", |item: &Thread, thread_states: &ThreadStates| { + thread_states + .get(&item.id) + .map(|state| state.as_str()) + .unwrap_or("unknown") + .into() + }), + ]; let picker = Picker::new( columns, 0, @@ -273,7 +250,10 @@ pub fn dap_launch(cx: &mut Context) { let templates = config.templates.clone(); - let columns = vec![]; + let columns = vec![ui::PickerColumn::new( + "template", + |item: &DebugTemplate, _| item.name.as_str().into(), + )]; cx.push_layer(Box::new(overlaid(Picker::new( columns, @@ -745,7 +725,9 @@ pub fn dap_switch_stack_frame(cx: &mut Context) { let frames = debugger.stack_frames[&thread_id].clone(); - let columns = vec![]; + let columns = vec![ui::PickerColumn::new("frame", |item: &StackFrame, _| { + item.name.as_str().into() // TODO: include thread_states in the label + })]; let picker = Picker::new(columns, 0, frames, (), 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 bf9747a4cc9b..2292569bf89d 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -9,10 +9,7 @@ use helix_lsp::{ Client, LanguageServerId, OffsetEncoding, }; use tokio_stream::StreamExt; -use tui::{ - text::{Span, Spans}, - widgets::Row, -}; +use tui::{text::Span, widgets::Row}; use super::{align_view, push_jump, Align, Context, Editor}; @@ -62,69 +59,11 @@ macro_rules! language_server_with_feature { }}; } -impl ui::menu::Item for lsp::Location { - /// Current working directory. - type Data = PathBuf; - - fn format(&self, cwdir: &Self::Data) -> Row { - // The preallocation here will overallocate a few characters since it will account for the - // URL's scheme, which is not used most of the time since that scheme will be "file://". - // Those extra chars will be used to avoid allocating when writing the line number (in the - // common case where it has 5 digits or less, which should be enough for a cast majority - // of usages). - let mut res = String::with_capacity(self.uri.as_str().len()); - - if self.uri.scheme() == "file" { - // With the preallocation above and UTF-8 paths already, this closure will do one (1) - // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`. - let mut write_path_to_res = || -> Option<()> { - let path = self.uri.to_file_path().ok()?; - res.push_str(&path.strip_prefix(cwdir).unwrap_or(&path).to_string_lossy()); - Some(()) - }; - write_path_to_res(); - } else { - // Never allocates since we declared the string with this capacity already. - res.push_str(self.uri.as_str()); - } - - // Most commonly, this will not allocate, especially on Unix systems where the root prefix - // is a simple `/` and not `C:\` (with whatever drive letter) - write!(&mut res, ":{}", self.range.start.line + 1) - .expect("Will only failed if allocating fail"); - res.into() - } -} - struct SymbolInformationItem { symbol: lsp::SymbolInformation, offset_encoding: OffsetEncoding, } -impl ui::menu::Item for SymbolInformationItem { - /// Path to currently focussed document - type Data = Option; - - fn format(&self, current_doc_path: &Self::Data) -> Row { - if current_doc_path.as_ref() == Some(&self.symbol.location.uri) { - self.symbol.name.as_str().into() - } else { - match self.symbol.location.uri.to_file_path() { - Ok(path) => { - let get_relative_path = path::get_relative_path(path.as_path()); - format!( - "{} ({})", - &self.symbol.name, - get_relative_path.to_string_lossy() - ) - .into() - } - Err(_) => format!("{} ({})", &self.symbol.name, &self.symbol.location.uri).into(), - } - } - } -} - struct DiagnosticStyles { hint: Style, info: Style, @@ -138,48 +77,6 @@ struct PickerDiagnostic { offset_encoding: OffsetEncoding, } -impl ui::menu::Item for PickerDiagnostic { - type Data = (DiagnosticStyles, DiagnosticsFormat); - - fn format(&self, (styles, format): &Self::Data) -> Row { - 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 = match self.diag.code.as_ref() { - Some(NumberOrString::Number(n)) => format!(" ({n})"), - Some(NumberOrString::String(s)) => format!(" ({s})"), - None => String::new(), - }; - - let path = match format { - DiagnosticsFormat::HideSourcePath => String::new(), - DiagnosticsFormat::ShowSourcePath => { - let path = path::get_truncated_path(&self.path); - format!("{}: ", path.to_string_lossy()) - } - }; - - Spans::from(vec![ - Span::raw(path), - Span::styled(&self.diag.message, style), - Span::styled(code, style), - ]) - .into() - } -} - fn location_to_file_location(location: &lsp::Location) -> FileLocation { let path = location.uri.to_file_path().unwrap(); let line = Some(( @@ -241,16 +138,82 @@ fn jump_to_position( } } -type SymbolPicker = Picker>; +fn display_symbol_kind(kind: lsp::SymbolKind) -> &'static str { + match kind { + lsp::SymbolKind::FILE => "file", + lsp::SymbolKind::MODULE => "module", + lsp::SymbolKind::NAMESPACE => "namespace", + lsp::SymbolKind::PACKAGE => "package", + lsp::SymbolKind::CLASS => "class", + lsp::SymbolKind::METHOD => "method", + lsp::SymbolKind::PROPERTY => "property", + lsp::SymbolKind::FIELD => "field", + lsp::SymbolKind::CONSTRUCTOR => "construct", + lsp::SymbolKind::ENUM => "enum", + lsp::SymbolKind::INTERFACE => "interface", + lsp::SymbolKind::FUNCTION => "function", + lsp::SymbolKind::VARIABLE => "variable", + lsp::SymbolKind::CONSTANT => "constant", + lsp::SymbolKind::STRING => "string", + lsp::SymbolKind::NUMBER => "number", + lsp::SymbolKind::BOOLEAN => "boolean", + lsp::SymbolKind::ARRAY => "array", + lsp::SymbolKind::OBJECT => "object", + lsp::SymbolKind::KEY => "key", + lsp::SymbolKind::NULL => "null", + lsp::SymbolKind::ENUM_MEMBER => "enummem", + lsp::SymbolKind::STRUCT => "struct", + lsp::SymbolKind::EVENT => "event", + lsp::SymbolKind::OPERATOR => "operator", + lsp::SymbolKind::TYPE_PARAMETER => "typeparam", + _ => { + log::warn!("Unknown symbol kind: {:?}", kind); + "" + } + } +} + +type SymbolPicker = Picker; + +fn sym_picker(symbols: Vec, workspace: bool) -> SymbolPicker { + let symbol_kind_column = ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { + display_symbol_kind(item.symbol.kind).into() + }); + + let columns = if workspace { + vec![ + symbol_kind_column, + 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(), + } + }), + ] + } else { + vec![ + symbol_kind_column, + // Some symbols in the document symbol picker may have a URI that isn't + // the current file. It should be rare though, so we concatenate that + // URI in with the symbol name in this picker. + ui::PickerColumn::new("name", |item: &SymbolInformationItem, _| { + item.symbol.name.as_str().into() + }), + ] + }; -fn sym_picker(symbols: Vec, current_path: Option) -> SymbolPicker { - // TODO: drop current_path comparison and instead use workspace: bool flag? - let columns = vec![]; Picker::new( columns, - 0, + 1, // name column symbols, - current_path, + (), move |cx, item, action| { jump_to_location( cx.editor, @@ -270,7 +233,7 @@ enum DiagnosticsFormat { HideSourcePath, } -type DiagnosticsPicker = Picker; +type DiagnosticsPicker = Picker; fn diag_picker( cx: &Context, @@ -302,12 +265,50 @@ fn diag_picker( error: cx.editor.theme.get("error"), }; - let columns = vec![]; + let mut columns = vec![ + ui::PickerColumn::new( + "severity", + |item: &PickerDiagnostic, styles: &DiagnosticStyles| { + match item.diag.severity { + Some(DiagnosticSeverity::HINT) => Span::styled("HINT", styles.hint), + Some(DiagnosticSeverity::INFORMATION) => Span::styled("INFO", styles.info), + Some(DiagnosticSeverity::WARNING) => Span::styled("WARN", styles.warning), + Some(DiagnosticSeverity::ERROR) => Span::styled("ERROR", styles.error), + _ => Span::raw(""), + } + .into() + }, + ), + ui::PickerColumn::new("code", |item: &PickerDiagnostic, _| { + match item.diag.code.as_ref() { + Some(NumberOrString::Number(n)) => n.to_string().into(), + Some(NumberOrString::String(s)) => s.as_str().into(), + None => "".into(), + } + }), + ui::PickerColumn::new("message", |item: &PickerDiagnostic, _| { + item.diag.message.as_str().into() + }), + ]; + let mut primary_column = 2; // message + + if format == DiagnosticsFormat::ShowSourcePath { + columns.insert( + // between message code and message + 2, + ui::PickerColumn::new("path", |item: &PickerDiagnostic, _| { + let path = path::get_truncated_path(&item.path); + path.to_string_lossy().to_string().into() + }), + ); + primary_column += 1; + } + Picker::new( columns, - 0, + primary_column, flat_diag, - (styles, format), + styles, move |cx, PickerDiagnostic { path, @@ -389,7 +390,6 @@ pub fn symbol_picker(cx: &mut Context) { } }) .collect(); - let current_url = doc.url(); if futures.is_empty() { cx.editor @@ -404,7 +404,7 @@ pub fn symbol_picker(cx: &mut Context) { symbols.append(&mut lsp_items); } let call = move |_editor: &mut Editor, compositor: &mut Compositor| { - let picker = sym_picker(symbols, current_url); + let picker = sym_picker(symbols, false); compositor.push(Box::new(overlaid(picker))) }; @@ -466,13 +466,12 @@ pub fn workspace_symbol_picker(cx: &mut Context) { .boxed() }; - let current_url = doc.url(); 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, current_url); + let picker = sym_picker(symbols, true); let dyn_picker = DynamicPicker::new(picker, Box::new(get_symbols)); compositor.push(Box::new(overlaid(dyn_picker))) }; @@ -753,13 +752,6 @@ pub fn code_action(cx: &mut Context) { }); } -impl ui::menu::Item for lsp::Command { - type Data = (); - fn format(&self, _data: &Self::Data) -> Row { - self.title.as_str().into() - } -} - pub fn execute_lsp_command( editor: &mut Editor, language_server_id: LanguageServerId, @@ -829,7 +821,37 @@ fn goto_impl( } [] => unreachable!("`locations` should be non-empty for `goto_impl`"), _locations => { - let columns = vec![]; + let columns = vec![ui::PickerColumn::new( + "location", + |item: &lsp::Location, cwdir: &std::path::PathBuf| { + // The preallocation here will overallocate a few characters since it will account for the + // URL's scheme, which is not used most of the time since that scheme will be "file://". + // Those extra chars will be used to avoid allocating when writing the line number (in the + // common case where it has 5 digits or less, which should be enough for a cast majority + // of usages). + let mut res = String::with_capacity(item.uri.as_str().len()); + + if item.uri.scheme() == "file" { + // With the preallocation above and UTF-8 paths already, this closure will do one (1) + // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`. + if let Ok(path) = item.uri.to_file_path() { + res.push_str( + &path.strip_prefix(cwdir).unwrap_or(&path).to_string_lossy(), + ); + } + } else { + // Never allocates since we declared the string with this capacity already. + res.push_str(item.uri.as_str()); + } + + // Most commonly, this will not allocate, especially on Unix systems where the root prefix + // is a simple `/` and not `C:\` (with whatever drive letter) + write!(&mut res, ":{}", item.range.start.line + 1) + .expect("Will only failed if allocating fail"); + res.into() + }, + )]; + let picker = Picker::new(columns, 0, locations, cwdir, move |cx, location, action| { jump_to_location(cx.editor, location, offset_encoding, action) }) diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index 232e5846b830..f65f96574a69 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1404,7 +1404,12 @@ fn lsp_workspace_command( let callback = async move { let call: job::Callback = Callback::EditorCompositor(Box::new( move |_editor: &mut Editor, compositor: &mut Compositor| { - let columns = vec![]; + let columns = vec![ui::PickerColumn::new( + "title", + |(_ls_id, command): &(_, helix_lsp::lsp::Command), _| { + command.title.as_str().into() + }, + )]; let picker = ui::Picker::new( columns, 0, diff --git a/helix-term/src/ui/menu.rs b/helix-term/src/ui/menu.rs index c5006f9580af..1520924c08d9 100644 --- a/helix-term/src/ui/menu.rs +++ b/helix-term/src/ui/menu.rs @@ -1,4 +1,4 @@ -use std::{borrow::Cow, cmp::Reverse, path::PathBuf}; +use std::{borrow::Cow, cmp::Reverse}; use crate::{ compositor::{Callback, Component, Compositor, Context, Event, EventResult}, @@ -31,18 +31,6 @@ pub trait Item: Sync + Send + 'static { } } -impl Item for PathBuf { - /// Root prefix to strip. - type Data = PathBuf; - - fn format(&self, root_path: &Self::Data) -> Row { - self.strip_prefix(root_path) - .unwrap_or(self) - .to_string_lossy() - .into() - } -} - pub type MenuCallback = Box, MenuEvent)>; pub struct Menu { diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 01b718d45da9..4f6b031d4535 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -219,7 +219,15 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi }); log::debug!("file_picker init {:?}", Instant::now().duration_since(now)); - let columns = vec![]; + let columns = vec![PickerColumn::new( + "path", + |item: &PathBuf, root: &PathBuf| { + item.strip_prefix(root) + .unwrap_or(item) + .to_string_lossy() + .into() + }, + )]; let picker = Picker::new( columns, 0, From 53ac833efbcd6b968fd7c88318525a8d491fea14 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 11:26:00 -0500 Subject: [PATCH 05/20] Replace picker shutdown bool with version number This works nicely for dynamic pickers: we stop any running jobs like global search that are pushing to the injector by incrementing the version number when we start a new request. The boolean only allowed us to shut the picker down once, but with a usize a picker can have multiple "sessions" / "life-cycles" where it receives new options from an injector. --- helix-term/src/ui/picker.rs | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 5e7bda21bcf4..4f94c6172d8d 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -34,7 +34,7 @@ use std::{ io::Read, path::{Path, PathBuf}, sync::{ - atomic::{self, AtomicBool}, + atomic::{self, AtomicUsize}, Arc, }, }; @@ -154,7 +154,8 @@ pub struct Injector { dst: nucleo::Injector, columns: Arc<[Column]>, editor_data: Arc, - shutown: Arc, + version: usize, + picker_version: Arc, } impl Clone for Injector { @@ -163,7 +164,8 @@ impl Clone for Injector { dst: self.dst.clone(), columns: self.columns.clone(), editor_data: self.editor_data.clone(), - shutown: self.shutown.clone(), + version: self.version, + picker_version: self.picker_version.clone(), } } } @@ -172,7 +174,7 @@ pub struct InjectorShutdown; impl Injector { pub fn push(&self, item: T) -> Result<(), InjectorShutdown> { - if self.shutown.load(atomic::Ordering::Relaxed) { + if self.version != self.picker_version.load(atomic::Ordering::Relaxed) { return Err(InjectorShutdown); } @@ -220,7 +222,7 @@ pub struct Picker { columns: Arc<[Column]>, primary_column: usize, editor_data: Arc, - shutdown: Arc, + version: Arc, matcher: Nucleo, /// Current height of the completions box @@ -261,7 +263,8 @@ impl Picker { dst: matcher.injector(), columns: columns.into(), editor_data: Arc::new(editor_data), - shutown: Arc::new(AtomicBool::new(false)), + version: 0, + picker_version: Arc::new(AtomicUsize::new(0)), }; (matcher, streamer) } @@ -290,7 +293,7 @@ impl Picker { columns.into(), primary_column, Arc::new(editor_data), - Arc::new(AtomicBool::new(false)), + Arc::new(AtomicUsize::new(0)), callback_fn, ) } @@ -306,7 +309,7 @@ impl Picker { injector.columns, primary_column, injector.editor_data, - injector.shutown, + injector.picker_version, callback_fn, ) } @@ -316,7 +319,7 @@ impl Picker { columns: Arc<[Column]>, default_column: usize, editor_data: Arc, - shutdown: Arc, + version: Arc, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { assert!(!columns.is_empty()); @@ -340,7 +343,7 @@ impl Picker { primary_column: default_column, matcher, editor_data, - shutdown, + version, cursor: 0, prompt, query, @@ -361,7 +364,8 @@ impl Picker { dst: self.matcher.injector(), columns: self.columns.clone(), editor_data: self.editor_data.clone(), - shutown: self.shutdown.clone(), + version: self.version.load(atomic::Ordering::Relaxed), + picker_version: self.version.clone(), } } @@ -916,7 +920,7 @@ impl Component for Picker Component for Picker Drop for Picker { fn drop(&mut self) { // ensure we cancel any ongoing background threads streaming into the picker - self.shutdown.store(true, atomic::Ordering::Relaxed) + self.version.fetch_add(1, atomic::Ordering::Relaxed); } } From 2c9f5b3efb72f4ab62fe29ddaa1c0871bca9b01d Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 12:08:59 -0500 Subject: [PATCH 06/20] Implement Error for InjectorShutdown --- Cargo.lock | 1 + helix-term/Cargo.toml | 1 + helix-term/src/ui/picker.rs | 3 +++ 3 files changed, 5 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index c614cadf13a8..57bdb235d5cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1467,6 +1467,7 @@ dependencies = [ "smallvec", "tempfile", "termini", + "thiserror", "tokio", "tokio-stream", "toml", diff --git a/helix-term/Cargo.toml b/helix-term/Cargo.toml index fb850f7c06ea..c50165e9aacd 100644 --- a/helix-term/Cargo.toml +++ b/helix-term/Cargo.toml @@ -56,6 +56,7 @@ ignore = "0.4" pulldown-cmark = { version = "0.11", default-features = false } # file type detection content_inspector = "0.2.4" +thiserror = "1.0" # opening URLs open = "5.2.0" diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 4f94c6172d8d..7e053c4add6d 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -18,6 +18,7 @@ use futures_util::future::BoxFuture; use helix_event::AsyncHook; use nucleo::pattern::CaseMatching; use nucleo::{Config, Nucleo, Utf32String}; +use thiserror::Error; use tokio::sync::mpsc::Sender; use tui::{ buffer::Buffer as Surface, @@ -170,6 +171,8 @@ impl Clone for Injector { } } +#[derive(Error, Debug)] +#[error("picker has been shut down")] pub struct InjectorShutdown; impl Injector { From 11f809c1779db9a02315b58466cc4e1bdf494202 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 21 Feb 2024 09:37:45 -0500 Subject: [PATCH 07/20] Bump nucleo to v0.4.1 We will use this in the child commit to improve the picker's running indicator. Nucleo 0.4.0 includes an `active_injectors` member that we can use to detect if anything can push to the picker. When that count drops to zero we can remove the running indicator. Nucleo 0.4.1 contains a fix for crashes with interactive global search on a large directory. --- Cargo.lock | 15 ++++----------- Cargo.toml | 2 +- helix-core/src/fuzzy.rs | 10 ++++++++-- helix-term/src/ui/menu.rs | 10 ++++++++-- helix-term/src/ui/picker.rs | 12 ++++++++---- 5 files changed, 29 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 57bdb235d5cd..d8e65a8f3a3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -209,12 +209,6 @@ version = "0.8.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e496a50fda8aacccc86d7529e2c1e0892dbd0f898a6b5645b5561b89c3210efa" -[[package]] -name = "cov-mark" -version = "1.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ffa3d3e0138386cd4361f63537765cac7ee40698028844635a54495a92f67f3" - [[package]] name = "crc32fast" version = "1.3.2" @@ -1785,9 +1779,9 @@ dependencies = [ [[package]] name = "nucleo" -version = "0.2.1" +version = "0.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae5331f4bcce475cf28cb29c95366c3091af4b0aa7703f1a6bc858f29718fdf3" +checksum = "6350a138d8860658523a7593cbf6813999d17a099371d14f70c5c905b37593e9" dependencies = [ "nucleo-matcher", "parking_lot", @@ -1796,11 +1790,10 @@ dependencies = [ [[package]] name = "nucleo-matcher" -version = "0.2.0" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b702b402fe286162d1f00b552a046ce74365d2ac473a2607ff36ba650f9bd57" +checksum = "bf33f538733d1a5a3494b836ba913207f14d9d4a1d3cd67030c5061bdd2cac85" dependencies = [ - "cov-mark", "memchr", "unicode-segmentation", ] diff --git a/Cargo.toml b/Cargo.toml index 9be265fc5864..d9541b648c86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ package.helix-term.opt-level = 2 [workspace.dependencies] tree-sitter = { version = "0.22" } -nucleo = "0.2.0" +nucleo = "0.4.1" slotmap = "1.0.7" thiserror = "1.0" diff --git a/helix-core/src/fuzzy.rs b/helix-core/src/fuzzy.rs index 549c6b0e54b6..da46518f961d 100644 --- a/helix-core/src/fuzzy.rs +++ b/helix-core/src/fuzzy.rs @@ -1,6 +1,6 @@ use std::ops::DerefMut; -use nucleo::pattern::{Atom, AtomKind, CaseMatching}; +use nucleo::pattern::{Atom, AtomKind, CaseMatching, Normalization}; use nucleo::Config; use parking_lot::Mutex; @@ -38,6 +38,12 @@ pub fn fuzzy_match>( if path { matcher.config.set_match_paths(); } - let pattern = Atom::new(pattern, CaseMatching::Smart, AtomKind::Fuzzy, false); + let pattern = Atom::new( + pattern, + CaseMatching::Smart, + Normalization::Smart, + AtomKind::Fuzzy, + false, + ); pattern.match_list(items, &mut matcher) } diff --git a/helix-term/src/ui/menu.rs b/helix-term/src/ui/menu.rs index 1520924c08d9..c120d0b25cf3 100644 --- a/helix-term/src/ui/menu.rs +++ b/helix-term/src/ui/menu.rs @@ -5,7 +5,7 @@ use crate::{ ctrl, key, shift, }; use helix_core::fuzzy::MATCHER; -use nucleo::pattern::{Atom, AtomKind, CaseMatching}; +use nucleo::pattern::{Atom, AtomKind, CaseMatching, Normalization}; use nucleo::{Config, Utf32Str}; use tui::{buffer::Buffer as Surface, widgets::Table}; @@ -80,7 +80,13 @@ impl Menu { pub fn score(&mut self, pattern: &str, incremental: bool) { let mut matcher = MATCHER.lock(); matcher.config = Config::DEFAULT; - let pattern = Atom::new(pattern, CaseMatching::Ignore, AtomKind::Fuzzy, false); + let pattern = Atom::new( + pattern, + CaseMatching::Ignore, + Normalization::Smart, + AtomKind::Fuzzy, + false, + ); let mut buf = Vec::new(); if incremental { self.matches.retain_mut(|(index, score)| { diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 7e053c4add6d..fc1eba6e7c84 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -16,7 +16,7 @@ use crate::{ }; use futures_util::future::BoxFuture; use helix_event::AsyncHook; -use nucleo::pattern::CaseMatching; +use nucleo::pattern::{CaseMatching, Normalization}; use nucleo::{Config, Nucleo, Utf32String}; use thiserror::Error; use tokio::sync::mpsc::Sender; @@ -506,9 +506,13 @@ impl Picker { continue; } let is_append = pattern.starts_with(old_pattern); - self.matcher - .pattern - .reparse(i, pattern, CaseMatching::Smart, is_append); + self.matcher.pattern.reparse( + i, + pattern, + CaseMatching::Smart, + Normalization::Smart, + is_append, + ); } } From 9e31ba547574ca77b1cf63a62237a51eeea2e222 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 16 Feb 2024 11:19:00 -0500 Subject: [PATCH 08/20] 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 2292569bf89d..3dfe46ad76ad 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 4f6b031d4535..93ac2e6516e8 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 fc1eba6e7c84..bfec9ddb4f08 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 f01c982a7ce0..e426adda7f87 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(); + }); + }) + } +} From 5622db6798f665cdb6c208970763a31902c3e37e Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 1 Apr 2024 11:45:49 -0400 Subject: [PATCH 09/20] Remove sym_picker helper fun The parent commit split out the workspace symbol picker to an inline definition so the `workspace` parameter is never passed as `true`. We should consolidate this picker definition into the symbol_picker function. --- helix-term/src/commands/lsp.rs | 86 ++++++++++++---------------------- 1 file changed, 31 insertions(+), 55 deletions(-) diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 3dfe46ad76ad..23bcb977f728 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -173,60 +173,6 @@ fn display_symbol_kind(kind: lsp::SymbolKind) -> &'static str { } } -type SymbolPicker = Picker; - -fn sym_picker(symbols: Vec, workspace: bool) -> SymbolPicker { - let symbol_kind_column = ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { - display_symbol_kind(item.symbol.kind).into() - }); - - let columns = if workspace { - vec![ - symbol_kind_column, - 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(), - } - }), - ] - } else { - vec![ - symbol_kind_column, - // Some symbols in the document symbol picker may have a URI that isn't - // the current file. It should be rare though, so we concatenate that - // URI in with the symbol name in this picker. - ui::PickerColumn::new("name", |item: &SymbolInformationItem, _| { - item.symbol.name.as_str().into() - }), - ] - }; - - Picker::new( - columns, - 1, // name column - symbols, - (), - move |cx, item, action| { - jump_to_location( - cx.editor, - &item.symbol.location, - item.offset_encoding, - action, - ); - }, - ) - .with_preview(move |_editor, item| Some(location_to_file_location(&item.symbol.location))) - .truncate_start(false) -} - #[derive(Copy, Clone, PartialEq)] enum DiagnosticsFormat { ShowSourcePath, @@ -404,7 +350,37 @@ pub fn symbol_picker(cx: &mut Context) { symbols.append(&mut lsp_items); } let call = move |_editor: &mut Editor, compositor: &mut Compositor| { - let picker = sym_picker(symbols, false); + let columns = vec![ + ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { + display_symbol_kind(item.symbol.kind).into() + }), + // Some symbols in the document symbol picker may have a URI that isn't + // the current file. It should be rare though, so we concatenate that + // URI in with the symbol name in this picker. + ui::PickerColumn::new("name", |item: &SymbolInformationItem, _| { + item.symbol.name.as_str().into() + }), + ]; + + let picker = Picker::new( + columns, + 1, // name column + symbols, + (), + move |cx, item, action| { + jump_to_location( + cx.editor, + &item.symbol.location, + item.offset_encoding, + action, + ); + }, + ) + .with_preview(move |_editor, item| { + Some(location_to_file_location(&item.symbol.location)) + }) + .truncate_start(false); + compositor.push(Box::new(overlaid(picker))) }; From 1d023b05ac7a5d5176ee7cb02042d045bdc3a095 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sat, 17 Feb 2024 10:37:21 -0500 Subject: [PATCH 10/20] Refactor global_search as a dynamic Picker --- helix-term/src/commands.rs | 383 ++++++++++++++++++------------------- 1 file changed, 183 insertions(+), 200 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index f4d2c634ce80..b7ffeeb7751c 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3,6 +3,7 @@ pub(crate) mod lsp; pub(crate) mod typed; pub use dap::*; +use futures_util::FutureExt; use helix_event::status; use helix_stdx::{ path::expand_tilde, @@ -2255,222 +2256,204 @@ fn global_search(cx: &mut Context) { } } + struct GlobalSearchConfig { + smart_case: bool, + file_picker_config: helix_view::editor::FilePickerConfig, + } + let config = cx.editor.config(); - let smart_case = config.search.smart_case; - let file_picker_config = config.file_picker.clone(); + let config = GlobalSearchConfig { + smart_case: config.search.smart_case, + file_picker_config: config.file_picker.clone(), + }; - let reg = cx.register.unwrap_or('/'); - let completions = search_completions(cx, Some(reg)); - ui::raw_regex_prompt( - cx, - "global-search:".into(), - Some(reg), - move |_editor: &Editor, input: &str| { - completions - .iter() - .filter(|comp| comp.starts_with(input)) - .map(|comp| (0.., std::borrow::Cow::Owned(comp.clone()))) - .collect() - }, - move |cx, _, input, event| { - if event != PromptEvent::Validate { - return; - } - cx.editor.registers.last_search_register = reg; + let columns = vec![ + PickerColumn::new("path", |item: &FileResult, _| { + let path = helix_stdx::path::get_relative_path(&item.path); + format!("{}:{}", path.to_string_lossy(), item.line_num + 1).into() + }), + PickerColumn::new("contents", |item: &FileResult, _| { + item.line_content.as_str().into() + }) + .without_filtering(), + ]; - let current_path = doc_mut!(cx.editor).path().cloned(); - let documents: Vec<_> = cx - .editor - .documents() - .map(|doc| (doc.path().cloned(), doc.text().to_owned())) - .collect(); + let get_files = |query: &str, + editor: &mut Editor, + config: std::sync::Arc, + injector: &ui::picker::Injector<_, _>| { + if query.is_empty() { + return async { Ok(()) }.boxed(); + } - if let Ok(matcher) = RegexMatcherBuilder::new() - .case_smart(smart_case) - .build(input) - { - let search_root = helix_stdx::env::current_working_dir(); - if !search_root.exists() { - cx.editor - .set_error("Current working directory does not exist"); - return; - } + let search_root = helix_stdx::env::current_working_dir(); + if !search_root.exists() { + return async { Err(anyhow::anyhow!("Current working directory does not exist")) } + .boxed(); + } + + let documents: Vec<_> = editor + .documents() + .map(|doc| (doc.path().cloned(), doc.text().to_owned())) + .collect(); - let columns = vec![ - PickerColumn::new( - "path", - |item: &FileResult, current_path: &Option| { - let relative_path = helix_stdx::path::get_relative_path(&item.path) - .to_string_lossy() - .into_owned(); - if current_path + let matcher = match RegexMatcherBuilder::new() + .case_smart(config.smart_case) + .build(query) + { + Ok(matcher) => { + // Clear any "Failed to compile regex" errors out of the statusline. + editor.clear_status(); + matcher + } + Err(err) => { + log::info!("Failed to compile search pattern in global search: {}", err); + return async { Err(anyhow::anyhow!("Failed to compile regex")) }.boxed(); + } + }; + + let dedup_symlinks = config.file_picker_config.deduplicate_links; + let absolute_root = search_root + .canonicalize() + .unwrap_or_else(|_| search_root.clone()); + + let injector = injector.clone(); + async move { + let searcher = SearcherBuilder::new() + .binary_detection(BinaryDetection::quit(b'\x00')) + .build(); + WalkBuilder::new(search_root) + .hidden(config.file_picker_config.hidden) + .parents(config.file_picker_config.parents) + .ignore(config.file_picker_config.ignore) + .follow_links(config.file_picker_config.follow_symlinks) + .git_ignore(config.file_picker_config.git_ignore) + .git_global(config.file_picker_config.git_global) + .git_exclude(config.file_picker_config.git_exclude) + .max_depth(config.file_picker_config.max_depth) + .filter_entry(move |entry| { + filter_picker_entry(entry, &absolute_root, dedup_symlinks) + }) + .add_custom_ignore_filename(helix_loader::config_dir().join("ignore")) + .add_custom_ignore_filename(".helix/ignore") + .build_parallel() + .run(|| { + let mut searcher = searcher.clone(); + let matcher = matcher.clone(); + let injector = injector.clone(); + let documents = &documents; + Box::new(move |entry: Result| -> WalkState { + let entry = match entry { + Ok(entry) => entry, + Err(_) => return WalkState::Continue, + }; + + match entry.file_type() { + Some(entry) if entry.is_file() => {} + // skip everything else + _ => return WalkState::Continue, + }; + + let mut stop = false; + let sink = sinks::UTF8(|line_num, line_content| { + stop = injector + .push(FileResult::new( + entry.path(), + line_num as usize - 1, + line_content.to_string(), + )) + .is_err(); + + Ok(!stop) + }); + let doc = documents.iter().find(|&(doc_path, _)| { + doc_path .as_ref() - .map(|p| p == &item.path) - .unwrap_or(false) - { - format!("{} (*)", relative_path).into() + .map_or(false, |doc_path| doc_path == entry.path()) + }); + + let result = if let Some((_, doc)) = doc { + // there is already a buffer for this file + // search the buffer instead of the file because it's faster + // and captures new edits without requiring a save + if searcher.multi_line_with_matcher(&matcher) { + // in this case a continous buffer is required + // convert the rope to a string + let text = doc.to_string(); + searcher.search_slice(&matcher, text.as_bytes(), sink) } else { - relative_path.into() + searcher.search_reader( + &matcher, + RopeReader::new(doc.slice(..)), + sink, + ) } - }, - ), - PickerColumn::new("contents", |item: &FileResult, _| { - item.line_content.as_str().into() - }), - ]; - let (picker, injector) = Picker::stream(columns, current_path); - - let dedup_symlinks = file_picker_config.deduplicate_links; - let absolute_root = search_root - .canonicalize() - .unwrap_or_else(|_| search_root.clone()); - let injector_ = injector.clone(); - - std::thread::spawn(move || { - let searcher = SearcherBuilder::new() - .binary_detection(BinaryDetection::quit(b'\x00')) - .build(); - - let mut walk_builder = WalkBuilder::new(search_root); - - walk_builder - .hidden(file_picker_config.hidden) - .parents(file_picker_config.parents) - .ignore(file_picker_config.ignore) - .follow_links(file_picker_config.follow_symlinks) - .git_ignore(file_picker_config.git_ignore) - .git_global(file_picker_config.git_global) - .git_exclude(file_picker_config.git_exclude) - .max_depth(file_picker_config.max_depth) - .filter_entry(move |entry| { - filter_picker_entry(entry, &absolute_root, dedup_symlinks) - }) - .add_custom_ignore_filename(helix_loader::config_dir().join("ignore")) - .add_custom_ignore_filename(".helix/ignore") - .build_parallel() - .run(|| { - let mut searcher = searcher.clone(); - let matcher = matcher.clone(); - let injector = injector_.clone(); - let documents = &documents; - Box::new(move |entry: Result| -> WalkState { - let entry = match entry { - Ok(entry) => entry, - Err(_) => return WalkState::Continue, - }; - - match entry.file_type() { - Some(entry) if entry.is_file() => {} - // skip everything else - _ => return WalkState::Continue, - }; - - let mut stop = false; - let sink = sinks::UTF8(|line_num, line_content| { - stop = injector - .push(FileResult::new( - entry.path(), - line_num as usize - 1, - line_content.to_string(), - )) - .is_err(); - - Ok(!stop) - }); - let doc = documents.iter().find(|&(doc_path, _)| { - doc_path - .as_ref() - .map_or(false, |doc_path| doc_path == entry.path()) - }); - - let result = if let Some((_, doc)) = doc { - // there is already a buffer for this file - // search the buffer instead of the file because it's faster - // and captures new edits without requiring a save - if searcher.multi_line_with_matcher(&matcher) { - // in this case a continous buffer is required - // convert the rope to a string - let text = doc.to_string(); - searcher.search_slice(&matcher, text.as_bytes(), sink) - } else { - searcher.search_reader( - &matcher, - RopeReader::new(doc.slice(..)), - sink, - ) - } - } else { - searcher.search_path(&matcher, entry.path(), sink) - }; - - if let Err(err) = result { - log::error!( - "Global search error: {}, {}", - entry.path().display(), - err - ); - } - if stop { - WalkState::Quit - } else { - WalkState::Continue - } - }) - }); + } else { + searcher.search_path(&matcher, entry.path(), sink) + }; + + if let Err(err) = result { + log::error!("Global search error: {}, {}", entry.path().display(), err); + } + if stop { + WalkState::Quit + } else { + WalkState::Continue + } + }) }); + Ok(()) + } + .boxed() + }; - cx.jobs.callback(async move { - let call = move |_: &mut Editor, compositor: &mut Compositor| { - let picker = Picker::with_stream( - picker, - 0, - injector, - move |cx, FileResult { path, line_num, .. }, action| { - let doc = match cx.editor.open(path, action) { - Ok(id) => doc_mut!(cx.editor, &id), - Err(e) => { - cx.editor.set_error(format!( - "Failed to open file '{}': {}", - path.display(), - e - )); - return; - } - }; + let mut picker = Picker::new( + columns, + 1, // contents + vec![], + config, + move |cx, FileResult { path, line_num, .. }, action| { + let doc = match cx.editor.open(path, action) { + Ok(id) => doc_mut!(cx.editor, &id), + Err(e) => { + cx.editor + .set_error(format!("Failed to open file '{}': {}", path.display(), e)); + return; + } + }; - let line_num = *line_num; - let view = view_mut!(cx.editor); - let text = doc.text(); - if line_num >= text.len_lines() { - cx.editor.set_error( + let line_num = *line_num; + let view = view_mut!(cx.editor); + let text = doc.text(); + if line_num >= text.len_lines() { + cx.editor.set_error( "The line you jumped to does not exist anymore because the file has changed.", ); - return; - } - let start = text.line_to_char(line_num); - let end = text.line_to_char((line_num + 1).min(text.len_lines())); + return; + } + let start = text.line_to_char(line_num); + let end = text.line_to_char((line_num + 1).min(text.len_lines())); - doc.set_selection(view.id, Selection::single(start, end)); - if action.align_view(view, doc.id()) { - align_view(doc, view, Align::Center); - } - }, - ) - .with_preview( - |_editor, FileResult { path, line_num, .. }| { - Some((path.clone().into(), Some((*line_num, *line_num)))) - }, - ); - compositor.push(Box::new(overlaid(picker))) - }; - Ok(Callback::EditorCompositor(Box::new(call))) - }) - } else { - // Otherwise do nothing - // log::warn!("Global Search Invalid Pattern") + doc.set_selection(view.id, Selection::single(start, end)); + if action.align_view(view, doc.id()) { + align_view(doc, view, Align::Center); } }, - ); + ) + .with_preview(|_editor, FileResult { path, line_num, .. }| { + Some((path.clone().into(), Some((*line_num, *line_num)))) + }) + .with_dynamic_query(get_files, Some(275)); + + if let Some((reg, line)) = cx + .register + .and_then(|reg| Some((reg, cx.editor.registers.first(reg, cx.editor)?))) + { + picker = picker.with_line(line.into_owned(), cx.editor); + cx.editor.registers.last_search_register = reg; + } + + cx.push_layer(Box::new(overlaid(picker))); } enum Extend { From 7b1131adf62f649daeb197b07327f5729d20a2b7 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 6 Mar 2024 11:04:37 -0500 Subject: [PATCH 11/20] global_search: Suggest latest '/' register value --- helix-term/src/commands.rs | 14 +++++--------- helix-term/src/ui/picker.rs | 22 ++++++++++++++++------ helix-term/src/ui/prompt.rs | 21 +++++++++++++++------ 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b7ffeeb7751c..1df25ba521c1 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2407,7 +2407,10 @@ fn global_search(cx: &mut Context) { .boxed() }; - let mut picker = Picker::new( + let reg = cx.register.unwrap_or('/'); + cx.editor.registers.last_search_register = reg; + + let picker = Picker::new( columns, 1, // contents vec![], @@ -2443,16 +2446,9 @@ fn global_search(cx: &mut Context) { .with_preview(|_editor, FileResult { path, line_num, .. }| { Some((path.clone().into(), Some((*line_num, *line_num)))) }) + .with_history_register(Some(reg)) .with_dynamic_query(get_files, Some(275)); - if let Some((reg, line)) = cx - .register - .and_then(|reg| Some((reg, cx.editor.registers.first(reg, cx.editor)?))) - { - picker = picker.with_line(line.into_owned(), cx.editor); - cx.editor.registers.last_search_register = reg; - } - cx.push_layer(Box::new(overlaid(picker))); } diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index bfec9ddb4f08..76a1805a0557 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -391,9 +391,8 @@ impl Picker { self } - pub fn with_line(mut self, line: String, editor: &Editor) -> Self { - self.prompt.set_line(line, editor); - self.handle_prompt_change(); + pub fn with_history_register(mut self, history_register: Option) -> Self { + self.prompt.with_history_register(history_register); self } @@ -977,10 +976,21 @@ impl Component for Picker { - if let Some(option) = self.selection() { - (self.callback_fn)(ctx, option, Action::Replace); + // If the prompt has a history completion and is empty, use enter to accept + // that completion + if let Some(completion) = self + .prompt + .first_history_completion(ctx.editor) + .filter(|_| self.prompt.line().is_empty()) + { + self.prompt.set_line(completion.to_string(), ctx.editor); + self.handle_prompt_change(); + } else { + if let Some(option) = self.selection() { + (self.callback_fn)(ctx, option, Action::Replace); + } + return close_fn(self); } - return close_fn(self); } ctrl!('s') => { if let Some(option) = self.selection() { diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 19183470c7a6..8e8896183d60 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -117,6 +117,19 @@ impl Prompt { &self.line } + pub fn with_history_register(&mut self, history_register: Option) -> &mut Self { + self.history_register = history_register; + self + } + + pub(crate) fn first_history_completion<'a>( + &'a self, + editor: &'a Editor, + ) -> Option> { + self.history_register + .and_then(|reg| editor.registers.first(reg, editor)) + } + pub fn recalculate_completion(&mut self, editor: &Editor) { self.exit_selection(); self.completion = (self.completion_fn)(editor, &self.line); @@ -480,10 +493,7 @@ impl Prompt { let line_area = area.clip_left(self.prompt.len() as u16).clip_top(line); if self.line.is_empty() { // Show the most recently entered value as a suggestion. - if let Some(suggestion) = self - .history_register - .and_then(|reg| cx.editor.registers.first(reg, cx.editor)) - { + if let Some(suggestion) = self.first_history_completion(cx.editor) { surface.set_string(line_area.x, line_area.y, suggestion, suggestion_color); } } else if let Some((language, loader)) = self.language.as_ref() { @@ -578,8 +588,7 @@ impl Component for Prompt { self.recalculate_completion(cx.editor); } else { let last_item = self - .history_register - .and_then(|reg| cx.editor.registers.first(reg, cx.editor)) + .first_history_completion(cx.editor) .map(|entry| entry.to_string()) .unwrap_or_else(|| String::from("")); From 6492f17e7da54081d9fb3bd2bf6828d0ef7f1a87 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Sun, 24 Mar 2024 11:52:57 -0400 Subject: [PATCH 12/20] Add a hidden column for the global search line contents We could expand on this in the future to have different preview modes that you can toggle between with C-t. Currently that binding just hides the preview but it could switch between different preview modes and in one mode hide the path and just show the line contents. --- helix-term/src/commands.rs | 17 ++++------------- helix-term/src/ui/picker.rs | 24 +++++++++++++++++++++++- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 1df25ba521c1..b23c5278755a 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2243,15 +2243,13 @@ fn global_search(cx: &mut Context) { path: PathBuf, /// 0 indexed lines line_num: usize, - line_content: String, } impl FileResult { - fn new(path: &Path, line_num: usize, line_content: String) -> Self { + fn new(path: &Path, line_num: usize) -> Self { Self { path: path.to_path_buf(), line_num, - line_content, } } } @@ -2272,10 +2270,7 @@ fn global_search(cx: &mut Context) { let path = helix_stdx::path::get_relative_path(&item.path); format!("{}:{}", path.to_string_lossy(), item.line_num + 1).into() }), - PickerColumn::new("contents", |item: &FileResult, _| { - item.line_content.as_str().into() - }) - .without_filtering(), + PickerColumn::hidden("contents"), ]; let get_files = |query: &str, @@ -2355,13 +2350,9 @@ fn global_search(cx: &mut Context) { }; let mut stop = false; - let sink = sinks::UTF8(|line_num, line_content| { + let sink = sinks::UTF8(|line_num, _line_content| { stop = injector - .push(FileResult::new( - entry.path(), - line_num as usize - 1, - line_content.to_string(), - )) + .push(FileResult::new(entry.path(), line_num as usize - 1)) .is_err(); Ok(!stop) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 76a1805a0557..6394fb87fb3e 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -191,6 +191,7 @@ pub struct Column { /// `DynamicPicker` uses this so that the dynamic column (for example regex in /// global search) is not used for filtering twice. filter: bool, + hidden: bool, } impl Column { @@ -199,6 +200,19 @@ impl Column { name: name.into(), format, filter: true, + hidden: false, + } + } + + /// A column which does not display any contents + pub fn hidden(name: impl Into>) -> Self { + let format = |_: &T, _: &D| unreachable!(); + + Self { + name: name.into(), + format, + filter: false, + hidden: true, } } @@ -677,6 +691,10 @@ impl Picker { let mut matcher_index = 0; Row::new(self.columns.iter().map(|column| { + if column.hidden { + return Cell::default(); + } + let Some(Constraint::Length(max_width)) = widths.next() else { unreachable!(); }; @@ -757,7 +775,11 @@ impl Picker { let header_style = cx.editor.theme.get("ui.picker.header"); table = table.header(Row::new(self.columns.iter().map(|column| { - Cell::from(Span::styled(Cow::from(&*column.name), header_style)) + if column.hidden { + Cell::default() + } else { + Cell::from(Span::styled(Cow::from(&*column.name), header_style)) + } }))); } From 6ccbfe9bdfedb95f2e3bb8e80bfdb11b5d01797e Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 26 Mar 2024 15:13:51 -0400 Subject: [PATCH 13/20] Request a UI redraw on Drop of an Injector This fixes the changed files picker when used against a clean worktree for example. Without it the running indicator does not disappear. It also simplifies the dynamic query handler's implementation so that it doesn't need to request a redraw explicitly. Co-authored-by: Pascal Kuthe --- helix-event/src/lib.rs | 4 +++- helix-event/src/redraw.rs | 9 +++++++++ helix-term/src/ui/picker.rs | 9 +++++++++ helix-term/src/ui/picker/handlers.rs | 7 ++----- 4 files changed, 23 insertions(+), 6 deletions(-) diff --git a/helix-event/src/lib.rs b/helix-event/src/lib.rs index 894de5e8d628..de018a79ddca 100644 --- a/helix-event/src/lib.rs +++ b/helix-event/src/lib.rs @@ -34,7 +34,9 @@ use anyhow::Result; pub use cancel::{cancelable_future, cancelation, CancelRx, CancelTx}; pub use debounce::{send_blocking, AsyncHook}; -pub use redraw::{lock_frame, redraw_requested, request_redraw, start_frame, RenderLockGuard}; +pub use redraw::{ + lock_frame, redraw_requested, request_redraw, start_frame, RenderLockGuard, RequestRedrawOnDrop, +}; pub use registry::Event; mod cancel; diff --git a/helix-event/src/redraw.rs b/helix-event/src/redraw.rs index 8fadb8aeaa64..d1a188995bc4 100644 --- a/helix-event/src/redraw.rs +++ b/helix-event/src/redraw.rs @@ -51,3 +51,12 @@ pub fn start_frame() { pub fn lock_frame() -> RenderLockGuard { RENDER_LOCK.read() } + +/// A zero sized type that requests a redraw via [request_redraw] when the type [Drop]s. +pub struct RequestRedrawOnDrop; + +impl Drop for RequestRedrawOnDrop { + fn drop(&mut self) { + request_redraw(); + } +} diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 6394fb87fb3e..1c47552d0723 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -153,6 +153,12 @@ pub struct Injector { editor_data: Arc, version: usize, picker_version: Arc, + /// A marker that requests a redraw when the injector drops. + /// This marker causes the "running" indicator to disappear when a background job + /// providing items is finished and drops. This could be wrapped in an [Arc] to ensure + /// that the redraw is only requested when all Injectors drop for a Picker (which removes + /// the "running" indicator) but the redraw handle is debounced so this is unnecessary. + _redraw: helix_event::RequestRedrawOnDrop, } impl Clone for Injector { @@ -163,6 +169,7 @@ impl Clone for Injector { editor_data: self.editor_data.clone(), version: self.version, picker_version: self.picker_version.clone(), + _redraw: helix_event::RequestRedrawOnDrop, } } } @@ -284,6 +291,7 @@ impl Picker { editor_data: Arc::new(editor_data), version: 0, picker_version: Arc::new(AtomicUsize::new(0)), + _redraw: helix_event::RequestRedrawOnDrop, }; (matcher, streamer) } @@ -386,6 +394,7 @@ impl Picker { editor_data: self.editor_data.clone(), version: self.version.load(atomic::Ordering::Relaxed), picker_version: self.version.clone(), + _redraw: helix_event::RequestRedrawOnDrop, } } diff --git a/helix-term/src/ui/picker/handlers.rs b/helix-term/src/ui/picker/handlers.rs index e426adda7f87..4896ccbc6139 100644 --- a/helix-term/src/ui/picker/handlers.rs +++ b/helix-term/src/ui/picker/handlers.rs @@ -174,11 +174,8 @@ impl AsyncHook for DynamicQu 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(); + // NOTE: the Drop implementation of Injector will request a redraw when the + // injector falls out of scope here, clearing the "running" indicator. }); }) } From 408282097f4df04444b87283c5a64e09b8509122 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Tue, 2 Apr 2024 02:34:20 +0200 Subject: [PATCH 14/20] avoid collecting columns to a temporary vec --- Cargo.lock | 4 ++-- Cargo.toml | 2 +- helix-term/src/ui/picker.rs | 13 ++++--------- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d8e65a8f3a3b..89c8553c02dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1779,9 +1779,9 @@ dependencies = [ [[package]] name = "nucleo" -version = "0.4.1" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6350a138d8860658523a7593cbf6813999d17a099371d14f70c5c905b37593e9" +checksum = "5262af4c94921c2646c5ac6ff7900c2af9cbb08dc26a797e18130a7019c039d4" dependencies = [ "nucleo-matcher", "parking_lot", diff --git a/Cargo.toml b/Cargo.toml index d9541b648c86..e7f78442882b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ package.helix-term.opt-level = 2 [workspace.dependencies] tree-sitter = { version = "0.22" } -nucleo = "0.4.1" +nucleo = "0.5.0" slotmap = "1.0.7" thiserror = "1.0" diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 1c47552d0723..69a87f25bc63 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -15,7 +15,7 @@ use crate::{ use futures_util::future::BoxFuture; use helix_event::AsyncHook; use nucleo::pattern::{CaseMatching, Normalization}; -use nucleo::{Config, Nucleo, Utf32String}; +use nucleo::{Config, Nucleo}; use thiserror::Error; use tokio::sync::mpsc::Sender; use tui::{ @@ -135,14 +135,9 @@ fn inject_nucleo_item( item: T, editor_data: &D, ) { - let column_texts: Vec = columns - .iter() - .filter(|column| column.filter) - .map(|column| column.format_text(&item, editor_data).into()) - .collect(); - injector.push(item, |dst| { - for (i, text) in column_texts.into_iter().enumerate() { - dst[i] = text; + injector.push(item, |item, dst| { + for (column, text) in columns.iter().filter(|column| column.filter).zip(dst) { + *text = column.format_text(item, editor_data).into() } }); } From f4a433f855880667eb7888557c2e8a4b6d2c6dd3 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 5 Apr 2024 12:20:35 -0400 Subject: [PATCH 15/20] Convert LSP URIs into custom URIs This introduces a custom URI type in core meant to be extended later if we want to support other schemes. For now it's just a wrapper over a PathBuf. We use this new URI type to firewall `lsp::Url`. This was previously done in 8141a4a but using a custom URI type is more flexible and will improve the way Pickers handle paths for previews in the child commit(s). Co-authored-by: soqb --- Cargo.lock | 1 + helix-core/Cargo.toml | 1 + helix-core/src/lib.rs | 3 + helix-core/src/uri.rs | 122 +++++++++++++++++++++++++++++++++ helix-term/src/application.rs | 26 +++---- helix-term/src/commands/lsp.rs | 63 ++++++++++------- helix-view/src/document.rs | 4 ++ helix-view/src/editor.rs | 12 ++-- helix-view/src/handlers/lsp.rs | 67 ++++++++++++------ 9 files changed, 235 insertions(+), 64 deletions(-) create mode 100644 helix-core/src/uri.rs diff --git a/Cargo.lock b/Cargo.lock index 89c8553c02dc..f1cd1632fa11 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1331,6 +1331,7 @@ dependencies = [ "unicode-general-category", "unicode-segmentation", "unicode-width", + "url", ] [[package]] diff --git a/helix-core/Cargo.toml b/helix-core/Cargo.toml index c6b87e6822fe..392b4a4ca5c1 100644 --- a/helix-core/Cargo.toml +++ b/helix-core/Cargo.toml @@ -34,6 +34,7 @@ bitflags = "2.6" ahash = "0.8.11" hashbrown = { version = "0.14.5", features = ["raw"] } dunce = "1.0" +url = "2.5.0" log = "0.4" serde = { version = "1.0", features = ["derive"] } diff --git a/helix-core/src/lib.rs b/helix-core/src/lib.rs index 1abd90d10b21..681d3456dc3b 100644 --- a/helix-core/src/lib.rs +++ b/helix-core/src/lib.rs @@ -27,6 +27,7 @@ pub mod test; pub mod text_annotations; pub mod textobject; mod transaction; +pub mod uri; pub mod wrap; pub mod unicode { @@ -66,3 +67,5 @@ pub use diagnostic::Diagnostic; pub use line_ending::{LineEnding, NATIVE_LINE_ENDING}; pub use transaction::{Assoc, Change, ChangeSet, Deletion, Operation, Transaction}; + +pub use uri::Uri; diff --git a/helix-core/src/uri.rs b/helix-core/src/uri.rs new file mode 100644 index 000000000000..4e03c58b16df --- /dev/null +++ b/helix-core/src/uri.rs @@ -0,0 +1,122 @@ +use std::path::{Path, PathBuf}; + +/// A generic pointer to a file location. +/// +/// Currently this type only supports paths to local files. +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] +#[non_exhaustive] +pub enum Uri { + File(PathBuf), +} + +impl Uri { + // This clippy allow mirrors url::Url::from_file_path + #[allow(clippy::result_unit_err)] + pub fn to_url(&self) -> Result { + match self { + Uri::File(path) => url::Url::from_file_path(path), + } + } + + pub fn as_path(&self) -> Option<&Path> { + match self { + Self::File(path) => Some(path), + } + } + + pub fn as_path_buf(self) -> Option { + match self { + Self::File(path) => Some(path), + } + } +} + +impl From for Uri { + fn from(path: PathBuf) -> Self { + Self::File(path) + } +} + +impl TryFrom for PathBuf { + type Error = (); + + fn try_from(uri: Uri) -> Result { + match uri { + Uri::File(path) => Ok(path), + } + } +} + +#[derive(Debug)] +pub struct UrlConversionError { + source: url::Url, + kind: UrlConversionErrorKind, +} + +#[derive(Debug)] +pub enum UrlConversionErrorKind { + UnsupportedScheme, + UnableToConvert, +} + +impl std::fmt::Display for UrlConversionError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.kind { + UrlConversionErrorKind::UnsupportedScheme => { + write!(f, "unsupported scheme in URL: {}", self.source.scheme()) + } + UrlConversionErrorKind::UnableToConvert => { + write!(f, "unable to convert URL to file path: {}", self.source) + } + } + } +} + +impl std::error::Error for UrlConversionError {} + +fn convert_url_to_uri(url: &url::Url) -> Result { + if url.scheme() == "file" { + url.to_file_path() + .map(|path| Uri::File(helix_stdx::path::normalize(path))) + .map_err(|_| UrlConversionErrorKind::UnableToConvert) + } else { + Err(UrlConversionErrorKind::UnsupportedScheme) + } +} + +impl TryFrom for Uri { + type Error = UrlConversionError; + + fn try_from(url: url::Url) -> Result { + convert_url_to_uri(&url).map_err(|kind| Self::Error { source: url, kind }) + } +} + +impl TryFrom<&url::Url> for Uri { + type Error = UrlConversionError; + + fn try_from(url: &url::Url) -> Result { + convert_url_to_uri(url).map_err(|kind| Self::Error { + source: url.clone(), + kind, + }) + } +} + +#[cfg(test)] +mod test { + use super::*; + use url::Url; + + #[test] + fn unknown_scheme() { + let url = Url::parse("csharp:/metadata/foo/bar/Baz.cs").unwrap(); + assert!(matches!( + Uri::try_from(url), + Err(UrlConversionError { + kind: UrlConversionErrorKind::UnsupportedScheme, + .. + }) + )); + } +} diff --git a/helix-term/src/application.rs b/helix-term/src/application.rs index 9adc764cc51b..9695703b7421 100644 --- a/helix-term/src/application.rs +++ b/helix-term/src/application.rs @@ -735,10 +735,10 @@ impl Application { } } Notification::PublishDiagnostics(mut params) => { - let path = match params.uri.to_file_path() { - Ok(path) => helix_stdx::path::normalize(path), - Err(_) => { - log::error!("Unsupported file URI: {}", params.uri); + let uri = match helix_core::Uri::try_from(params.uri) { + Ok(uri) => uri, + Err(err) => { + log::error!("{err}"); return; } }; @@ -749,11 +749,11 @@ impl Application { } // have to inline the function because of borrow checking... let doc = self.editor.documents.values_mut() - .find(|doc| doc.path().map(|p| p == &path).unwrap_or(false)) + .find(|doc| doc.uri().is_some_and(|u| u == uri)) .filter(|doc| { if let Some(version) = params.version { if version != doc.version() { - log::info!("Version ({version}) is out of date for {path:?} (expected ({}), dropping PublishDiagnostic notification", doc.version()); + log::info!("Version ({version}) is out of date for {uri:?} (expected ({}), dropping PublishDiagnostic notification", doc.version()); return false; } } @@ -765,7 +765,7 @@ impl Application { let lang_conf = doc.language.clone(); if let Some(lang_conf) = &lang_conf { - if let Some(old_diagnostics) = self.editor.diagnostics.get(&path) { + if let Some(old_diagnostics) = self.editor.diagnostics.get(&uri) { if !lang_conf.persistent_diagnostic_sources.is_empty() { // Sort diagnostics first by severity and then by line numbers. // Note: The `lsp::DiagnosticSeverity` enum is already defined in decreasing order @@ -798,7 +798,7 @@ impl Application { // 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. - let diagnostics = match self.editor.diagnostics.entry(path) { + let diagnostics = match self.editor.diagnostics.entry(uri) { Entry::Occupied(o) => { let current_diagnostics = o.into_mut(); // there may entries of other language servers, which is why we can't overwrite the whole entry @@ -1132,20 +1132,22 @@ impl Application { .. } = params; - let path = match uri.to_file_path() { - Ok(path) => path, + let uri = match helix_core::Uri::try_from(uri) { + Ok(uri) => uri, Err(err) => { - log::error!("unsupported file URI: {}: {:?}", uri, err); + log::error!("{err}"); return lsp::ShowDocumentResult { success: false }; } }; + // If `Uri` gets another variant other than `Path` this may not be valid. + let path = uri.as_path().expect("URIs are valid paths"); let action = match take_focus { Some(true) => helix_view::editor::Action::Replace, _ => helix_view::editor::Action::VerticalSplit, }; - let doc_id = match self.editor.open(&path, action) { + let doc_id = match self.editor.open(path, action) { Ok(id) => id, Err(err) => { log::error!("failed to open path: {:?}: {:?}", uri, err); diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 23bcb977f728..fae0833e074f 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -13,7 +13,9 @@ use tui::{text::Span, widgets::Row}; use super::{align_view, push_jump, Align, Context, Editor}; -use helix_core::{syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection}; +use helix_core::{ + syntax::LanguageServerFeature, text_annotations::InlineAnnotation, Selection, Uri, +}; use helix_stdx::path; use helix_view::{ document::{DocumentInlayHints, DocumentInlayHintsId}, @@ -34,7 +36,7 @@ use std::{ collections::{BTreeMap, HashSet}, fmt::Write, future::Future, - path::{Path, PathBuf}, + path::Path, }; /// Gets the first language server that is attached to a document which supports a specific feature. @@ -72,7 +74,7 @@ struct DiagnosticStyles { } struct PickerDiagnostic { - path: PathBuf, + uri: Uri, diag: lsp::Diagnostic, offset_encoding: OffsetEncoding, } @@ -183,20 +185,20 @@ type DiagnosticsPicker = Picker; fn diag_picker( cx: &Context, - diagnostics: BTreeMap>, + diagnostics: BTreeMap>, format: DiagnosticsFormat, ) -> DiagnosticsPicker { // 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 (path, diags) in diagnostics { + for (uri, diags) in diagnostics { flat_diag.reserve(diags.len()); for (diag, ls) in diags { if let Some(ls) = cx.editor.language_server_by_id(ls) { flat_diag.push(PickerDiagnostic { - path: path.clone(), + uri: uri.clone(), diag, offset_encoding: ls.offset_encoding(), }); @@ -243,8 +245,14 @@ fn diag_picker( // between message code and message 2, ui::PickerColumn::new("path", |item: &PickerDiagnostic, _| { - let path = path::get_truncated_path(&item.path); - path.to_string_lossy().to_string().into() + if let Some(path) = item.uri.as_path() { + path::get_truncated_path(path) + .to_string_lossy() + .to_string() + .into() + } else { + Default::default() + } }), ); primary_column += 1; @@ -257,17 +265,20 @@ fn diag_picker( styles, move |cx, PickerDiagnostic { - path, + uri, diag, offset_encoding, }, action| { + let Some(path) = uri.as_path() else { + return; + }; jump_to_position(cx.editor, path, diag.range, *offset_encoding, action) }, ) - .with_preview(move |_editor, PickerDiagnostic { path, diag, .. }| { + .with_preview(move |_editor, PickerDiagnostic { uri, diag, .. }| { let line = Some((diag.range.start.line as usize, diag.range.end.line as usize)); - Some((path.clone().into(), line)) + Some((uri.clone().as_path_buf()?.into(), line)) }) .truncate_start(false) } @@ -456,12 +467,17 @@ pub fn workspace_symbol_picker(cx: &mut Context) { }) .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(), + if let Ok(uri) = Uri::try_from(&item.symbol.location.uri) { + if let Some(path) = uri.as_path() { + path::get_relative_path(path) + .to_string_lossy() + .to_string() + .into() + } else { + item.symbol.location.uri.to_string().into() + } + } else { + item.symbol.location.uri.to_string().into() } }), ]; @@ -489,16 +505,11 @@ pub fn workspace_symbol_picker(cx: &mut Context) { pub fn diagnostics_picker(cx: &mut Context) { let doc = doc!(cx.editor); - if let Some(current_path) = doc.path() { - let diagnostics = cx - .editor - .diagnostics - .get(current_path) - .cloned() - .unwrap_or_default(); + if let Some(uri) = doc.uri() { + let diagnostics = cx.editor.diagnostics.get(&uri).cloned().unwrap_or_default(); let picker = diag_picker( cx, - [(current_path.clone(), diagnostics)].into(), + [(uri, diagnostics)].into(), DiagnosticsFormat::HideSourcePath, ); cx.push_layer(Box::new(overlaid(picker))); @@ -842,6 +853,8 @@ fn goto_impl( // With the preallocation above and UTF-8 paths already, this closure will do one (1) // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`. if let Ok(path) = item.uri.to_file_path() { + // We don't convert to a `helix_core::Uri` here because we've already checked the scheme. + // This path won't be normalized but it's only used for display. res.push_str( &path.strip_prefix(cwdir).unwrap_or(&path).to_string_lossy(), ); diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index ccf2fa8c387c..3314a243fd8b 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1741,6 +1741,10 @@ impl Document { Url::from_file_path(self.path()?).ok() } + pub fn uri(&self) -> Option { + Some(self.path()?.clone().into()) + } + #[inline] pub fn text(&self) -> &Rope { &self.text diff --git a/helix-view/src/editor.rs b/helix-view/src/editor.rs index 3eeb4830ed21..ae942cf081fa 100644 --- a/helix-view/src/editor.rs +++ b/helix-view/src/editor.rs @@ -44,7 +44,7 @@ pub use helix_core::diagnostic::Severity; use helix_core::{ auto_pairs::AutoPairs, syntax::{self, AutoPairConfig, IndentationHeuristic, LanguageServerFeature, SoftWrap}, - Change, LineEnding, Position, Range, Selection, NATIVE_LINE_ENDING, + Change, LineEnding, Position, Range, Selection, Uri, NATIVE_LINE_ENDING, }; use helix_dap as dap; use helix_lsp::lsp; @@ -1022,7 +1022,7 @@ pub struct Editor { pub macro_recording: Option<(char, Vec)>, pub macro_replaying: Vec, pub language_servers: helix_lsp::Registry, - pub diagnostics: BTreeMap>, + pub diagnostics: BTreeMap>, pub diff_providers: DiffProviderRegistry, pub debugger: Option, @@ -1929,7 +1929,7 @@ impl Editor { /// Returns all supported diagnostics for the document pub fn doc_diagnostics<'a>( language_servers: &'a helix_lsp::Registry, - diagnostics: &'a BTreeMap>, + diagnostics: &'a BTreeMap>, document: &Document, ) -> impl Iterator + 'a { Editor::doc_diagnostics_with_filter(language_servers, diagnostics, document, |_, _| true) @@ -1939,15 +1939,15 @@ impl Editor { /// filtered by `filter` which is invocated with the raw `lsp::Diagnostic` and the language server id it came from pub fn doc_diagnostics_with_filter<'a>( language_servers: &'a helix_lsp::Registry, - diagnostics: &'a BTreeMap>, + diagnostics: &'a BTreeMap>, document: &Document, filter: impl Fn(&lsp::Diagnostic, LanguageServerId) -> bool + 'a, ) -> impl Iterator + 'a { let text = document.text().clone(); let language_config = document.language.clone(); document - .path() - .and_then(|path| diagnostics.get(path)) + .uri() + .and_then(|uri| diagnostics.get(&uri)) .map(|diags| { diags.iter().filter_map(move |(diagnostic, lsp_id)| { let ls = language_servers.get_by_id(*lsp_id)?; diff --git a/helix-view/src/handlers/lsp.rs b/helix-view/src/handlers/lsp.rs index beb106b2bf83..d817a423556a 100644 --- a/helix-view/src/handlers/lsp.rs +++ b/helix-view/src/handlers/lsp.rs @@ -1,6 +1,7 @@ use crate::editor::Action; use crate::Editor; use crate::{DocumentId, ViewId}; +use helix_core::Uri; use helix_lsp::util::generate_transaction_from_edits; use helix_lsp::{lsp, OffsetEncoding}; @@ -54,18 +55,30 @@ pub struct ApplyEditError { pub enum ApplyEditErrorKind { DocumentChanged, FileNotFound, - UnknownURISchema, + InvalidUrl(helix_core::uri::UrlConversionError), IoError(std::io::Error), // TODO: check edits before applying and propagate failure // InvalidEdit, } +impl From for ApplyEditErrorKind { + fn from(err: std::io::Error) -> Self { + ApplyEditErrorKind::IoError(err) + } +} + +impl From for ApplyEditErrorKind { + fn from(err: helix_core::uri::UrlConversionError) -> Self { + ApplyEditErrorKind::InvalidUrl(err) + } +} + impl ToString for ApplyEditErrorKind { fn to_string(&self) -> String { match self { ApplyEditErrorKind::DocumentChanged => "document has changed".to_string(), ApplyEditErrorKind::FileNotFound => "file not found".to_string(), - ApplyEditErrorKind::UnknownURISchema => "URI schema not supported".to_string(), + ApplyEditErrorKind::InvalidUrl(err) => err.to_string(), ApplyEditErrorKind::IoError(err) => err.to_string(), } } @@ -74,25 +87,28 @@ impl ToString for ApplyEditErrorKind { impl Editor { fn apply_text_edits( &mut self, - uri: &helix_lsp::Url, + url: &helix_lsp::Url, version: Option, text_edits: Vec, offset_encoding: OffsetEncoding, ) -> Result<(), ApplyEditErrorKind> { - let path = match uri.to_file_path() { - Ok(path) => path, - Err(_) => { - let err = format!("unable to convert URI to filepath: {}", uri); - log::error!("{}", err); - self.set_error(err); - return Err(ApplyEditErrorKind::UnknownURISchema); + let uri = match Uri::try_from(url) { + Ok(uri) => uri, + Err(err) => { + log::error!("{err}"); + return Err(err.into()); } }; + let path = uri.as_path().expect("URIs are valid paths"); - let doc_id = match self.open(&path, Action::Load) { + let doc_id = match self.open(path, Action::Load) { Ok(doc_id) => doc_id, Err(err) => { - let err = format!("failed to open document: {}: {}", uri, err); + let err = format!( + "failed to open document: {}: {}", + path.to_string_lossy(), + err + ); log::error!("{}", err); self.set_error(err); return Err(ApplyEditErrorKind::FileNotFound); @@ -158,9 +174,9 @@ impl Editor { for (i, operation) in operations.iter().enumerate() { match operation { lsp::DocumentChangeOperation::Op(op) => { - self.apply_document_resource_op(op).map_err(|io| { + self.apply_document_resource_op(op).map_err(|err| { ApplyEditError { - kind: ApplyEditErrorKind::IoError(io), + kind: err, failed_change_idx: i, } })?; @@ -214,12 +230,18 @@ impl Editor { Ok(()) } - fn apply_document_resource_op(&mut self, op: &lsp::ResourceOp) -> std::io::Result<()> { + fn apply_document_resource_op( + &mut self, + op: &lsp::ResourceOp, + ) -> Result<(), ApplyEditErrorKind> { use lsp::ResourceOp; use std::fs; + // NOTE: If `Uri` gets another variant than `Path`, the below `expect`s + // may no longer be valid. match op { ResourceOp::Create(op) => { - let path = op.uri.to_file_path().unwrap(); + let uri = Uri::try_from(&op.uri)?; + let path = uri.as_path_buf().expect("URIs are valid paths"); let ignore_if_exists = op.options.as_ref().map_or(false, |options| { !options.overwrite.unwrap_or(false) && options.ignore_if_exists.unwrap_or(false) }); @@ -236,7 +258,8 @@ impl Editor { } } ResourceOp::Delete(op) => { - let path = op.uri.to_file_path().unwrap(); + let uri = Uri::try_from(&op.uri)?; + let path = uri.as_path_buf().expect("URIs are valid paths"); if path.is_dir() { let recursive = op .options @@ -251,17 +274,19 @@ impl Editor { } self.language_servers.file_event_handler.file_changed(path); } else if path.is_file() { - fs::remove_file(&path)?; + fs::remove_file(path)?; } } ResourceOp::Rename(op) => { - let from = op.old_uri.to_file_path().unwrap(); - let to = op.new_uri.to_file_path().unwrap(); + let from_uri = Uri::try_from(&op.old_uri)?; + let from = from_uri.as_path().expect("URIs are valid paths"); + let to_uri = Uri::try_from(&op.new_uri)?; + let to = to_uri.as_path().expect("URIs are valid paths"); let ignore_if_exists = op.options.as_ref().map_or(false, |options| { !options.overwrite.unwrap_or(false) && options.ignore_if_exists.unwrap_or(false) }); if !ignore_if_exists || !to.exists() { - self.move_path(&from, &to)?; + self.move_path(from, to)?; } } } From 3906f6605fd1ba1ed72ada9f4bbd8b88facc51ce Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Fri, 5 Apr 2024 14:49:02 -0400 Subject: [PATCH 16/20] Avoid allocations in Picker file preview callback The `FileLocation` and `PathOrId` types can borrow paths rather than requiring them to be owned. This takes a refactor of the preview functions and preview internals within `Picker`. With this change we avoid an unnecessary `PathBuf` clone per render for any picker with a file preview function (i.e. most pickers). This refactor is not fully complete. The `PathOrId` is _sometimes_ an owned `PathBuf`. This is for pragmatic reasons rather than technical ones. We need a further refactor to introduce more core types like `Location` in order to eliminate the Cow and only use `&Path`s within `PathOrId`. This is left for future work as it will be a larger refactor almost entirely fitting into the LSP commands module and helix-core - i.e. mostly unrelated to refactoring the `Picker` code itself. Co-authored-by: Pascal Kuthe --- helix-term/src/commands.rs | 4 +- helix-term/src/commands/dap.rs | 6 +-- helix-term/src/commands/lsp.rs | 90 ++++++++++++++++++++++++---------- helix-term/src/ui/mod.rs | 2 +- helix-term/src/ui/picker.rs | 84 ++++++++++++++++--------------- 5 files changed, 112 insertions(+), 74 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index b23c5278755a..7e59bbdd6594 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2435,7 +2435,7 @@ fn global_search(cx: &mut Context) { }, ) .with_preview(|_editor, FileResult { path, line_num, .. }| { - Some((path.clone().into(), Some((*line_num, *line_num)))) + Some((path.as_path().into(), Some((*line_num, *line_num)))) }) .with_history_register(Some(reg)) .with_dynamic_query(get_files, Some(275)); @@ -3098,7 +3098,7 @@ fn changed_file_picker(cx: &mut Context) { } }, ) - .with_preview(|_editor, meta| Some((meta.path().to_path_buf().into(), None))); + .with_preview(|_editor, meta| Some((meta.path().into(), None))); let injector = picker.injector(); cx.editor diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 14cb83d290e9..39c79a66ca22 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -61,7 +61,7 @@ fn thread_picker( .with_preview(move |editor, thread| { let frames = editor.debugger.as_ref()?.stack_frames.get(&thread.id)?; let frame = frames.first()?; - let path = frame.source.as_ref()?.path.clone()?; + let path = frame.source.as_ref()?.path.as_ref()?.as_path(); let pos = Some(( frame.line.saturating_sub(1), frame.end_line.unwrap_or(frame.line).saturating_sub(1), @@ -747,10 +747,10 @@ pub fn dap_switch_stack_frame(cx: &mut Context) { frame .source .as_ref() - .and_then(|source| source.path.clone()) + .and_then(|source| source.path.as_ref()) .map(|path| { ( - path.into(), + path.as_path().into(), Some(( frame.line.saturating_sub(1), frame.end_line.unwrap_or(frame.line).saturating_sub(1), diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index fae0833e074f..059fb814df09 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -64,6 +64,7 @@ macro_rules! language_server_with_feature { struct SymbolInformationItem { symbol: lsp::SymbolInformation, offset_encoding: OffsetEncoding, + uri: Uri, } struct DiagnosticStyles { @@ -79,13 +80,10 @@ struct PickerDiagnostic { offset_encoding: OffsetEncoding, } -fn location_to_file_location(location: &lsp::Location) -> FileLocation { - let path = location.uri.to_file_path().unwrap(); - let line = Some(( - location.range.start.line as usize, - location.range.end.line as usize, - )); - (path.into(), line) +fn uri_to_file_location<'a>(uri: &'a Uri, range: &lsp::Range) -> Option> { + let path = uri.as_path()?; + let line = Some((range.start.line as usize, range.end.line as usize)); + Some((path.into(), line)) } fn jump_to_location( @@ -278,7 +276,7 @@ fn diag_picker( ) .with_preview(move |_editor, PickerDiagnostic { uri, diag, .. }| { let line = Some((diag.range.start.line as usize, diag.range.end.line as usize)); - Some((uri.clone().as_path_buf()?.into(), line)) + Some((uri.as_path()?.into(), line)) }) .truncate_start(false) } @@ -287,6 +285,7 @@ pub fn symbol_picker(cx: &mut Context) { fn nested_to_flat( list: &mut Vec, file: &lsp::TextDocumentIdentifier, + uri: &Uri, symbol: lsp::DocumentSymbol, offset_encoding: OffsetEncoding, ) { @@ -301,9 +300,10 @@ pub fn symbol_picker(cx: &mut Context) { container_name: None, }, offset_encoding, + uri: uri.clone(), }); for child in symbol.children.into_iter().flatten() { - nested_to_flat(list, file, child, offset_encoding); + nested_to_flat(list, file, uri, child, offset_encoding); } } let doc = doc!(cx.editor); @@ -317,6 +317,9 @@ pub fn symbol_picker(cx: &mut Context) { let request = language_server.document_symbols(doc.identifier()).unwrap(); let offset_encoding = language_server.offset_encoding(); let doc_id = doc.identifier(); + let doc_uri = doc + .uri() + .expect("docs with active language servers must be backed by paths"); async move { let json = request.await?; @@ -331,6 +334,7 @@ pub fn symbol_picker(cx: &mut Context) { lsp::DocumentSymbolResponse::Flat(symbols) => symbols .into_iter() .map(|symbol| SymbolInformationItem { + uri: doc_uri.clone(), symbol, offset_encoding, }) @@ -338,7 +342,13 @@ pub fn symbol_picker(cx: &mut Context) { lsp::DocumentSymbolResponse::Nested(symbols) => { let mut flat_symbols = Vec::new(); for symbol in symbols { - nested_to_flat(&mut flat_symbols, &doc_id, symbol, offset_encoding) + nested_to_flat( + &mut flat_symbols, + &doc_id, + &doc_uri, + symbol, + offset_encoding, + ) } flat_symbols } @@ -388,7 +398,7 @@ pub fn symbol_picker(cx: &mut Context) { }, ) .with_preview(move |_editor, item| { - Some(location_to_file_location(&item.symbol.location)) + uri_to_file_location(&item.uri, &item.symbol.location.range) }) .truncate_start(false); @@ -431,9 +441,19 @@ pub fn workspace_symbol_picker(cx: &mut Context) { serde_json::from_value::>>(json)? .unwrap_or_default() .into_iter() - .map(|symbol| SymbolInformationItem { - symbol, - offset_encoding, + .filter_map(|symbol| { + let uri = match Uri::try_from(&symbol.location.uri) { + Ok(uri) => uri, + Err(err) => { + log::warn!("discarding symbol with invalid URI: {err}"); + return None; + } + }; + Some(SymbolInformationItem { + symbol, + uri, + offset_encoding, + }) }) .collect(); @@ -467,15 +487,11 @@ pub fn workspace_symbol_picker(cx: &mut Context) { }) .without_filtering(), ui::PickerColumn::new("path", |item: &SymbolInformationItem, _| { - if let Ok(uri) = Uri::try_from(&item.symbol.location.uri) { - if let Some(path) = uri.as_path() { - path::get_relative_path(path) - .to_string_lossy() - .to_string() - .into() - } else { - item.symbol.location.uri.to_string().into() - } + if let Some(path) = item.uri.as_path() { + path::get_relative_path(path) + .to_string_lossy() + .to_string() + .into() } else { item.symbol.location.uri.to_string().into() } @@ -496,7 +512,7 @@ pub fn workspace_symbol_picker(cx: &mut Context) { ); }, ) - .with_preview(|_editor, item| Some(location_to_file_location(&item.symbol.location))) + .with_preview(|_editor, item| uri_to_file_location(&item.uri, &item.symbol.location.range)) .with_dynamic_query(get_symbols, None) .truncate_start(false); @@ -875,7 +891,31 @@ fn goto_impl( let picker = Picker::new(columns, 0, locations, cwdir, move |cx, location, action| { jump_to_location(cx.editor, location, offset_encoding, action) }) - .with_preview(move |_editor, location| Some(location_to_file_location(location))); + .with_preview(move |_editor, location| { + use crate::ui::picker::PathOrId; + + let lines = Some(( + location.range.start.line as usize, + location.range.end.line as usize, + )); + + // TODO: we should avoid allocating by doing the Uri conversion ahead of time. + // + // To do this, introduce a `Location` type in `helix-core` that reuses the core + // `Uri` type instead of the LSP `Url` type and replaces the LSP `Range` type. + // Refactor the callers of `goto_impl` to pass iterators that translate the + // LSP location type to the custom one in core, or have them collect and pass + // `Vec`s. Replace the `uri_to_file_location` function with + // `location_to_file_location` that takes only `&helix_core::Location` as + // parameters. + // + // By doing this we can also eliminate the duplicated URI info in the + // `SymbolInformationItem` type and introduce a custom Symbol type in `helix-core` + // which will be reused in the future for tree-sitter based symbol pickers. + let path = Uri::try_from(&location.uri).ok()?.as_path_buf()?; + #[allow(deprecated)] + Some((PathOrId::from_path_buf(path), lines)) + }); compositor.push(Box::new(overlaid(picker))); } } diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 93ac2e6516e8..14d92b575949 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -244,7 +244,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi } }, ) - .with_preview(|_editor, path| Some((path.clone().into(), None))); + .with_preview(|_editor, path| Some((path.as_path().into(), None))); let injector = picker.injector(); let timeout = std::time::Instant::now() + std::time::Duration::from_millis(30); diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 69a87f25bc63..4f50953002d3 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -60,37 +60,41 @@ pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72; pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024; #[derive(PartialEq, Eq, Hash)] -pub enum PathOrId { +pub enum PathOrId<'a> { Id(DocumentId), - Path(Arc), + // See [PathOrId::from_path_buf]: this will eventually become `Path(&Path)`. + Path(Cow<'a, Path>), } -impl PathOrId { - fn get_canonicalized(self) -> Self { - use PathOrId::*; - match self { - Path(path) => Path(helix_stdx::path::canonicalize(&path).into()), - Id(id) => Id(id), - } +impl<'a> PathOrId<'a> { + /// Creates a [PathOrId] from a PathBuf + /// + /// # Deprecated + /// The owned version of PathOrId will be removed in a future refactor + /// and replaced with `&'a Path`. See the caller of this function for + /// more details on its removal. + #[deprecated] + pub fn from_path_buf(path_buf: PathBuf) -> Self { + Self::Path(Cow::Owned(path_buf)) } } -impl From for PathOrId { - fn from(v: PathBuf) -> Self { - Self::Path(v.as_path().into()) +impl<'a> From<&'a Path> for PathOrId<'a> { + fn from(path: &'a Path) -> Self { + Self::Path(Cow::Borrowed(path)) } } -impl From for PathOrId { +impl<'a> From for PathOrId<'a> { fn from(v: DocumentId) -> Self { Self::Id(v) } } -type FileCallback = Box Option>; +type FileCallback = Box Fn(&'a Editor, &'a T) -> Option>>; /// File path and range of lines (used to align and highlight lines) -pub type FileLocation = (PathOrId, Option<(usize, usize)>); +pub type FileLocation<'a> = (PathOrId<'a>, Option<(usize, usize)>); pub enum CachedPreview { Document(Box), @@ -400,7 +404,7 @@ impl Picker { pub fn with_preview( mut self, - preview_fn: impl Fn(&Editor, &T) -> Option + 'static, + preview_fn: impl for<'a> Fn(&'a Editor, &'a T) -> Option> + 'static, ) -> Self { self.file_fn = Some(Box::new(preview_fn)); // assumption: if we have a preview we are matching paths... If this is ever @@ -544,40 +548,35 @@ impl Picker { } } - fn current_file(&self, editor: &Editor) -> Option { - self.selection() - .and_then(|current| (self.file_fn.as_ref()?)(editor, current)) - .map(|(path_or_id, line)| (path_or_id.get_canonicalized(), line)) - } - - /// Get (cached) preview for a given path. If a document corresponding + /// Get (cached) preview for the currently selected item. If a document corresponding /// to the path is already open in the editor, it is used instead. fn get_preview<'picker, 'editor>( &'picker mut self, - path_or_id: PathOrId, editor: &'editor Editor, - ) -> Preview<'picker, 'editor> { + ) -> Option<(Preview<'picker, 'editor>, Option<(usize, usize)>)> { + let current = self.selection()?; + let (path_or_id, range) = (self.file_fn.as_ref()?)(editor, current)?; + match path_or_id { PathOrId::Path(path) => { - if let Some(doc) = editor.document_by_path(&path) { - return Preview::EditorDocument(doc); + let path = path.as_ref(); + if let Some(doc) = editor.document_by_path(path) { + return Some((Preview::EditorDocument(doc), range)); } - if self.preview_cache.contains_key(&path) { - let preview = &self.preview_cache[&path]; - match preview { - // If the document isn't highlighted yet, attempt to highlight it. - CachedPreview::Document(doc) if doc.language_config().is_none() => { - helix_event::send_blocking( - &self.preview_highlight_handler, - path.clone(), - ); - } - _ => (), + if self.preview_cache.contains_key(path) { + // NOTE: we use `HashMap::get_key_value` here instead of indexing so we can + // retrieve the `Arc` key. The `path` in scope here is a `&Path` and + // we can cheaply clone the key for the preview highlight handler. + let (path, preview) = self.preview_cache.get_key_value(path).unwrap(); + if matches!(preview, CachedPreview::Document(doc) if doc.language_config().is_none()) + { + helix_event::send_blocking(&self.preview_highlight_handler, path.clone()); } - return Preview::Cached(preview); + return Some((Preview::Cached(preview), range)); } + let path: Arc = path.into(); let data = std::fs::File::open(&path).and_then(|file| { let metadata = file.metadata()?; // Read up to 1kb to detect the content type @@ -607,11 +606,11 @@ impl Picker { ) .unwrap_or(CachedPreview::NotFound); self.preview_cache.insert(path.clone(), preview); - Preview::Cached(&self.preview_cache[&path]) + Some((Preview::Cached(&self.preview_cache[&path]), range)) } PathOrId::Id(id) => { let doc = editor.documents.get(&id).unwrap(); - Preview::EditorDocument(doc) + Some((Preview::EditorDocument(doc), range)) } } } @@ -816,8 +815,7 @@ impl Picker { let inner = inner.inner(margin); BLOCK.render(area, surface); - if let Some((path, range)) = self.current_file(cx.editor) { - let preview = self.get_preview(path, cx.editor); + if let Some((preview, range)) = self.get_preview(cx.editor) { let doc = match preview.document() { Some(doc) if range.map_or(true, |(start, end)| { From 8555248b012ddfc4578c5d82f9c7b31495b03ca0 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 23 Apr 2024 14:34:43 -0400 Subject: [PATCH 17/20] Accept 'IntoIterator' for Picker::new options `Picker::new` loops through the input options to inject each of them, so there's no need to collect into an intermediary Vec. This removes some unnecessary collections. Also, pickers that start with no initial options can now pass an empty slice instead of an empty Vec. Co-authored-by: Luis Useche --- helix-term/src/commands.rs | 37 ++++++++++++++++------------------ helix-term/src/commands/lsp.rs | 2 +- helix-term/src/ui/mod.rs | 26 +++++++++--------------- helix-term/src/ui/picker.rs | 2 +- 4 files changed, 29 insertions(+), 38 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 7e59bbdd6594..8476959467d8 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2404,7 +2404,7 @@ fn global_search(cx: &mut Context) { let picker = Picker::new( columns, 1, // contents - vec![], + [], config, move |cx, FileResult { path, line_num, .. }, action| { let doc = match cx.editor.open(path, action) { @@ -2991,16 +2991,12 @@ fn jumplist_picker(cx: &mut Context) { let picker = Picker::new( columns, 1, // path - cx.editor - .tree - .views() - .flat_map(|(view, _)| { - view.jumps - .iter() - .rev() - .map(|(doc_id, selection)| new_meta(view, *doc_id, selection.clone())) - }) - .collect(), + cx.editor.tree.views().flat_map(|(view, _)| { + view.jumps + .iter() + .rev() + .map(|(doc_id, selection)| new_meta(view, *doc_id, selection.clone())) + }), (), |cx, meta, action| { cx.editor.switch(meta.id, action); @@ -3077,7 +3073,7 @@ fn changed_file_picker(cx: &mut Context) { let picker = Picker::new( columns, 1, // path - Vec::new(), + [], FileChangeData { cwd: cwd.clone(), style_untracked: added, @@ -3124,14 +3120,15 @@ pub fn command_palette(cx: &mut Context) { [&cx.editor.mode] .reverse_map(); - let mut commands: Vec = MappableCommand::STATIC_COMMAND_LIST.into(); - commands.extend(typed::TYPABLE_COMMAND_LIST.iter().map(|cmd| { - MappableCommand::Typable { - name: cmd.name.to_owned(), - doc: cmd.doc.to_owned(), - args: Vec::new(), - } - })); + let commands = MappableCommand::STATIC_COMMAND_LIST.iter().cloned().chain( + typed::TYPABLE_COMMAND_LIST + .iter() + .map(|cmd| MappableCommand::Typable { + name: cmd.name.to_owned(), + args: Vec::new(), + doc: cmd.doc.to_owned(), + }), + ); let columns = vec![ ui::PickerColumn::new("name", |item, _| match item { diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 059fb814df09..601c58ebee9a 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -501,7 +501,7 @@ pub fn workspace_symbol_picker(cx: &mut Context) { let picker = Picker::new( columns, 1, // name column - vec![], + [], (), move |cx, item, action| { jump_to_location( diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index 14d92b575949..e1ecf0a66d65 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -228,22 +228,16 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi .into() }, )]; - let picker = Picker::new( - columns, - 0, - Vec::new(), - root, - move |cx, path: &PathBuf, action| { - if let Err(e) = cx.editor.open(path, action) { - let err = if let Some(err) = e.source() { - format!("{}", err) - } else { - format!("unable to open \"{}\"", path.display()) - }; - cx.editor.set_error(err); - } - }, - ) + let picker = Picker::new(columns, 0, [], root, move |cx, path: &PathBuf, action| { + if let Err(e) = cx.editor.open(path, action) { + let err = if let Some(err) = e.source() { + format!("{}", err) + } else { + format!("unable to open \"{}\"", path.display()) + }; + cx.editor.set_error(err); + } + }) .with_preview(|_editor, path| Some((path.as_path().into(), None))); let injector = picker.injector(); let timeout = std::time::Instant::now() + std::time::Duration::from_millis(30); diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 4f50953002d3..ebc06cc02832 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -298,7 +298,7 @@ impl Picker { pub fn new( columns: Vec>, primary_column: usize, - options: Vec, + options: impl IntoIterator, editor_data: D, callback_fn: impl Fn(&mut Context, &T, Action) + 'static, ) -> Self { From 009bbdaf8dc076178cc888cfaa1513bf1baa35f5 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 23 Apr 2024 20:25:56 -0400 Subject: [PATCH 18/20] Picker: Reset the cursor on prompt change --- helix-term/src/ui/picker.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index ebc06cc02832..c65e839cb0f8 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -512,6 +512,8 @@ impl Picker { if self.query == old_query { return; } + // If the query has meaningfully changed, reset the cursor to the top of the results. + self.cursor = 0; // Have nucleo reparse each changed column. for (i, column) in self .columns From a7777b3c118d7cd08868cb688e281b290898a31e Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 24 Apr 2024 09:22:22 -0400 Subject: [PATCH 19/20] Accept 'IntoIterator>' for picker columns This allows us to replace any `vec![..]`s of columns where all columns are static with static slices `[..]`. --- helix-term/src/commands.rs | 10 +++++----- helix-term/src/commands/dap.rs | 6 +++--- helix-term/src/commands/lsp.rs | 6 +++--- helix-term/src/commands/typed.rs | 2 +- helix-term/src/ui/mod.rs | 2 +- helix-term/src/ui/picker.rs | 26 ++++++++++++++++++-------- 6 files changed, 31 insertions(+), 21 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 8476959467d8..097c3493cf91 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -2265,7 +2265,7 @@ fn global_search(cx: &mut Context) { file_picker_config: config.file_picker.clone(), }; - let columns = vec![ + let columns = [ PickerColumn::new("path", |item: &FileResult, _| { let path = helix_stdx::path::get_relative_path(&item.path); format!("{}:{}", path.to_string_lossy(), item.line_num + 1).into() @@ -2886,7 +2886,7 @@ fn buffer_picker(cx: &mut Context) { // mru items.sort_unstable_by_key(|item| std::cmp::Reverse(item.focused_at)); - let columns = vec![ + let columns = [ PickerColumn::new("id", |meta: &BufferMeta, _| meta.id.to_string().into()), PickerColumn::new("flags", |meta: &BufferMeta, _| { let mut flags = String::new(); @@ -2960,7 +2960,7 @@ fn jumplist_picker(cx: &mut Context) { } }; - let columns = vec![ + let columns = [ ui::PickerColumn::new("id", |item: &JumpMeta, _| item.id.to_string().into()), ui::PickerColumn::new("path", |item: &JumpMeta, _| { let path = item @@ -3039,7 +3039,7 @@ fn changed_file_picker(cx: &mut Context) { let deleted = cx.editor.theme.get("diff.minus"); let renamed = cx.editor.theme.get("diff.delta.moved"); - let columns = vec![ + let columns = [ PickerColumn::new("change", |change: &FileChange, data: &FileChangeData| { match change { FileChange::Untracked { .. } => Span::styled("+ untracked", data.style_untracked), @@ -3130,7 +3130,7 @@ pub fn command_palette(cx: &mut Context) { }), ); - let columns = vec![ + let columns = [ ui::PickerColumn::new("name", |item, _| match item { MappableCommand::Typable { name, .. } => format!(":{name}").into(), MappableCommand::Static { name, .. } => (*name).into(), diff --git a/helix-term/src/commands/dap.rs b/helix-term/src/commands/dap.rs index 39c79a66ca22..0b754bc21992 100644 --- a/helix-term/src/commands/dap.rs +++ b/helix-term/src/commands/dap.rs @@ -41,7 +41,7 @@ fn thread_picker( let debugger = debugger!(editor); let thread_states = debugger.thread_states.clone(); - let columns = vec![ + let columns = [ ui::PickerColumn::new("name", |item: &Thread, _| item.name.as_str().into()), ui::PickerColumn::new("state", |item: &Thread, thread_states: &ThreadStates| { thread_states @@ -250,7 +250,7 @@ pub fn dap_launch(cx: &mut Context) { let templates = config.templates.clone(); - let columns = vec![ui::PickerColumn::new( + let columns = [ui::PickerColumn::new( "template", |item: &DebugTemplate, _| item.name.as_str().into(), )]; @@ -725,7 +725,7 @@ pub fn dap_switch_stack_frame(cx: &mut Context) { let frames = debugger.stack_frames[&thread_id].clone(); - let columns = vec![ui::PickerColumn::new("frame", |item: &StackFrame, _| { + let columns = [ui::PickerColumn::new("frame", |item: &StackFrame, _| { item.name.as_str().into() // TODO: include thread_states in the label })]; let picker = Picker::new(columns, 0, frames, (), move |cx, frame, _action| { diff --git a/helix-term/src/commands/lsp.rs b/helix-term/src/commands/lsp.rs index 601c58ebee9a..103d1df24ebd 100644 --- a/helix-term/src/commands/lsp.rs +++ b/helix-term/src/commands/lsp.rs @@ -371,7 +371,7 @@ pub fn symbol_picker(cx: &mut Context) { symbols.append(&mut lsp_items); } let call = move |_editor: &mut Editor, compositor: &mut Compositor| { - let columns = vec![ + let columns = [ ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { display_symbol_kind(item.symbol.kind).into() }), @@ -478,7 +478,7 @@ pub fn workspace_symbol_picker(cx: &mut Context) { } .boxed() }; - let columns = vec![ + let columns = [ ui::PickerColumn::new("kind", |item: &SymbolInformationItem, _| { display_symbol_kind(item.symbol.kind).into() }), @@ -855,7 +855,7 @@ fn goto_impl( } [] => unreachable!("`locations` should be non-empty for `goto_impl`"), _locations => { - let columns = vec![ui::PickerColumn::new( + let columns = [ui::PickerColumn::new( "location", |item: &lsp::Location, cwdir: &std::path::PathBuf| { // The preallocation here will overallocate a few characters since it will account for the diff --git a/helix-term/src/commands/typed.rs b/helix-term/src/commands/typed.rs index f65f96574a69..1b828b3f4330 100644 --- a/helix-term/src/commands/typed.rs +++ b/helix-term/src/commands/typed.rs @@ -1404,7 +1404,7 @@ fn lsp_workspace_command( let callback = async move { let call: job::Callback = Callback::EditorCompositor(Box::new( move |_editor: &mut Editor, compositor: &mut Compositor| { - let columns = vec![ui::PickerColumn::new( + let columns = [ui::PickerColumn::new( "title", |(_ls_id, command): &(_, helix_lsp::lsp::Command), _| { command.title.as_str().into() diff --git a/helix-term/src/ui/mod.rs b/helix-term/src/ui/mod.rs index e1ecf0a66d65..fae64062d11b 100644 --- a/helix-term/src/ui/mod.rs +++ b/helix-term/src/ui/mod.rs @@ -219,7 +219,7 @@ pub fn file_picker(root: PathBuf, config: &helix_view::editor::Config) -> FilePi }); log::debug!("file_picker init {:?}", Instant::now().duration_since(now)); - let columns = vec![PickerColumn::new( + let columns = [PickerColumn::new( "path", |item: &PathBuf, root: &PathBuf| { item.strip_prefix(root) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index c65e839cb0f8..8694316543a7 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -275,7 +275,11 @@ pub struct Picker { } impl Picker { - pub fn stream(columns: Vec>, editor_data: D) -> (Nucleo, Injector) { + pub fn stream( + columns: impl IntoIterator>, + editor_data: D, + ) -> (Nucleo, Injector) { + let columns: Arc<[_]> = columns.into_iter().collect(); let matcher_columns = columns.iter().filter(|col| col.filter).count() as u32; assert!(matcher_columns > 0); let matcher = Nucleo::new( @@ -286,7 +290,7 @@ impl Picker { ); let streamer = Injector { dst: matcher.injector(), - columns: columns.into(), + columns, editor_data: Arc::new(editor_data), version: 0, picker_version: Arc::new(AtomicUsize::new(0)), @@ -295,13 +299,19 @@ impl Picker { (matcher, streamer) } - pub fn new( - columns: Vec>, + pub fn new( + columns: C, primary_column: usize, - options: impl IntoIterator, + options: O, editor_data: D, - callback_fn: impl Fn(&mut Context, &T, Action) + 'static, - ) -> Self { + callback_fn: F, + ) -> Self + where + C: IntoIterator>, + O: IntoIterator, + F: Fn(&mut Context, &T, Action) + 'static, + { + let columns: Arc<[_]> = columns.into_iter().collect(); let matcher_columns = columns.iter().filter(|col| col.filter).count() as u32; assert!(matcher_columns > 0); let matcher = Nucleo::new( @@ -316,7 +326,7 @@ impl Picker { } Self::with( matcher, - columns.into(), + columns, primary_column, Arc::new(editor_data), Arc::new(AtomicUsize::new(0)), From 9de5f5cefab30f5c6d0b8ebbc0a1c5310a67e7ea Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Thu, 25 Apr 2024 16:13:48 -0400 Subject: [PATCH 20/20] Picker: Highlight the currently active column We can track the ranges in the input text that correspond to each column and use this information during rendering to apply a new theme key that makes the "active column" stand out. This makes it easier to tell at a glance which column you're entering. --- book/src/themes.md | 1 + helix-term/src/ui/picker.rs | 10 +++- helix-term/src/ui/picker/query.rs | 94 +++++++++++++++++++++++++++++-- helix-term/src/ui/prompt.rs | 6 ++ 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/book/src/themes.md b/book/src/themes.md index a59df2fd72e1..b8e271374947 100644 --- a/book/src/themes.md +++ b/book/src/themes.md @@ -298,6 +298,7 @@ These scopes are used for theming the editor interface: | `ui.popup` | Documentation popups (e.g. Space + k) | | `ui.popup.info` | Prompt for multiple key options | | `ui.picker.header` | Column names in pickers with multiple columns | +| `ui.picker.header.active` | The column name in pickers with multiple columns where the cursor is entering into. | | `ui.window` | Borderlines separating splits | | `ui.help` | Description box for commands | | `ui.text` | Default text style, command prompts, popup text, etc. | diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 8694316543a7..079012396a1e 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -787,13 +787,21 @@ impl Picker { // -- Header if self.columns.len() > 1 { + let active_column = self.query.active_column(self.prompt.position()); let header_style = cx.editor.theme.get("ui.picker.header"); table = table.header(Row::new(self.columns.iter().map(|column| { if column.hidden { Cell::default() } else { - Cell::from(Span::styled(Cow::from(&*column.name), header_style)) + let style = if active_column.is_some_and(|name| Arc::ptr_eq(name, &column.name)) + { + cx.editor.theme.get("ui.picker.header.active") + } else { + header_style + }; + + Cell::from(Span::styled(Cow::from(&*column.name), style)) } }))); } diff --git a/helix-term/src/ui/picker/query.rs b/helix-term/src/ui/picker/query.rs index 89ade95ff4a1..e433a11fa224 100644 --- a/helix-term/src/ui/picker/query.rs +++ b/helix-term/src/ui/picker/query.rs @@ -1,4 +1,4 @@ -use std::{collections::HashMap, mem, sync::Arc}; +use std::{collections::HashMap, mem, ops::Range, sync::Arc}; #[derive(Debug)] pub(super) struct PickerQuery { @@ -11,6 +11,10 @@ pub(super) struct PickerQuery { /// The mapping between column names and input in the query /// for those columns. inner: HashMap, Arc>, + /// The byte ranges of the input text which are used as input for each column. + /// This is calculated at parsing time for use in [Self::active_column]. + /// This Vec is naturally sorted in ascending order and ranges do not overlap. + column_ranges: Vec<(Range, Option>)>, } impl PartialEq, Arc>> for PickerQuery { @@ -26,10 +30,12 @@ impl PickerQuery { ) -> Self { let column_names: Box<[_]> = column_names.collect(); let inner = HashMap::with_capacity(column_names.len()); + let column_ranges = vec![(0..usize::MAX, Some(column_names[primary_column].clone()))]; Self { column_names, primary_column, inner, + column_ranges, } } @@ -44,6 +50,9 @@ impl PickerQuery { let mut in_field = false; let mut field = None; let mut text = String::new(); + self.column_ranges.clear(); + self.column_ranges + .push((0..usize::MAX, Some(primary_field.clone()))); macro_rules! finish_field { () => { @@ -59,7 +68,7 @@ impl PickerQuery { }; } - for ch in input.chars() { + for (idx, ch) in input.char_indices() { match ch { // Backslash escaping _ if escaped => { @@ -77,9 +86,19 @@ impl PickerQuery { if !text.is_empty() { finish_field!(); } + let (range, _field) = self + .column_ranges + .last_mut() + .expect("column_ranges is non-empty"); + range.end = idx; in_field = true; } ' ' if in_field => { + text.clear(); + in_field = false; + } + _ if in_field => { + text.push(ch); // Go over all columns and their indices, find all that starts with field key, // select a column that fits key the most. field = self @@ -88,8 +107,17 @@ impl PickerQuery { .filter(|col| col.starts_with(&text)) // select "fittest" column .min_by_key(|col| col.len()); - text.clear(); - in_field = false; + + // Update the column range for this column. + if let Some((_range, current_field)) = self + .column_ranges + .last_mut() + .filter(|(range, _)| range.end == usize::MAX) + { + *current_field = field.cloned(); + } else { + self.column_ranges.push((idx..usize::MAX, field.cloned())); + } } _ => text.push(ch), } @@ -106,6 +134,23 @@ impl PickerQuery { mem::replace(&mut self.inner, new_inner) } + + /// Finds the column which the cursor is 'within' in the last parse. + /// + /// The cursor is considered to be within a column when it is placed within any + /// of a column's text. See the `active_column_test` unit test below for examples. + /// + /// `cursor` is a byte index that represents the location of the prompt's cursor. + pub fn active_column(&self, cursor: usize) -> Option<&Arc> { + let point = self + .column_ranges + .partition_point(|(range, _field)| cursor > range.end); + + self.column_ranges + .get(point) + .filter(|(range, _field)| cursor >= range.start && cursor <= range.end) + .and_then(|(_range, field)| field.as_ref()) + } } #[cfg(test)] @@ -279,4 +324,45 @@ mod test { ) ); } + + #[test] + fn active_column_test() { + fn active_column<'a>(query: &'a mut PickerQuery, input: &str) -> Option<&'a str> { + let cursor = input.find('|').expect("cursor must be indicated with '|'"); + let input = input.replace('|', ""); + query.parse(&input); + query.active_column(cursor).map(AsRef::as_ref) + } + + let mut query = PickerQuery::new( + ["primary".into(), "foo".into(), "bar".into()].into_iter(), + 0, + ); + + assert_eq!(active_column(&mut query, "|"), Some("primary")); + assert_eq!(active_column(&mut query, "hello| world"), Some("primary")); + assert_eq!(active_column(&mut query, "|%foo hello"), Some("primary")); + assert_eq!(active_column(&mut query, "%foo|"), Some("foo")); + assert_eq!(active_column(&mut query, "%|"), None); + assert_eq!(active_column(&mut query, "%baz|"), None); + assert_eq!(active_column(&mut query, "%quiz%|"), None); + assert_eq!(active_column(&mut query, "%foo hello| world"), Some("foo")); + assert_eq!(active_column(&mut query, "%foo hello world|"), Some("foo")); + assert_eq!(active_column(&mut query, "%foo| hello world"), Some("foo")); + assert_eq!(active_column(&mut query, "%|foo hello world"), Some("foo")); + assert_eq!(active_column(&mut query, "%f|oo hello world"), Some("foo")); + assert_eq!(active_column(&mut query, "hello %f|oo world"), Some("foo")); + assert_eq!( + active_column(&mut query, "hello %f|oo world %bar !"), + Some("foo") + ); + assert_eq!( + active_column(&mut query, "hello %foo wo|rld %bar !"), + Some("foo") + ); + assert_eq!( + active_column(&mut query, "hello %foo world %bar !|"), + Some("bar") + ); + } } diff --git a/helix-term/src/ui/prompt.rs b/helix-term/src/ui/prompt.rs index 8e8896183d60..3518ddf774e5 100644 --- a/helix-term/src/ui/prompt.rs +++ b/helix-term/src/ui/prompt.rs @@ -92,6 +92,12 @@ impl Prompt { } } + /// Gets the byte index in the input representing the current cursor location. + #[inline] + pub(crate) fn position(&self) -> usize { + self.cursor + } + pub fn with_line(mut self, line: String, editor: &Editor) -> Self { self.set_line(line, editor); self