From 92e12dbe8812df1da323117b5ee85a1f18c09335 Mon Sep 17 00:00:00 2001 From: Gokul Soumya Date: Thu, 2 Feb 2023 19:23:06 +0530 Subject: [PATCH] Move FilePicker::render from Component impl to normal impl Merges the code for the Picker and FilePicker into a single Picker that can show a file preview if a preview callback is provided. This change was mainly made to facilitate refactoring out a simple skeleton of a picker that does not do any filtering to be reused in a normal Picker and a DynamicPicker (see #5714; in particular [mikes-comment] and [gokuls-comment]). The crux of the issue is that a picker maintains a list of predefined options (eg. list of files in the directory) and (re-)filters them every time the picker prompt changes, while a dynamic picker (eg. interactive global search, #4687) recalculates the full list of options on every prompt change. Using a filtering picker to drive a dynamic picker hence does duplicate work of filtering thousands of matches for no reason. It could also cause problems like interfering with the regex pattern in the global search. I tried to directly extract a PickerBase to be reused in Picker and FilePicker and DynamicPicker, but the problem is that DynamicPicker is actually a DynamicFilePicker (i.e. it can preview file contents) which means we would need PickerBase, Picker, FilePicker, DynamicPicker and DynamicFilePicker and then another way of sharing the previewing code between a FilePicker and a DynamicFilePicker. By merging Picker and FilePicker into Picker, we only need PickerBase, Picker and DynamicPicker. [gokuls-comment]: https://github.com/helix-editor/helix/issues/5714#issuecomment-1410949578 [mikes-comment]: https://github.com/helix-editor/helix/issues/5714#issuecomment-1407451963 --- helix-term/src/ui/picker.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs index 5fa75136f1048..e1edefadb7718 100644 --- a/helix-term/src/ui/picker.rs +++ b/helix-term/src/ui/picker.rs @@ -120,7 +120,7 @@ impl Preview<'_, '_> { } } -impl FilePicker { +impl FilePicker { pub fn new( options: Vec, editor_data: T::Data, @@ -227,10 +227,8 @@ impl FilePicker { EventResult::Consumed(None) } -} -impl Component for FilePicker { - fn render(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { + fn render_picker(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { // +---------+ +---------+ // |prompt | |preview | // +---------+ | | @@ -345,6 +343,12 @@ impl Component for FilePicker { ); } } +} + +impl Component for FilePicker { + fn render(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) { + self.render_picker(area, surface, cx) + } fn handle_event(&mut self, event: &Event, ctx: &mut Context) -> EventResult { if let Event::IdleTimeout = event {