Skip to content

Conversation

@stefanhaller
Copy link
Collaborator

  • PR Description

Improve the dispatching of key bindings so that remapping "pullFiles" to a different key works correctly in the Commits panel.

Fixes #4614.

  • 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

@codacy-production
Copy link

codacy-production bot commented Jun 1, 2025

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for ac0c3db1 44.12%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ac0c3db) Report Missing Report Missing Report Missing
Head commit (c752f35) 56637 49192 86.85%

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 (#4617) 34 15 44.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.

Adaptions are for this gocui commit:

Cleanup: remove Is* error functions

- Use errors.Is instead of quality comparisons. This is better because it
  matches wrapped errors as well, which we will need later in this branch.
- Inline the errors.Is calls at the call sites. This is idiomatic go, we don't
  need helper functions for this.

See https://go.dev/blog/go1.13-errors for more about this.
There was no reason to declare a variable for disabledReason, assign it inside
the "if binding.GetDisabledReason != nil" statement, and then check its value
again after that if statement. Move all that code inside the first if statement
to make the control flow easier to understand.
If a DisabledReason has its AllowFurtherDispatching flag set, it is returned as
a ErrKeybindingNotHandled error, instead of shown as a toast right away. This
allows gocui to continue to dispatch the keybinding, and we can unwrap the error
at the other end (in our global ErrorHandler) and display it then.

This allows having keybindings for the same key at the local and global levels,
and they will continue to be dispatched even if the first one returns a
DisabledReason. It is opt-in, so we only use it for cases where we know that a
local and a global handler share the same (default) keybinding.
Previously we would call pullFiles() from the pick() handler if we were not in a
rebase, assuming that the default keybinding for both is "p". This needn't be
the case of course, if the user has remapped one or the other.

The consequence of this was that swapping the keybindings for "pullFiles" and
"pushFiles" would work in all panels except the Commits panel (unless "pick" was
also remapped in the same way).

Fix this by using the new AllowFurtherDispatching mechanism of DisabledReasons
to pass the keybinding on to the next handler.
@stefanhaller stefanhaller force-pushed the fix-pull-vs-pick-keybinding branch from 4251ef6 to c752f35 Compare June 5, 2025 11:20
@stefanhaller stefanhaller enabled auto-merge June 5, 2025 11:21
@stefanhaller stefanhaller merged commit aa331e5 into master Jun 5, 2025
14 checks passed
@stefanhaller stefanhaller deleted the fix-pull-vs-pick-keybinding branch June 5, 2025 11:24
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jun 12, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [jesseduffield/lazygit](https://github.com/jesseduffield/lazygit) | minor | `v0.51.1` -> `v0.52.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.52.0`](https://github.com/jesseduffield/lazygit/releases/tag/v0.52.0)

[Compare Source](jesseduffield/lazygit@v0.51.1...v0.52.0)

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

#### What's Changed

##### Enhancements 🔥

-   Add user config for hiding the root item in the file tree by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4593
-   Use branchPrefix when moving commits to new branch by [@&#8203;EliasA5](https://github.com/EliasA5) in jesseduffield/lazygit#4604
-   Show default option when prompting to create a new git repo by [@&#8203;Joshuahuahua](https://github.com/Joshuahuahua) in jesseduffield/lazygit#4596

##### Fixes 🔧

-   Fix selecting large hunks, and fix problems editing very long commit descriptions by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4589
-   Kill background fetch when it requests a passphrase by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4588
-   Fix branch selection jumping back on background fetch by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4597
-   Fix resetting to a branch when a tag shares the same name, or vice versa by [@&#8203;ChrisMcD1](https://github.com/ChrisMcD1) in jesseduffield/lazygit#4571
-   Fix wrong inactive highlight when switching between repos by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4621
-   Fix assigning custom key to pullFiles command in the Commits panel by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4617

##### Maintenance ⚙️

-   Replace literal with ConfigFilename constant by [@&#8203;mloskot](https://github.com/mloskot) in jesseduffield/lazygit#4613
-   Improve MR template wrt release notes by [@&#8203;stefanhaller](https://github.com/stefanhaller) in jesseduffield/lazygit#4607

#### New Contributors

-   [@&#8203;mloskot](https://github.com/mloskot) made their first contribution in jesseduffield/lazygit#4613
-   [@&#8203;Joshuahuahua](https://github.com/Joshuahuahua) made their first contribution in jesseduffield/lazygit#4596

**Full Changelog**: jesseduffield/lazygit@v0.51.1...v0.52.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:eyJjcmVhdGVkSW5WZXIiOiI0MC41MC4wIiwidXBkYXRlZEluVmVyIjoiNDAuNTAuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
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.

Custom binding for key p is ignored when a commit has focus (behaviour reverts to pull)

3 participants