From b6063cff5b4dad85223af69675d1c095b53dbce2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 16:33:13 +0200 Subject: [PATCH] Restore commit selection even when the commit's TODO status changed When restoring the commit selection after a refresh we match by hash and TODO status. The TODO status is part of the match so that a commit being reverted or cherry-picked is matched to the real commit rather than to the rebase TODO entry that shares its hash. But a selected commit can also change its TODO status across a refresh: when starting an interactive rebase that stops to edit it, the real commit becomes a TODO entry. Fall back to matching by hash alone when there is no exact match, so the selection is still restored in that case. The next commit relies on this to remove bespoke selection-restoration code in the local commits controller that matched by hash alone, which the generic mechanism otherwise wouldn't fully replace. --- pkg/gui/controllers/helpers/refresh_helper.go | 34 +++++++++++++++---- .../helpers/refresh_helper_test.go | 13 +++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 3ea742c8e..605309f9f 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -533,12 +533,10 @@ func findLocalCommitSelectionRange( commits []*models.Commit, selectionRange *localCommitSelectionRange, ) (int, int, bool, bool) { - _, selectedIdx, foundSelected := lo.FindIndexOf(commits, func(commit *models.Commit) bool { - return commit.Hash() == selectionRange.selectedHash && commit.IsTODO() == selectionRange.selectedIsTODO - }) - _, rangeStartIdx, foundRangeStart := lo.FindIndexOf(commits, func(commit *models.Commit) bool { - return commit.Hash() == selectionRange.rangeStartHash && commit.IsTODO() == selectionRange.rangeStartIsTODO - }) + selectedIdx, foundSelected := findCommitByHashPreferringTODOStatus( + commits, selectionRange.selectedHash, selectionRange.selectedIsTODO) + rangeStartIdx, foundRangeStart := findCommitByHashPreferringTODOStatus( + commits, selectionRange.rangeStartHash, selectionRange.rangeStartIsTODO) if !foundSelected || !foundRangeStart { return 0, 0, false, false } @@ -547,6 +545,30 @@ func findLocalCommitSelectionRange( return selectedIdx, rangeStartIdx, didMove, true } +// findCommitByHashPreferringTODOStatus finds the commit with the given hash. +// When both a TODO and a non-TODO commit share that hash - which happens while +// reverting or cherry-picking, where the rebase TODO entry has the same hash as +// the real commit - it returns the one whose TODO status matches isTODO. When +// only one commit has the hash, it is returned regardless of its TODO status, +// so that a selected commit which turned into a TODO entry across the refresh is +// still found (e.g. when starting an interactive rebase that stops to edit it). +func findCommitByHashPreferringTODOStatus(commits []*models.Commit, hash string, isTODO bool) (int, bool) { + fallbackIdx := -1 + for idx, commit := range commits { + if commit.Hash() != hash { + continue + } + if commit.IsTODO() == isTODO { + return idx, true + } + if fallbackIdx == -1 { + fallbackIdx = idx + } + } + + return fallbackIdx, fallbackIdx != -1 +} + func hasRestorableCommitHash(commits []*models.Commit, idx int) bool { return idx >= 0 && idx < len(commits) && commits[idx].Hash() != "" } diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index e8be06d61..3a5f6ea82 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -130,6 +130,19 @@ func TestFindLocalCommitSelectionRange(t *testing.T) { found: true, }, }, + { + name: "falls back to a todo entry when the selected commit became one", + commits: []*models.Commit{ + makeTodoCommitWithHash("b", todo.Pick), + makeCommits("c")[0], + }, + expected: expectation{ + selectedIdx: 0, + rangeStartIdx: 1, + moved: true, + found: true, + }, + }, } for _, testCase := range testCases {