Configurable labels for custom menus and typable commands

[MattCheely]

I haven't done a ton of Rust development, and only a very little bit with Serde, so consider this an initial draft to see if the approach and proposed config format are sound. I'm happy to add docs as well if this looks reasonable.

[poliorcetics]

That's a very cool change, though you'll have to update the doc.

I'm in mobile so can't check, but does this work recursively or only one level deep ? (I don't expect to ever need more than one level deep personally but others may)

[MattCheely]

It should work recursively. You can have have a labeled menu inside of another labeled menu. A very contrived version might look like:

[keys.normal.space.f]
label = "File"
f = "file_picker"
s = { label = "Save", command = ":write" }
c = { label = "Edit Config", command = ":open ~/.config/helix/config.toml" }

[keys.normal.space.f.e]
label = "Extra"
r = { label = "Reload", command = ":reload" }

I'm happy to update any docs, I just wanted to make sure the approach is sound and that label and command as terms in the configuration were OK before I started in on it.

[PeepNSheep]

This looks great! I use backspace to bring up a menu of build commands for different languages, so it'd be nice to relabel everything to make it look a bit prettier

[MattCheely]

It seems like there are no major objections to this approach, so I am going to try to start working on some documentation.

[MattCheely]

I believe the latest changes should return errors in any cases where I was previously choosing some default behavior that could differ from user intent.

[bbodi]

What is the situation with this MR? @MattCheely are you still working on it?

[MattCheely]

What is the situation with this MR? @MattCheely are you still working on it?

As far as I'm aware I've resolved all of the review comments. I'm just waiting for it to be merged or to get more feedback.

[bbodi]

As far as I'm aware I've resolved all of the review comments. I'm just waiting for it to be merged or to get more feedback.

Thanks for the update!

As I can see the situation, there is an open PR which contains this functionality and much more: #5635, probably that one is more favourable.

[pascalkuthe]

Not necessarily. I think smaller self contained PRS can also be preferable and are a lot easier to review.

Our review bandwith is limited and considering the amount of PRs we recive it might take a while until a PR is reviewed

[gibbz00]

The merge in #5635 is honestly a complete rewrite as the Keymap API is basically re-written, I just wanted to give attribution to the PR author for the work he put in regardless. Sure, this PR is smaller (although the diff stat of the feature isn't). But you would have to put in at least twice the amount of time to review this PR and then mine, for basically the same feature implementations in the end.

(My extension to this feature on top of this PR is the added ability to give command sequences descriptions :) Possibly some documentation clarifications too.)

[MattCheely]

FWIW, I'm not too worried about attribution, so I don't mind what PR gets merged. I'm just keen to have the feature. 😁

[MattCheely]

Not sure if anybody is still interested in the PR, but I've been using it locally. I just rebased on the latest from master, and it seems there were some changes that introduced runtime config parsing errors. Some string type wrangling seems to have resolved it though.

[danillos]

I was testing it and could not make it work when it was a list of commands.

b = { label = "New Buffer", command = ":new" } work
b = { label = "New Buffer", command = [":new"] } don't work