Support custom keybindings in custom command menu prompts

[HerrNaN]

Summary

resolves: #3626

This adds support for keybindings on menu options.

Example Usage:

customCommands:
  - key: 'e'
    context: 'files'
    command: 'echo {{.Form.Choice | quote}} > result.txt'
    prompts:
      - type: 'menu'
        title: 'Choose an option'
        key: 'Choice'
        options:
          - value: 'foo'
            description: 'FOO'
            key: 'f'
          - value: 'bar'
            description: 'BAR'
            key: 'b'

Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go generate ./...)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
• Docs have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[HerrNaN]

I'm quite sure that the changes can be hot-reloaded since they are part of an already existing config struct. Please let me know if this is wrong and I'll try to fix it!

[HerrNaN]

When I get the time, I'll tidy up the commits and split into 2: precedence fix and adding keybind support

Done

[stefanhaller]

Thanks. The change in the second commit looks good; I find it a bit unfortunate that we have a "key" field in the menu prompt itself, which means something very different than the new "key" fields in the individual prompt items; maybe we should rename the former to "id" or something like that to make this less confusing. But that would be a breaking change, so I'm hesitant to do that. Maybe we'll just live with the awkwardness.

I pushed a fixup (eb5b0f1) to move the doc changes to the right place; see #5010 for more about that.

There's some validation code missing for checking that the keys are valid; without that, if you set a key to "xyz" it will crash when you open the menu. We need to guard against that by adding validation code to user_config_validation.go, similar to how we already do it for other keybindings. I don't have time to add this right now; if you want to give that a try yourself, feel free to go ahead, but if you would just ask copilot to do it, then please don't. (I'm not fond of reviewing ai-generated code.)

As for the first commit, I don't think this is the right approach. It has two problems:

• it works for 'H' and 'L', but not for 'j' and 'k' (although this would easily be solved by removing NextItemAlt and PrevItemAlt from your list of essential keys)
• it would still keep the keys visible that conflict with essential bindings, and that's confusing; we need to remove them from display too.

Since this is a problem that's unrelated to the feature you are adding here (we have the same problem already with custom command menus), I opened another PR for solving these in a better way; see #5131.

[stefanhaller]

We tend to not comment our tests so much. These comments only add noise, I added a fixup to remove most of them (d7a787f).

[HerrNaN]

@stefanhaller

I find it a bit unfortunate that we have a "key" field in the menu prompt itself, which means something very different than the new "key" fields in the individual prompt items; maybe we should rename the former to "id" or something like that to make this less confusing. But that would be a breaking change, so I'm hesitant to do that. Maybe we'll just live with the awkwardness.

I agree that id would be better but I also agree that it might not be as big a deal as to warrant a breaking change.

There's some validation code missing for checking that the keys are valid; without that, if you set a key to "xyz" it will crash when you open the menu. We need to guard against that by adding validation code to user_config_validation.go, similar to how we already do it for other keybindings. I don't have time to add this right now; if you want to give that a try yourself, feel free to go ahead, but if you would just ask copilot to do it, then please don't. (I'm not fond of reviewing ai-generated code.)

I've added validation in a fixup commit (cb001ff), feel free to take a look and give feedback when you have time.

As for the first commit, I don't think this is the right approach. It has two problems:

it works for 'H' and 'L', but not for 'j' and 'k' (although this would easily be solved by removing NextItemAlt and PrevItemAlt from your list of essential keys)
it would still keep the keys visible that conflict with essential bindings, and that's confusing; we need to remove them from display too.
Since this is a problem that's unrelated to the feature you are adding here (we have the same problem already with custom command menus), I opened another PR for solving these in a better way; see #5131.

In this case do you want me to revert these changes and let that be fixed in your linked PR?

[stefanhaller]

I've added validation in a fixup commit (cb001ff),

Very nice! I pushed another fixup for this (283991b) to make the error text more consistent with the other keybinding checks we already have.

In this case do you want me to revert these changes and let that be fixed in your linked PR?

Yes, please drop those changes. (Feel free to squash the fixups too.)

Your tests will not be green without #5131 because the H key doesn't work yet. If we merge that other PR first, yours will be green too. Do you feel like reviewing it?

[HerrNaN]

Sure thing! I'll try to get to the review and cleaning this up tomorrow! 👍

[stefanhaller]

@HerrNaN Thanks! Both PRs are merged, so you can go ahead and rebase this one onto master.

[HerrNaN]

@stefanhaller rebased and ready!

[HerrNaN]

@stefanhaller Thank you for maintaining such a great tool and for fast feedback and merging of my PR!
Happy Holidays!