lazygit icon indicating copy to clipboard operation
lazygit copied to clipboard

Add alternative Confirm and Return hotkeys for confirmation windows

Open vlad-psh opened this issue 2 years ago • 9 comments

PR Description

Motivation:

Currently we have confirm (enter) and return (esc) key bindings which are being used everywhere extensively. Some localization are even assuming that we'll never remap these keys (and for a good reason), eg: https://github.com/jesseduffield/lazygit/blob/master/pkg/i18n/english.go#L828

Indeed, for many cases (such as search fields, input prompts, etc) any other hotkeys doesn't makes much sense (especially if they're just a single English letter)

But sometimes it may be convenient to use keys, located closer than enter/esc to confirm/cancel simple actions.

In this PR I added two new configuration options (unset by default), so it will be possible to configure alternatives for these actions: confirm-alt and return-alt

PS: Previously I opened an issue, but discovered that I can fix this by myself without knowing golang :) https://github.com/jesseduffield/lazygit/issues/2767

Please check if the PR fulfills these requirements

  • [x] Cheatsheets are up-to-date (run go run scripts/cheatsheet/main.go generate)
  • [x] Code has been formatted (see here)
  • [ ] Tests have been added/updated (see here for the integration test guide)
  • [x] Text is internationalised (see here)
  • [x] Docs (specifically docs/Config.md) have been updated if necessary
  • [x] You've read through your own file changes for silly mistakes etc

vlad-psh avatar Jul 10 '23 12:07 vlad-psh

We have had alternate mappings for confirming (and for cancelling, I think) but it was decided that why would we complicate things, who would ever need 3 ways to confirm something so it was removed. I'm not sure the decision will be reversed.

mark2185 avatar Jul 10 '23 12:07 mark2185

It's a good point about it being more convenient to reach for a closer letter key than reaching for enter/escape when you're not in a prompt. And I like this being opt-in so that it doesn't cost us any realestate by default. That was my reason for wanting to remove the alternatives in the first place.

Something worth checking is what happens when the confirmation key matches the key on a menu item? We'd want the confirmation key to take precedence.

jesseduffield avatar Jul 10 '23 12:07 jesseduffield

Thanks for response!

We have had alternate mappings for confirming (and for cancelling, I think) but it was decided that why would we complicate things, who would ever need 3 ways to confirm something so https://github.com/jesseduffield/lazygit/pull/2495.

Oh, I see, so this was a supported feature until recently 🤔 Though my PR is a little bit different because it has much narrower scope, I got your point.

As for 'who would ever need', well... I don't know, I still keep pushing all kinds of keys (y/n/x/q) to confirm/cancel actions in lazygit 😅 I guess that's my muscle memory :)

Previous implementation was much more widely used throughout the app, so I totally understand that it may be causing conflicts in some places.

What I found a little bit confusing is that though confirm and return are configurable, doing that would cause a lot of problems so it is better not to touch this configuration at all, which kinda eliminates its purpose 🤔

Something worth checking is what happens when the confirmation key matches the key on a menu item? We'd want the confirmation key to take precedence.

🙏

As far as I can see, confirmations are creating new contexts, so it doesn't overlap with any other hotkeys. For example even global "exit app" hotkey doesn't work when "confirmation window" is opened. So I suppose everything should be fine 🤔

PS: Maybe these hotkeys are TOO specific, but to be honest lazygit just got a lot more intuitive for me when I changed this :)

vlad-psh avatar Jul 10 '23 13:07 vlad-psh

As far as I can see, confirmations are creating new contexts, so it doesn't overlap with any other hotkeys. For example even global "exit app" hotkey doesn't work when "confirmation window" is opened. So I suppose everything should be fine 🤔

But what about if you're in a menu? E.g. if confirm-alt is 'y' and you go to the commits view and press '?' then 'y' does it invoke the first menu item or the yank menu item (which has 'y' as a keybinding)?

jesseduffield avatar Jul 10 '23 23:07 jesseduffield

But what about if you're in a menu? E.g. if confirm-alt is 'y' and you go to the commits view and press '?' then 'y' does it invoke the first menu item or the yank menu item (which has 'y' as a keybinding)?

In this PR I only added these alternative hotkeys to the y/n-type confirmation windows. So this change doesn't affect "help window" in any way. That means, that pressing "y" there (and everywhere else, excluding y/n-type confirmation windows) would indeed execute "yank" action, as expected.

I also think maybe it would be better to change the names of keybindings from confirm-alt/return-alt to the confirm-prompt/cancel-prompt or something like this, to be more precise.

What do you think?

vlad-psh avatar Jul 11 '23 00:07 vlad-psh

I think we should add it to menus as well for the sake of consistency, and we can just grey-out keybindings that match on the key (see pkg/gui/context/menu_context.go:96).

jesseduffield avatar Jul 11 '23 01:07 jesseduffield

I think we should add it to menus as well for the sake of consistency, and we can just grey-out keybindings that match on the key (see pkg/gui/context/menu_context.go:96).

Hmm... I'm actually not sure now :)

If we add "alternatives" also to the menus, it would be not very much different from how it was initially in lazygit (and as @mark2185 mentioned earlier, this was removed with absolutely valid reasons)

I totally get that we need to be consistent, but to be honest, our current return and confirm hotkeys are a little bit "too powerful". To the extent it's almost not recommended to remap them at all (eg: no other keys have sense other than Enter when we're searching/filtering, etc)

That's why I thought it may be useful to add more granular hotkeys to use in some contexts, like prompts (and to call these hotkeys accordingly, not just confirm-alt, but confirm-prompt)

But these additional hotkeys indeed may look TOO specific.

PS: I also think if we'll "strike out" some hotkeys out of the menu, it would make things a little bit less convenient, because in this case we cannot just press, let's say y for "yank", and instead we need to use arrow keys to select needed menu item, and them press y (or Enter) to confirm. Far from ideal :)

vlad-psh avatar Jul 19 '23 13:07 vlad-psh

We have had alternate mappings for confirming (and for cancelling, I think) but it was decided that why would we complicate things, who would ever need 3 ways to confirm something so it was removed. I'm not sure the decision will be reversed.

Is it not possible to add again the alternative return keybinding? 'cos sometimes it is more comfy to use backspace for returning the previous menu than using esc key.

jeeeem avatar Aug 05 '23 16:08 jeeeem

I wanted to remap q from Quit to Return for non prompt options (this is hard-wired in my brain from tig). But with the current available keybindings that's not possible, because if I change the value from Return to q there is no way for me to exit prompts with text fields such as the commit prompt. It seems like #2495 removed the functionality that I'm looking for. That PR seemed to remove it because the default y value had the yank meaning in other contexts. Would you accept a PR that reverts that PR, but doesn't enable the Alt options by default. At least people will be able to configure lazyvim a bit better.

<esc> is litterally one of the most common buttons that you now need to press, and unless you remap your keyboard it's one of the furthest ones away from the homerow. Right now I'm falling back to remapping it to <c-q>

EDIT: I created a PR that does this now in https://github.com/jesseduffield/lazygit/pull/3750

JelteF avatar Jul 15 '24 15:07 JelteF

I am using <right> for goInto and <left> for return, this is the only way I can navigate through commits/stashes and their changes. BUT we've "dialogs" (like asking if you want to download newest version when checking for updates) that I really want to cancel with <ESC> rather than <left>, so I wan't to have BOTH keys for return.

Grueslayer avatar Sep 04 '24 14:09 Grueslayer