search

helix-editor / helix

A post-modern modal text editor.
https://helix-editor.com
Mozilla Public License 2.0
32.72k stars 2.41k forks source link

Command palette cannot create new pickers #4508

Open the-mikedavis opened 1 year ago

the-mikedavis commented 1 year ago

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

26f21da531179ccc77e70b73e32a1b458c4e225b

nrabulinski commented 1 year ago

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

woojiq commented 1 year ago

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: https://github.com/helix-editor/helix/blob/5ec126d3e27dcfe2544396b664cbad04611ad7eb/helix-term/src/commands/lsp.rs#L1155

Sync pickers, on the other hand, use the push_layer method, which internally populates the callback field: https://github.com/helix-editor/helix/blob/5ec126d3e27dcfe2544396b664cbad04611ad7eb/helix-term/src/commands.rs#L95-L96

The interesting part about this field is that it only resolves in one place - EditorView::handle_event: https://github.com/helix-editor/helix/blob/5ec126d3e27dcfe2544396b664cbad04611ad7eb/helix-term/src/ui/editor.rs#L1332 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: https://github.com/helix-editor/helix/blob/5ec126d3e27dcfe2544396b664cbad04611ad7eb/helix-term/src/ui/picker.rs#L781 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 commented 1 year ago

I believe the proper fix for this overlaps with #1383 / #4709 (also see https://github.com/helix-editor/helix/pull/5294#issuecomment-1365454233 and #5555). Those need a very large refactor to commands that should probably be taken care of internally (see https://github.com/helix-editor/helix/pull/5581).