Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reuse menu::Item trait in picker #2814

Merged
merged 8 commits into from
Jul 2, 2022

Conversation

sudormrfbin
Copy link
Member

@sudormrfbin sudormrfbin commented Jun 19, 2022

This opens the way for merging the menu and picker code in the future, since a picker is essentially a menu + prompt. More excitingly, this change will also allow aligning items in the picker, which would be useful (for example) in the command palette for aligning the descriptions to the left and the keybinds to the right in two separate columns. This requires more work though, and this PR only lays the foundations for it.

The item formatting of each picker has been kept as is, even though there is room for improvement now that we can format the data into columns, since that is better tackled in a separate PR.

Will be useful for storing editor state when reused by pickers.
This opens the way for merging the menu and picker code in the
future, since a picker is essentially a menu + prompt. More
excitingly, this change will also allow aligning items in the
picker, which would be useful (for example) in the command palette
for aligning the descriptions to the left and the keybinds to
the right in two separate columns.

The item formatting of each picker has been kept as is, even though
there is room for improvement now that we can format the data into
columns, since that is better tackled in a separate PR.
@sudormrfbin sudormrfbin changed the title Refactor menu::Item to accomodate external state Reuse menu::Item trait in picker Jun 20, 2022
Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change, I attempted this before but didn't think of using an associated type for the external data.

helix-term/src/ui/menu.rs Outdated Show resolved Hide resolved
helix-term/src/ui/completion.rs Outdated Show resolved Hide resolved
@archseer
Copy link
Member

More excitingly, this change will also allow aligning items in the picker, which would be useful (for example) in the command palette for aligning the descriptions to the left and the keybinds to the right in two separate columns.

This is something I want for #2452 and as a follow-up to #2013

Copy link
Member

@archseer archseer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@archseer
Copy link
Member

archseer commented Jul 1, 2022

Ah wait, didn't see the conflicts :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants