Skip to content

Conversation

@stefanhaller
Copy link
Collaborator

PR Description

Remapping keybinding.universal.confirm from <enter> to something like y is currently impossible because the same keybinding is also used to confirm prompts (e.g. "New branch") and the search prompt. Fix this by hard-coding enter for those; it doesn't really make sense to use any other key for prompts.

While at it, add separate bindings for confirmMenu and confirmSuggestion for those who would like to have different keys for these. Of these, confirmMenu could be a little tricky because menus are sometimes used purely as a choice (e.g. in "Amend commit attribute" or the global keybindings menu), in which case you might want to use <enter>, but other times as a substitute for a confirmation (e.g. for "Delete branch"), in which case you might want to remap to y. I don't have a great idea what to do about that, to be honest. Feedback welcome.

In this PR we only take care of Confirm, which many people seem to be concerned about. We might consider doing something similar for Esc, but it seems less urgent, and I'm out of time now. 😄

This seemingly simple change required some serious refactoring under the hood, so thorough testing would be good to ensure we didn't break anything.

Closes #2611
Closes #2767
Closes #3471

Related: #2768

@stefanhaller stefanhaller added the bug Something isn't working label Aug 31, 2025
@codacy-production
Copy link

codacy-production bot commented Aug 31, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for 94aa1101 94.12%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (94aa110) Report Missing Report Missing Report Missing
Head commit (b3a3410) 57738 50192 86.93%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#4860) 204 192 94.12%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@AndrewSav
Copy link
Contributor

You do not emumerate the list of dialogs/prompts it will apply to, so I'm not sure if it will also close #2009. Please advise? Thank you for your work.

@stefanhaller
Copy link
Collaborator Author

You do not emumerate the list of dialogs/prompts it will apply to, so I'm not sure if it will also close #2009. Please advise? Thank you for your work.

It applies to all popups that show a message (not editable, not a menu); that's why the keybinding is called universal.confirm. If you change it, it will not only affect the force-push prompt, but all other confirmations, and even popups that are purely informational and don't offer a choice (e.g. bisect finished).

So far, confirmations and prompts were handled by the same view, context, and
controller, with a bunch of conditional code based on whether the view is
editable. This was more or less ok so far, since it does save a little bit of
code duplication; however, now we need separate views, because we don't have
dynamic keybindings, but we want to map "confirm" to different keys in
confirmations (the "universal.confirm" user config) and prompts (hard-coded to
enter, because it doesn't make sense to customize it there).

It also allows us to get rid of the conditional code, which is a nice benefit;
and the code duplication is actually not *that* bad.
Done in a separate commit because the diff is already so long.
Rebinding the universal.confirm keybinding currently doesn't make sense, because
the rebound key would also be used for editable prompts, which means you would
only be able to bind it to a ctrl key (not "y", which is desirable for some
people), and also it would allow you to enter a line feed in a branch name.

Fix this by always using enter for editable prompts.
Like with the previous commit, it doesn't make sense to use any other key than
enter for the search prompt.
The universal.confirm keybinding is the wrong one to use for this, we want
universal.goInto instead. They are both bound to "enter" by default, but when
remapping confirm to "y" we don't want to use that for entering worktrees.
This one doesn't make a difference in practice because we don't remap the key in
tests, but if we would, then this would no longer work correctly. It's just more
correct this way.
It seems useful to have the flexibility to remap "enter" in confirmations to
"y", but keep "enter" for menus and suggestions (even though we sometimes use
menus as confirmations, but it's still good to give users the choice).
…rm key

This is needed when remapping the confirmMenu key to, say, "y", and there's a
menu that has an item with a "y" binding. This already worked correctly (confirm
takes precedence, as desired), but it's still confusing to see the item binding.
@stefanhaller stefanhaller force-pushed the improve-confirm-keybinding branch from 987d41d to b3a3410 Compare September 5, 2025 08:42
@stefanhaller stefanhaller merged commit c0805f1 into master Sep 5, 2025
12 checks passed
@stefanhaller stefanhaller deleted the improve-confirm-keybinding branch September 5, 2025 08:46
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 9, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.54.2` -> `v0.55.0` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>jesseduffield/lazygit (jesseduffield/lazygit)</summary>

### [`v0.55.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.55.0)

[Compare Source](jesseduffield/lazygit@v0.54.2...v0.55.0)

<!-- Release notes generated using configuration in .github/release.yml at v0.55.0 -->

#### What's Changed

##### Enhancements 🔥

- Allow filtering the keybindings menu by keybinding by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4821](jesseduffield/lazygit#4821)
- Add support for suspending LazyGit with Ctrl+Z on Unix systems by [@&#8203;cowboy8625](https://github.com/cowboy8625) in [#&#8203;4757](jesseduffield/lazygit#4757)
- Add "CopyToClipboard" command to `ConfirmationController` by [@&#8203;kyu08](https://github.com/kyu08) in [#&#8203;4810](jesseduffield/lazygit#4810)
- Add a user config for using git's external diff command for paging by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4832](jesseduffield/lazygit#4832)
- Log the hash of dropped stashes by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4850](jesseduffield/lazygit#4850)

##### Fixes 🔧

- Fix right-alignment of divergence from base branch for branch checked out in a worktree by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4824](jesseduffield/lazygit#4824)
- Support Azure DevOps vs-ssh.visualstudio.com SSH remotes as hosting provider by [@&#8203;Kahitar](https://github.com/Kahitar) in [#&#8203;4822](jesseduffield/lazygit#4822)
- Improve display of "esc" keybinding in the keybindings status bar by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4819](jesseduffield/lazygit#4819)
- Use external diff command in stashes panel by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4836](jesseduffield/lazygit#4836)
- Remove the git.paging.useConfig option by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4837](jesseduffield/lazygit#4837)
- Don't auto-forward branches that are checked out in another worktree by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4833](jesseduffield/lazygit#4833)
- Fix dropping range selection of filtered stashes by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4849](jesseduffield/lazygit#4849)
- Fix rare crash in interactive rebase (merge command without comment) by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4872](jesseduffield/lazygit#4872)
- Make it possible to rebind the Confirm keybinding by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4860](jesseduffield/lazygit#4860)

##### Maintenance ⚙️

- Pass only Git-tracked Go files to gofumpt by [@&#8203;kyu08](https://github.com/kyu08) in [#&#8203;4809](jesseduffield/lazygit#4809)
- Update donation wording so that it's clear there's no strings attached by [@&#8203;jesseduffield](https://github.com/jesseduffield) in [#&#8203;4827](jesseduffield/lazygit#4827)
- Enhance MR/Issue templates readability by [@&#8203;kyu08](https://github.com/kyu08) in [#&#8203;4829](jesseduffield/lazygit#4829)
- Run label check workflow only on label events and open pr event by [@&#8203;kyu08](https://github.com/kyu08) in [#&#8203;4830](jesseduffield/lazygit#4830)

##### Docs 📖

- Add installation with gah by [@&#8203;marverix](https://github.com/marverix) in [#&#8203;4820](jesseduffield/lazygit#4820)
- docs(VISION): fix "Dicoverability" typo by [@&#8203;Rudxain](https://github.com/Rudxain) in [#&#8203;4866](jesseduffield/lazygit#4866)
- Add dev container feature as installation method to README by [@&#8203;HenningLorenzen-ext-bayer](https://github.com/HenningLorenzen-ext-bayer) in [#&#8203;4876](jesseduffield/lazygit#4876)

##### I18n 🌎

- Update translations from Crowdin by [@&#8203;stefanhaller](https://github.com/stefanhaller) in [#&#8203;4873](jesseduffield/lazygit#4873)

#### New Contributors

- [@&#8203;marverix](https://github.com/marverix) made their first contribution in [#&#8203;4820](jesseduffield/lazygit#4820)
- [@&#8203;Kahitar](https://github.com/Kahitar) made their first contribution in [#&#8203;4822](jesseduffield/lazygit#4822)
- [@&#8203;cowboy8625](https://github.com/cowboy8625) made their first contribution in [#&#8203;4757](jesseduffield/lazygit#4757)
- [@&#8203;Rudxain](https://github.com/Rudxain) made their first contribution in [#&#8203;4866](jesseduffield/lazygit#4866)
- [@&#8203;HenningLorenzen-ext-bayer](https://github.com/HenningLorenzen-ext-bayer) made their first contribution in [#&#8203;4876](jesseduffield/lazygit#4876)

**Full Changelog**: <jesseduffield/lazygit@v0.54.2...v0.55.0>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS45Ny40IiwidXBkYXRlZEluVmVyIjoiNDEuOTcuNCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
karolzwolak pushed a commit to karolzwolak/lazygit that referenced this pull request Sep 27, 2025
### PR Description

Remapping `keybinding.universal.confirm` from `<enter>` to something
like `y` is currently impossible because the same keybinding is also
used to confirm prompts (e.g. "New branch") and the search prompt. Fix
this by hard-coding enter for those; it doesn't really make sense to use
any other key for prompts.

While at it, add separate bindings for `confirmMenu` and
`confirmSuggestion` for those who would like to have different keys for
these. Of these, `confirmMenu` could be a little tricky because menus
are sometimes used purely as a choice (e.g. in "Amend commit attribute"
or the global keybindings menu), in which case you might want to use
`<enter>`, but other times as a substitute for a confirmation (e.g. for
"Delete branch"), in which case you might want to remap to `y`. I don't
have a great idea what to do about that, to be honest. Feedback welcome.

In this PR we only take care of Confirm, which many people seem to be
concerned about. We might consider doing something similar for Esc, but
it seems less urgent, and I'm out of time now. 😄

This seemingly simple change required some serious refactoring under the
hood, so thorough testing would be good to ensure we didn't break
anything.

Closes jesseduffield#2611
Closes jesseduffield#2767
Closes jesseduffield#3471

Related: jesseduffield#2768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Risky enter confirmation Remapped 'confirm' hotkey doesn't go well with the search prompt Bring back y to confirm dialogs

3 participants