Skip to content

Commit

Permalink
Move FilePicker::render from Component impl to normal impl
Browse files Browse the repository at this point in the history
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 helix-editor#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, helix-editor#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]: helix-editor#5714 (comment)
[mikes-comment]: helix-editor#5714 (comment)
  • Loading branch information
sudormrfbin committed Feb 3, 2023
1 parent 3041236 commit a33b23d
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions helix-term/src/ui/picker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ impl Preview<'_, '_> {
}
}

impl<T: Item> FilePicker<T> {
impl<T: Item + 'static> FilePicker<T> {
pub fn new(
options: Vec<T>,
editor_data: T::Data,
Expand Down Expand Up @@ -227,10 +227,8 @@ impl<T: Item> FilePicker<T> {

EventResult::Consumed(None)
}
}

impl<T: Item + 'static> Component for FilePicker<T> {
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 |
// +---------+ | |
Expand Down Expand Up @@ -345,6 +343,12 @@ impl<T: Item + 'static> Component for FilePicker<T> {
);
}
}
}

impl<T: Item + 'static> Component for FilePicker<T> {
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 {
Expand Down

0 comments on commit a33b23d

Please sign in to comment.