Command palette cannot create new pickers

[the-mikedavis]

Summary

Running a command from the command palette (<space>?) which pushes a new picker to the compositor will not create the picker.

Reproduction Steps

<space>?jumplist_picker<ret> should bring up the jumplist picker but it has no effect. Running the jumplist picker command from its keybind (<space>j) works.

Helix log

No response

Platform

Linux

Terminal Emulator

Kitty 0.26.2

Helix Version

26f21da

[nrabulinski]

Has this been picked up yet? If not I can get on it as I identified what the issue is

[woojiq]

What I dug up:

Actually, async pickers work even from the command palette. The reason is that they use callback method that internally add a picker to the application's task list:

helix/helix-term/src/commands/lsp.rs

Line 1155 in 5ec126d

cx.callback(

Sync pickers, on the other hand, use the push_layer method, which internally populates the callback field:

helix/helix-term/src/commands.rs

Lines 95 to 96 in 5ec126d

	pub fn push_layer(&mut self, component: Box<dyn Component>) {
	self.callback = Some(Box::new(|compositor: &mut Compositor, _| {

The interesting part about this field is that it only resolves in one place - EditorView::handle_event:

helix/helix-term/src/ui/editor.rs

Line 1332 in 5ec126d

mode => self.command_mode(mode, &mut cx, key),

So when you populate this field while in the command palette, it will be lost and incomplete just because the Picker has a different handle_event implementation:

helix/helix-term/src/ui/picker.rs

Line 781 in 5ec126d

fn handle_event(&mut self, event: &Event, ctx: &mut Context) -> EventResult {

So far I have only one idea to solve this: inside the push_layer make the callback asynchronous and add it to the application's task list (same as the async pickers do). Otherwise, anything that uses push_layer (file, jumplist, buffer pickers...) will only work if you start them from EditorView (the first layer).

P.S. I wrote this so tomorrow I will recall what I was thinking, xd. Maybe someone will be interested.

[the-mikedavis]

I believe the proper fix for this overlaps with #1383 / #4709 (also see #5294 (comment) and #5555). Those need a very large refactor to commands that should probably be taken care of internally (see #5581).