helix
helix copied to clipboard
Keymap presentation refinements and underlying restructuring + consistent config use
Commit messages explain most of it. Proceeds: https://github.com/helix-editor/helix/pull/3053
Before | After |
---|---|
![]() |
![]() |
![]() |
![]() |
Command palette: Should I change the ">" character to a unicode arrow like: → ? Should I properly escape less than and greater than from lt and gt to < and > respectively?
Infoboxes issues which haven't been touched upon: https://github.com/helix-editor/helix/pull/5203, https://github.com/helix-editor/helix/pull/3958
EDIT: Accidentally snuck in https://github.com/helix-editor/helix/pull/5130, so guess this proceeds that one too, can open up a new PR which leaves it out if needed.
It looks like the keys are now sorted in the popup? We deliberately keep the order the same as defined in the keymap, because left
/right
/up
/down
should be grouped together and not sorted alphabetically for example.
@gibbz00 could you maybe provide a summary of what exactly this does. There are 43 commits in this PR so reading trough them all is kind of a chore
It looks like the keys are now sorted in the popup? We deliberately keep the order the same as defined in the keymap, because
left
/right
/up
/down
should be grouped together and not sorted alphabetically for example.
Sorting only worked when keymap was added by the keymap! macro and not when loaded by user config. Also, arrow keys are often also bound with hjkl keys, which will show first. Order is: one letter -> C keys -> multi-letter. They aren't shown in the images because I have no_op them.
Right, but either way the popup should be arranged in definition order. Certain popups like goto
are manually ordered by grouping together similar sets of functionality.
For instance w
v
and t
are common operations so they were listed on top. left
/right
/up
/down
are now spread across the whole infobox
Right, but either way the popup should be arranged in definition order. Certain popups like
goto
are manually ordered by grouping together similar sets of functionality.For instance
w
v
andt
are common operations so they were listed on top.left
/right
/up
/down
are now spread across the whole infobox
No config:
Hmm it could be non-deterministic? I have no config and it's sorting by arrow direction first (not hjkl)
Hmm it could be non-deterministic? I have no config and it's sorting by arrow direction first (not hjkl)
When using this PR?
Certain popups like
goto
are manually ordered by grouping together similar sets of functionality.
I would still argue that it is easier to find the command when they're sorted alphabetically.
@gibbz00 could you maybe provide a summary of what exactly this does. There are 43 commits in this PR so reading trough them all is kind of a chore
@pascalkuthe clean up of keymap, main.rs, docgen.rs, health.rs. It's a large PR so the commits are the summaries. I'm sorry :(
Alphabetical sort: we had that initially then replaced it, so this is strictly a downgrade. The goto popup is a great example:
- g is on the top since it's a repetition of the key
- h/l are grouped together
- t/c/b are grouped together
- a/m are grouped together
- n/p
- All the jumps that are LSP specific are also grouped
Alphabetical sort is useful if you're trying to find what a specific key does. But the current order is intended to make it easier to find based on functionality. Kakoune also sorts this similarly.
I like some of the cleanup in this PR though so I might start cherry picking changes into master to shrink the size of this PR a bit and make it easier to review. But I'd avoid renaming types just for the sake of clarity where possible: this is a large project with a lot of PRs in queue and any type of renaming can cause merge conflicts on a large portion of them.
I've merged #5130 and #3053 so they can be rebased out from this PR now
Alphabetical sort is useful if you're trying to find what a specific key does. But the current order is intended to make it easier to find based on functionality.
Ok, I'm starting to see your point. I can put the ordering from the default ordering back, can, but can I add alphabetical sort as a config option? I have a lot of config keybindings and the info boxes become because of that very messy.
I can also send in split PRs, maybe that can help with the review process.
But I'd avoid renaming types just for the sake of clarity where possible: this is a large project with a lot of PRs in queue and any type of renaming can cause merge conflicts on a large portion of them.
Of course, I'll try to limit this unless I strongly feel that the naming is outright deceptive.