From 02be5e74edcb5eeffcc13fd5d41e634603043c7c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 08:52:36 +0200 Subject: [PATCH 1/9] Tighten a test expectation This guards against regressions from the changes that follow. We're about to add a mechanism that keeps the selection anchored by commit hash, but we need to make sure that it doesn't take effect here; after a merge we want to select the newly added merge commit. In the current state of the code this happens to work because we keep the selection index the same, which happened to be 0 here; later we will change this to explicitly select the head commit after the merge. --- pkg/integration/tests/sync/pull_merge.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/integration/tests/sync/pull_merge.go b/pkg/integration/tests/sync/pull_merge.go index 39e447ebc..295923b56 100644 --- a/pkg/integration/tests/sync/pull_merge.go +++ b/pkg/integration/tests/sync/pull_merge.go @@ -29,7 +29,7 @@ var PullMerge = NewIntegrationTest(NewIntegrationTestArgs{ Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Commits(). Lines( - Contains("four"), + Contains("four").IsSelected(), Contains("one"), ) @@ -43,7 +43,7 @@ var PullMerge = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Lines( - Contains("Merge branch 'master' of ../origin"), + Contains("Merge branch 'master' of ../origin").IsSelected(), Contains("three"), Contains("two"), Contains("four"), From d673a0f3b90b8dfcaa3606e236c0cbec370bf331 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 09:02:11 +0200 Subject: [PATCH 2/9] Add tests for IsHeadCommit --- pkg/commands/models/commit_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 pkg/commands/models/commit_test.go diff --git a/pkg/commands/models/commit_test.go b/pkg/commands/models/commit_test.go new file mode 100644 index 000000000..d24238023 --- /dev/null +++ b/pkg/commands/models/commit_test.go @@ -0,0 +1,29 @@ +package models + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/stefanhaller/git-todo-parser/todo" + "github.com/stretchr/testify/assert" +) + +func TestIsHeadCommit(t *testing.T) { + commits := []*Commit{ + makeTestTodoCommit(todo.Pick), + makeTestCommit("a"), + makeTestCommit("b"), + } + + assert.False(t, IsHeadCommit(commits, 0)) + assert.True(t, IsHeadCommit(commits, 1)) + assert.False(t, IsHeadCommit(commits, 2)) +} + +func makeTestCommit(hash string) *Commit { + return NewCommit(&utils.StringPool{}, NewCommitOpts{Hash: hash}) +} + +func makeTestTodoCommit(action todo.TodoCommand) *Commit { + return NewCommit(&utils.StringPool{}, NewCommitOpts{Action: action}) +} From cfb46f440cdb76a6a32391f87ae6638aa1b4b9ba Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 09:15:32 +0200 Subject: [PATCH 3/9] Add HeadCommitIdx helper function Not used yet, we'll need it in the next commit. --- pkg/commands/models/commit.go | 10 ++++++ pkg/commands/models/commit_test.go | 52 ++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index 137528ee6..69aca8d73 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -160,3 +160,13 @@ func (c *Commit) IsTODO() bool { func IsHeadCommit(commits []*Commit, index int) bool { return !commits[index].IsTODO() && (index == 0 || commits[index-1].IsTODO()) } + +func HeadCommitIdx(commits []*Commit) int { + for index, commit := range commits { + if !commit.IsTODO() { + return index + } + } + + return -1 +} diff --git a/pkg/commands/models/commit_test.go b/pkg/commands/models/commit_test.go index d24238023..ecddec9c5 100644 --- a/pkg/commands/models/commit_test.go +++ b/pkg/commands/models/commit_test.go @@ -8,6 +8,49 @@ import ( "github.com/stretchr/testify/assert" ) +func TestHeadCommitIdx(t *testing.T) { + testCases := []struct { + name string + commits []*Commit + expected int + }{ + { + name: "first commit without rebase todos", + commits: makeTestCommits("a", "b"), + expected: 0, + }, + { + name: "first non-todo commit during an interactive rebase", + commits: []*Commit{ + makeTestTodoCommit(todo.Pick), + makeTestTodoCommit(todo.Reword), + makeTestCommit("a"), + makeTestCommit("b"), + }, + expected: 2, + }, + { + name: "no commits", + commits: nil, + expected: -1, + }, + { + name: "only rebase todos", + commits: []*Commit{ + makeTestTodoCommit(todo.Pick), + makeTestTodoCommit(todo.Reword), + }, + expected: -1, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + assert.Equal(t, testCase.expected, HeadCommitIdx(testCase.commits)) + }) + } +} + func TestIsHeadCommit(t *testing.T) { commits := []*Commit{ makeTestTodoCommit(todo.Pick), @@ -20,6 +63,15 @@ func TestIsHeadCommit(t *testing.T) { assert.False(t, IsHeadCommit(commits, 2)) } +func makeTestCommits(hashes ...string) []*Commit { + commits := make([]*Commit, 0, len(hashes)) + for _, hash := range hashes { + commits = append(commits, makeTestCommit(hash)) + } + + return commits +} + func makeTestCommit(hash string) *Commit { return NewCommit(&utils.StringPool{}, NewCommitOpts{Hash: hash}) } From 3f8dc527b5778c15867992cce2b919ac8037d8ee Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 23 Jun 2026 08:42:51 +0200 Subject: [PATCH 4/9] Cleanup: remove unnecessary `if` statement --- pkg/gui/controllers/helpers/merge_and_rebase_helper.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index cd141c697..ab7d11c29 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -111,10 +111,7 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { ) } result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) - if err := self.CheckMergeOrRebase(result); err != nil { - return err - } - return nil + return self.CheckMergeOrRebase(result) } func (self *MergeAndRebaseHelper) hasExecTodos() bool { From 5d5aa0a865a75a4b71a5125dc7c23dacb8a34db7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 23 Jun 2026 08:30:47 +0200 Subject: [PATCH 5/9] Cleanup: wrap long parameter lists This makes the following diff a little easier to read. --- pkg/gui/controllers/helpers/gpg_helper.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index 30ec6ceef..e8e46e403 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -23,7 +23,13 @@ func NewGpgHelper(c *HelperCommon) *GpgHelper { // WithWaitingStatus we get stuck there and can't return to lazygit. We could // fix this bug, or just stop running subprocesses from within there, given that // we don't need to see a loading status if we're in a subprocess. -func (self *GpgHelper) WithGpgHandling(cmdObj *oscommands.CmdObj, configKey git_commands.GpgConfigKey, waitingStatus string, onSuccess func() error, refreshScope []types.RefreshableView) error { +func (self *GpgHelper) WithGpgHandling( + cmdObj *oscommands.CmdObj, + configKey git_commands.GpgConfigKey, + waitingStatus string, + onSuccess func() error, + refreshScope []types.RefreshableView, +) error { useSubprocess := self.c.Git().Config.NeedsGpgSubprocess(configKey) if useSubprocess { success, err := self.c.RunSubprocess(cmdObj) @@ -40,7 +46,12 @@ func (self *GpgHelper) WithGpgHandling(cmdObj *oscommands.CmdObj, configKey git_ return self.runAndStream(cmdObj, waitingStatus, onSuccess, refreshScope) } -func (self *GpgHelper) runAndStream(cmdObj *oscommands.CmdObj, waitingStatus string, onSuccess func() error, refreshScope []types.RefreshableView) error { +func (self *GpgHelper) runAndStream( + cmdObj *oscommands.CmdObj, + waitingStatus string, + onSuccess func() error, + refreshScope []types.RefreshableView, +) error { return self.c.WithWaitingStatus(waitingStatus, func(gocui.Task) error { if err := cmdObj.StreamOutput().Run(); err != nil { self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope}) From 10d2f9f7156fd0343a2b78b358591cc0a49025ff Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 09:22:18 +0200 Subject: [PATCH 6/9] Allow GpgHelper to refresh differently on success and failure Preparation for the next commit, which selects the newly created commit after a commit succeeds, while leaving the selection alone on failure. For now success and failure use the same refresh options, so behavior is unchanged. --- pkg/gui/controllers/helpers/gpg_helper.go | 37 +++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index e8e46e403..46fbee703 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -19,16 +19,29 @@ func NewGpgHelper(c *HelperCommon) *GpgHelper { } } -// Currently there is a bug where if we switch to a subprocess from within -// WithWaitingStatus we get stuck there and can't return to lazygit. We could -// fix this bug, or just stop running subprocesses from within there, given that -// we don't need to see a loading status if we're in a subprocess. func (self *GpgHelper) WithGpgHandling( cmdObj *oscommands.CmdObj, configKey git_commands.GpgConfigKey, waitingStatus string, onSuccess func() error, refreshScope []types.RefreshableView, +) error { + refreshOptions := types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope} + return self.withGpgHandling( + cmdObj, configKey, waitingStatus, onSuccess, refreshOptions, refreshOptions) +} + +// Currently there is a bug where if we switch to a subprocess from within +// WithWaitingStatus we get stuck there and can't return to lazygit. We could +// fix this bug, or just stop running subprocesses from within there, given that +// we don't need to see a loading status if we're in a subprocess. +func (self *GpgHelper) withGpgHandling( + cmdObj *oscommands.CmdObj, + configKey git_commands.GpgConfigKey, + waitingStatus string, + onSuccess func() error, + failureRefreshOptions types.RefreshOptions, + successRefreshOptions types.RefreshOptions, ) error { useSubprocess := self.c.Git().Config.NeedsGpgSubprocess(configKey) if useSubprocess { @@ -38,23 +51,29 @@ func (self *GpgHelper) WithGpgHandling( return err } } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope}) + if success { + self.c.Refresh(successRefreshOptions) + } else { + self.c.Refresh(failureRefreshOptions) + } return err } - return self.runAndStream(cmdObj, waitingStatus, onSuccess, refreshScope) + return self.runAndStream( + cmdObj, waitingStatus, onSuccess, failureRefreshOptions, successRefreshOptions) } func (self *GpgHelper) runAndStream( cmdObj *oscommands.CmdObj, waitingStatus string, onSuccess func() error, - refreshScope []types.RefreshableView, + failureRefreshOptions types.RefreshOptions, + successRefreshOptions types.RefreshOptions, ) error { return self.c.WithWaitingStatus(waitingStatus, func(gocui.Task) error { if err := cmdObj.StreamOutput().Run(); err != nil { - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope}) + self.c.Refresh(failureRefreshOptions) return fmt.Errorf( self.c.Tr.GitCommandFailed, self.c.UserConfig().Keybinding.Universal.ExtrasMenu, ) @@ -66,7 +85,7 @@ func (self *GpgHelper) runAndStream( } } - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope}) + self.c.Refresh(successRefreshOptions) return nil }) } From c15ab5db5daf352eec3534c69a4e7c53b591da17 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 21 Jun 2026 11:59:13 +0200 Subject: [PATCH 7/9] Keep selected commits stable across refreshes With the recently added external change detection, it happens more often now that we refresh the commits list because an agent made a commit in the background. In this case, if we keep the selection index the same, it now points at a different commit, making the main view show a different commit too, which is confusing and annoying. To fix this, track the selected commit and range anchor by hash before reloading, then restore those rows if both hashes still exist. This also allows us to get rid of some bespoke code that did this for the specific cases of reverting a commit or cherry-picking commits, because those are now handled by the generic mechanism. --- pkg/gui/controllers/branches_controller.go | 6 +- .../controllers/helpers/cherry_pick_helper.go | 10 -- pkg/gui/controllers/helpers/gpg_helper.go | 15 ++ .../helpers/merge_and_rebase_helper.go | 38 ++++- pkg/gui/controllers/helpers/refresh_helper.go | 95 ++++++++++- .../helpers/refresh_helper_test.go | 153 ++++++++++++++++++ pkg/gui/controllers/helpers/refs_helper.go | 27 +++- .../helpers/working_tree_helper.go | 4 +- .../controllers/local_commits_controller.go | 16 +- pkg/gui/controllers/remotes_controller.go | 1 + pkg/gui/controllers/sync_controller.go | 2 +- pkg/gui/types/refresh.go | 26 +++ .../cherry_pick_commit_that_becomes_empty.go | 20 +-- .../cherry_pick/cherry_pick_conflicts.go | 6 +- ..._conflicts_empty_commit_after_resolving.go | 19 +-- ...p_selected_commit_after_external_commit.go | 46 ++++++ pkg/integration/tests/test_list.go | 1 + 17 files changed, 412 insertions(+), 73 deletions(-) create mode 100644 pkg/integration/tests/commit/keep_selected_commit_after_external_commit.go diff --git a/pkg/gui/controllers/branches_controller.go b/pkg/gui/controllers/branches_controller.go index 24ef84d54..c2c0595c3 100644 --- a/pkg/gui/controllers/branches_controller.go +++ b/pkg/gui/controllers/branches_controller.go @@ -594,7 +594,11 @@ func (self *BranchesController) createNewBranchWithName(newBranchName string) er } self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, KeepBranchSelectionIndex: true}) + self.c.Refresh(types.RefreshOptions{ + Mode: types.ASYNC, + KeepBranchSelectionIndex: true, + CommitSelection: types.KeepCommitSelectionIndex, + }) return nil } diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 079fdedcf..e2fe46545 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -100,16 +100,6 @@ func (self *CherryPickHelper) Paste() error { return result } - // Move the selection down by the number of commits we just - // cherry-picked, to keep the same commit selected as before. - // Don't do this if a rebase todo is selected, because in this - // case we are in a rebase and the cherry-picked commits end up - // below the selection. - if commit := self.c.Contexts().LocalCommits.GetSelected(); commit != nil && !commit.IsTODO() { - self.c.Contexts().LocalCommits.MoveSelection(len(cherryPickedCommits)) - self.c.Contexts().LocalCommits.FocusLine(true) - } - // If we're in the cherry-picking state at this point, it must // be because there were conflicts. Don't clear the copied // commits in this case, since we might want to abort and try diff --git a/pkg/gui/controllers/helpers/gpg_helper.go b/pkg/gui/controllers/helpers/gpg_helper.go index 46fbee703..fb8fae628 100644 --- a/pkg/gui/controllers/helpers/gpg_helper.go +++ b/pkg/gui/controllers/helpers/gpg_helper.go @@ -31,6 +31,21 @@ func (self *GpgHelper) WithGpgHandling( cmdObj, configKey, waitingStatus, onSuccess, refreshOptions, refreshOptions) } +// WithGpgHandlingAndSelectHeadCommit is like WithGpgHandling, but on success it +// selects the new HEAD commit rather than restoring the previous selection. For +// committing, where the commit we just created is the one we want selected. +func (self *GpgHelper) WithGpgHandlingAndSelectHeadCommit( + cmdObj *oscommands.CmdObj, + configKey git_commands.GpgConfigKey, + waitingStatus string, + onSuccess func() error, +) error { + failureRefreshOptions := types.RefreshOptions{Mode: types.ASYNC} + successRefreshOptions := types.RefreshOptions{Mode: types.ASYNC, CommitSelection: types.SelectHeadCommit} + return self.withGpgHandling( + cmdObj, configKey, waitingStatus, onSuccess, failureRefreshOptions, successRefreshOptions) +} + // Currently there is a bug where if we switch to a subprocess from within // WithWaitingStatus we get stuck there and can't return to lazygit. We could // fix this bug, or just stop running subprocesses from within there, given that diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index ab7d11c29..536c254dd 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -95,6 +95,8 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { } commandType := status.CommandName() + selectHeadCommitOnSuccess := command == REBASE_OPTION_CONTINUE && + effectiveStatus == models.WORKING_TREE_STATE_MERGING // we should end up with a command like 'git merge --continue' @@ -106,12 +108,29 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction - return self.c.RunSubprocessAndRefresh( - self.c.Git().Rebase.GenericMergeOrRebaseActionCmdObj(commandType, command), - ) + success, err := self.c.RunSubprocess(self.c.Git().Rebase.GenericMergeOrRebaseActionCmdObj(commandType, command)) + self.c.Refresh(types.RefreshOptions{ + Mode: types.ASYNC, + CommitSelection: commitSelectionAfterMerge(success && selectHeadCommitOnSuccess), + }) + return err } result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) - return self.CheckMergeOrRebase(result) + return self.CheckMergeOrRebaseWithRefreshOptions(result, + types.RefreshOptions{ + Mode: types.ASYNC, + CommitSelection: commitSelectionAfterMerge(result == nil && selectHeadCommitOnSuccess), + }) +} + +// commitSelectionAfterMerge maps whether a merge/rebase/pull created a new +// commit at HEAD to the corresponding commit-selection behavior: select that +// new commit, or otherwise keep the previous selection by hash. +func commitSelectionAfterMerge(createdNewCommit bool) types.CommitSelectionBehavior { + if createdNewCommit { + return types.SelectHeadCommit + } + return types.KeepCommitSelectionByHash } func (self *MergeAndRebaseHelper) hasExecTodos() bool { @@ -166,6 +185,15 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error { return self.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.ASYNC}) } +// Like CheckMergeOrRebase, but for operations that create a new commit at HEAD +// (a merge, or a pull that merges): on success it selects that new commit, +// which the keep-selection-by-hash logic can't do since the commit didn't exist +// before the refresh. +func (self *MergeAndRebaseHelper) CheckMergeOrRebaseAndSelectHeadCommit(result error) error { + return self.CheckMergeOrRebaseWithRefreshOptions(result, + types.RefreshOptions{Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(result == nil)}) +} + func (self *MergeAndRebaseHelper) CheckForConflicts(result error) error { if result == nil { return nil @@ -489,7 +517,7 @@ func (self *MergeAndRebaseHelper) RegularMerge(refName string, variant git_comma return func() error { self.c.LogAction(self.c.Tr.Actions.Merge) err := self.c.Git().Branch.Merge(refName, variant) - return self.CheckMergeOrRebase(err) + return self.CheckMergeOrRebaseAndSelectHeadCommit(err) } } diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 31035d104..3ea742c8e 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -12,6 +12,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" + "github.com/jesseduffield/lazygit/pkg/gui/context/traits" "github.com/jesseduffield/lazygit/pkg/gui/filetree" "github.com/jesseduffield/lazygit/pkg/gui/mergeconflicts" "github.com/jesseduffield/lazygit/pkg/gui/presentation" @@ -164,7 +165,9 @@ func (self *RefreshHelper) Refresh(options types.RefreshOptions) { // whenever we change commits, we should update branches because the upstream/downstream // counts can change. Whenever we change branches we should also change commits // e.g. in the case of switching branches. - refresh("commits and commit files", self.refreshCommitsAndCommitFiles) + refresh("commits and commit files", func() { + self.refreshCommitsAndCommitFiles(options.CommitSelection) + }) includeWorktreesWithBranches = scopeSet.Includes(types.WORKTREES) if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { @@ -385,8 +388,8 @@ func (self *RefreshHelper) refreshReflogAndBranches(refreshWorktrees bool, keepB self.refreshBranches(refreshWorktrees, keepBranchSelectionIndex, loadBehindCounts) } -func (self *RefreshHelper) refreshCommitsAndCommitFiles() { - _ = self.refreshCommitsWithLimit() +func (self *RefreshHelper) refreshCommitsAndCommitFiles(commitSelection types.CommitSelectionBehavior) { + _ = self.refreshCommitsWithLimit(commitSelection) ctx := self.c.Contexts().CommitFiles.GetParentContext() if ctx != nil && ctx.GetKey() == context.LOCAL_COMMITS_CONTEXT_KEY { // This makes sense when we've e.g. just amended a commit, meaning we get a new commit hash at the same position. @@ -430,10 +433,16 @@ func (self *RefreshHelper) determineCheckedOutRef() models.Ref { return nil } -func (self *RefreshHelper) refreshCommitsWithLimit() error { +func (self *RefreshHelper) refreshCommitsWithLimit(commitSelection types.CommitSelectionBehavior) error { self.c.Mutexes().LocalCommitsMutex.Lock() defer self.c.Mutexes().LocalCommitsMutex.Unlock() + var selectionRange *localCommitSelectionRange + if commitSelection == types.KeepCommitSelectionByHash { + selectedIdx, rangeStartIdx, rangeSelectMode := self.c.Contexts().LocalCommits.GetSelectionRangeAndMode() + selectionRange = captureLocalCommitSelectionRange(self.c.Model().Commits, selectedIdx, rangeStartIdx, rangeSelectMode) + } + checkedOutRef := self.determineCheckedOutRef() commits, err := self.c.Git().Loaders.CommitLoader.GetCommits( git_commands.GetCommitsOptions{ @@ -460,10 +469,88 @@ func (self *RefreshHelper) refreshCommitsWithLimit() error { self.c.Model().CheckedOutBranch = "" } + scrollSelectionIntoView := false + switch commitSelection { + case types.SelectHeadCommit: + if headCommitIdx := models.HeadCommitIdx(commits); headCommitIdx >= 0 { + self.c.Contexts().LocalCommits.SetSelection(headCommitIdx) + scrollSelectionIntoView = true + } + case types.KeepCommitSelectionByHash: + if selectionRange != nil { + selectedIdx, rangeStartIdx, didMove, found := findLocalCommitSelectionRange(commits, selectionRange) + if found { + self.c.Contexts().LocalCommits.SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, selectionRange.mode) + scrollSelectionIntoView = didMove + } + } + case types.KeepCommitSelectionIndex: + // The caller set the selection index deliberately; leave it untouched. + } + self.refreshView(self.c.Contexts().LocalCommits) + if scrollSelectionIntoView { + self.c.OnUIThread(func() error { + self.c.Contexts().LocalCommits.FocusLine(true) + return nil + }) + } return nil } +type localCommitSelectionRange struct { + selectedHash string + selectedIsTODO bool + rangeStartHash string + rangeStartIsTODO bool + selectedIdx int + rangeStartIdx int + mode traits.RangeSelectMode +} + +func captureLocalCommitSelectionRange( + commits []*models.Commit, + selectedIdx int, + rangeStartIdx int, + mode traits.RangeSelectMode, +) *localCommitSelectionRange { + if !hasRestorableCommitHash(commits, selectedIdx) || !hasRestorableCommitHash(commits, rangeStartIdx) { + return nil + } + + return &localCommitSelectionRange{ + selectedHash: commits[selectedIdx].Hash(), + selectedIsTODO: commits[selectedIdx].IsTODO(), + rangeStartHash: commits[rangeStartIdx].Hash(), + rangeStartIsTODO: commits[rangeStartIdx].IsTODO(), + selectedIdx: selectedIdx, + rangeStartIdx: rangeStartIdx, + mode: mode, + } +} + +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 + }) + if !foundSelected || !foundRangeStart { + return 0, 0, false, false + } + + didMove := selectedIdx != selectionRange.selectedIdx || rangeStartIdx != selectionRange.rangeStartIdx + return selectedIdx, rangeStartIdx, didMove, true +} + +func hasRestorableCommitHash(commits []*models.Commit, idx int) bool { + return idx >= 0 && idx < len(commits) && commits[idx].Hash() != "" +} + func (self *RefreshHelper) refreshSubCommitsWithLimit() error { if self.c.Contexts().SubCommits.GetRef() == nil { return nil diff --git a/pkg/gui/controllers/helpers/refresh_helper_test.go b/pkg/gui/controllers/helpers/refresh_helper_test.go index cebd044c4..e8be06d61 100644 --- a/pkg/gui/controllers/helpers/refresh_helper_test.go +++ b/pkg/gui/controllers/helpers/refresh_helper_test.go @@ -5,10 +5,148 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/hosting_service" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/gui/context/traits" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" + "github.com/stefanhaller/git-todo-parser/todo" "github.com/stretchr/testify/assert" ) +func TestCaptureLocalCommitSelectionRange(t *testing.T) { + testCases := []struct { + name string + commits []*models.Commit + selectedIdx int + rangeStartIdx int + expected *localCommitSelectionRange + }{ + { + name: "captures selected commit and range start", + commits: makeCommits("a", "b"), + selectedIdx: 1, + rangeStartIdx: 0, + expected: &localCommitSelectionRange{ + selectedHash: "b", + rangeStartHash: "a", + selectedIdx: 1, + rangeStartIdx: 0, + mode: traits.RangeSelectModeSticky, + }, + }, + { + name: "ignores invalid range start index", + commits: makeCommits("a"), + selectedIdx: 0, + rangeStartIdx: 1, + expected: nil, + }, + { + name: "ignores empty selected hash", + commits: append(makeCommits("a"), makeTodoCommit(todo.UpdateRef)), + selectedIdx: 1, + rangeStartIdx: 0, + expected: nil, + }, + { + name: "ignores empty range start hash", + commits: append(makeCommits("a"), makeTodoCommit(todo.Exec)), + selectedIdx: 0, + rangeStartIdx: 1, + expected: nil, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + selectionRange := captureLocalCommitSelectionRange( + testCase.commits, + testCase.selectedIdx, + testCase.rangeStartIdx, + traits.RangeSelectModeSticky, + ) + + assert.Equal(t, testCase.expected, selectionRange) + }) + } +} + +func TestFindLocalCommitSelectionRange(t *testing.T) { + type expectation struct { + selectedIdx int + rangeStartIdx int + moved bool + found bool + } + + selectionRange := localCommitSelectionRange{ + selectedHash: "b", + rangeStartHash: "c", + selectedIdx: 1, + rangeStartIdx: 2, + mode: traits.RangeSelectModeSticky, + } + + testCases := []struct { + name string + commits []*models.Commit + expected expectation + }{ + { + name: "finds selection after commits are inserted above it", + commits: makeCommits("new", "a", "b", "c"), + expected: expectation{ + selectedIdx: 2, + rangeStartIdx: 3, + moved: true, + found: true, + }, + }, + { + name: "finds selection that did not move", + commits: makeCommits("a", "b", "c"), + expected: expectation{ + selectedIdx: 1, + rangeStartIdx: 2, + found: true, + }, + }, + { + name: "reports not found when a hash is missing", + commits: makeCommits("a", "b"), + expected: expectation{}, + }, + { + name: "skips todo entries with the same hash as a selected commit", + commits: []*models.Commit{ + makeTodoCommitWithHash("b", todo.Revert), + makeCommits("a")[0], + makeCommits("b")[0], + makeCommits("c")[0], + }, + expected: expectation{ + selectedIdx: 2, + rangeStartIdx: 3, + moved: true, + found: true, + }, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + selectedIdx, rangeStartIdx, moved, found := findLocalCommitSelectionRange(testCase.commits, &selectionRange) + actual := expectation{ + selectedIdx: selectedIdx, + rangeStartIdx: rangeStartIdx, + moved: moved, + found: found, + } + + assert.Equal(t, testCase.expected, actual) + }) + } +} + func TestGetGithubBaseRemote(t *testing.T) { cases := []struct { name string @@ -122,3 +260,18 @@ func makeAuthenticatedGithubRemoteInfo(name string, webDomain string, authToken info.authToken = authToken return info } + +func makeCommits(hashes ...string) []*models.Commit { + hashPool := &utils.StringPool{} + return lo.Map(hashes, func(hash string, _ int) *models.Commit { + return models.NewCommit(hashPool, models.NewCommitOpts{Hash: hash}) + }) +} + +func makeTodoCommit(action todo.TodoCommand) *models.Commit { + return models.NewCommit(&utils.StringPool{}, models.NewCommitOpts{Action: action}) +} + +func makeTodoCommitWithHash(hash string, action todo.TodoCommand) *models.Commit { + return models.NewCommit(&utils.StringPool{}, models.NewCommitOpts{Hash: hash, Action: action}) +} diff --git a/pkg/gui/controllers/helpers/refs_helper.go b/pkg/gui/controllers/helpers/refs_helper.go index a3db043ef..99e9f47ec 100644 --- a/pkg/gui/controllers/helpers/refs_helper.go +++ b/pkg/gui/controllers/helpers/refs_helper.go @@ -66,7 +66,12 @@ func (self *RefsHelper) CheckoutRef(ref string, options types.CheckoutRefOptions if options.RefreshPullRequests { scope = append(scope, types.PULL_REQUESTS) } - self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI, Scope: scope, KeepBranchSelectionIndex: true}) + self.c.Refresh(types.RefreshOptions{ + Mode: types.BLOCK_UI, + Scope: scope, + KeepBranchSelectionIndex: true, + CommitSelection: types.KeepCommitSelectionIndex, + }) } localBranch, found := lo.Find(self.c.Model().Branches, func(branch *models.Branch) bool { @@ -209,7 +214,7 @@ func (self *RefsHelper) ResetToRef(ref string, strength string, envVars []string // loading a heap of commits is slow so we limit them whenever doing a reset self.c.Contexts().LocalCommits.SetLimitCommits(true) - self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}}) + self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.FILES, types.BRANCHES, types.REFLOG, types.COMMITS}, CommitSelection: types.KeepCommitSelectionIndex}) return nil } @@ -370,7 +375,11 @@ func (self *RefsHelper) NewBranch(from string, fromFormattedName string, suggest self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI, KeepBranchSelectionIndex: true}) + self.c.Refresh(types.RefreshOptions{ + Mode: types.BLOCK_UI, + KeepBranchSelectionIndex: true, + CommitSelection: types.KeepCommitSelectionIndex, + }) } self.c.Prompt(types.PromptOpts{ @@ -525,7 +534,11 @@ func (self *RefsHelper) moveCommitsToNewBranchStackedOnCurrentBranch(newBranchNa self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI, KeepBranchSelectionIndex: true}) + self.c.Refresh(types.RefreshOptions{ + Mode: types.BLOCK_UI, + KeepBranchSelectionIndex: true, + CommitSelection: types.KeepCommitSelectionIndex, + }) return nil } @@ -563,7 +576,11 @@ func (self *RefsHelper) moveCommitsToNewBranchOffOfMainBranch(newBranchName stri self.SelectFirstBranchAndFirstCommit() - self.c.Refresh(types.RefreshOptions{Mode: types.BLOCK_UI, KeepBranchSelectionIndex: true}) + self.c.Refresh(types.RefreshOptions{ + Mode: types.BLOCK_UI, + KeepBranchSelectionIndex: true, + CommitSelection: types.KeepCommitSelectionIndex, + }) return nil } diff --git a/pkg/gui/controllers/helpers/working_tree_helper.go b/pkg/gui/controllers/helpers/working_tree_helper.go index 3ad2c54cf..7e070ba31 100644 --- a/pkg/gui/controllers/helpers/working_tree_helper.go +++ b/pkg/gui/controllers/helpers/working_tree_helper.go @@ -147,11 +147,11 @@ func (self *WorkingTreeHelper) HandleCommitPressWithMessage(initialMessage strin func (self *WorkingTreeHelper) handleCommit(summary string, description string, forceSkipHooks bool) error { cmdObj := self.c.Git().Commit.CommitCmdObj(summary, description, forceSkipHooks) self.c.LogAction(self.c.Tr.Actions.Commit) - return self.gpgHelper.WithGpgHandling(cmdObj, git_commands.CommitGpgSign, self.c.Tr.CommittingStatus, + return self.gpgHelper.WithGpgHandlingAndSelectHeadCommit(cmdObj, git_commands.CommitGpgSign, self.c.Tr.CommittingStatus, func() error { self.commitsHelper.ClearPreservedCommitMessage() return nil - }, nil) + }) } func (self *WorkingTreeHelper) switchFromCommitMessagePanelToEditor(filepath string, forceSkipHooks bool) error { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 4ab436bbc..083da4f4d 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -767,7 +767,9 @@ func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, s self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, + Mode: types.SYNC, + Scope: []types.RefreshableView{types.REBASE_COMMITS}, + CommitSelection: types.KeepCommitSelectionIndex, }) return nil } @@ -780,7 +782,7 @@ func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, s self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( - err, types.RefreshOptions{Mode: types.SYNC}) + err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -793,7 +795,9 @@ func (self *LocalCommitsController) moveUp(selectedCommits []*models.Commit, sta self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) self.c.Refresh(types.RefreshOptions{ - Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, + Mode: types.SYNC, + Scope: []types.RefreshableView{types.REBASE_COMMITS}, + CommitSelection: types.KeepCommitSelectionIndex, }) return nil } @@ -806,7 +810,7 @@ func (self *LocalCommitsController) moveUp(selectedCommits []*models.Commit, sta self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) } return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( - err, types.RefreshOptions{Mode: types.SYNC}) + err, types.RefreshOptions{Mode: types.SYNC, CommitSelection: types.KeepCommitSelectionIndex}) }) } @@ -966,8 +970,6 @@ func (self *LocalCommitsController) revert(commits []*models.Commit, start, end if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(result, types.RefreshOptions{Mode: types.SYNC}); err != nil { return err } - self.context().MoveSelection(len(commits)) - self.context().HandleFocus(types.OnFocusOpts{ScrollSelectionIntoView: true}) if mustStash { if err := self.c.Git().Stash.Pop(0); err != nil { @@ -1013,7 +1015,6 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err return err } - self.context().MoveSelectedLine(1) self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) return nil }) @@ -1114,7 +1115,6 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc return err } - self.context().MoveSelectedLine(1) self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) return nil }) diff --git a/pkg/gui/controllers/remotes_controller.go b/pkg/gui/controllers/remotes_controller.go index b2e30f231..8f5dd1ae6 100644 --- a/pkg/gui/controllers/remotes_controller.go +++ b/pkg/gui/controllers/remotes_controller.go @@ -374,6 +374,7 @@ func (self *RemotesController) fetchAndCheckout(remote *models.Remote, branchNam self.c.Context().Push(self.c.Contexts().Branches, types.OnFocusOpts{}) self.c.Helpers().Refs.SelectFirstBranchAndFirstCommit() refreshOptions.KeepBranchSelectionIndex = true + refreshOptions.CommitSelection = types.KeepCommitSelectionIndex } } self.c.Refresh(refreshOptions) diff --git a/pkg/gui/controllers/sync_controller.go b/pkg/gui/controllers/sync_controller.go index 649b53338..f1b794e97 100644 --- a/pkg/gui/controllers/sync_controller.go +++ b/pkg/gui/controllers/sync_controller.go @@ -175,7 +175,7 @@ func (self *SyncController) pullWithLock(task gocui.Task, opts PullFilesOptions) }, ) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseAndSelectHeadCommit(err) } type pushOpts struct { diff --git a/pkg/gui/types/refresh.go b/pkg/gui/types/refresh.go index c9e156180..17d917bf6 100644 --- a/pkg/gui/types/refresh.go +++ b/pkg/gui/types/refresh.go @@ -33,6 +33,28 @@ const ( BLOCK_UI // wrap code in an update call to ensure UI updates all at once and keybindings aren't executed till complete ) +// CommitSelectionBehavior controls which local commit is selected after the +// commits list is reloaded by a refresh. +type CommitSelectionBehavior int + +const ( + // Keep the same commit selected by hash (and the same range, when + // range-selecting), restoring it at its new position if it moved. This is + // the right default whenever the list reloads underneath a selection the + // user hasn't deliberately changed. + KeepCommitSelectionByHash CommitSelectionBehavior = iota + + // Leave the selection index untouched, because the caller set it itself + // before refreshing. Used when jumping to the top of the list after a + // checkout, and when following a commit that was just moved up or down. + KeepCommitSelectionIndex + + // Select the HEAD commit. Used by operations that create a new commit at + // HEAD (committing, merging, pulling with a merge); the by-hash behavior + // can't restore a commit that didn't exist before the refresh. + SelectHeadCommit +) + type RefreshOptions struct { Then func() Scope []RefreshableView // e.g. []RefreshableView{COMMITS, BRANCHES}. Leave empty to refresh everything @@ -45,6 +67,10 @@ type RefreshOptions struct { // head, and selecting index 0. KeepBranchSelectionIndex bool + // Controls which local commit is selected after the refresh. Defaults to + // KeepCommitSelectionByHash. + CommitSelection CommitSelectionBehavior + // When true, this refresh was initiated by a background routine rather than // by a user action. We use it to keep background `git status` calls from // taking optional git locks, so they don't contend for index.lock with git diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go b/pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go index fbd8ee9a6..081a71f66 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go @@ -73,25 +73,11 @@ var CherryPickCommitThatBecomesEmpty = NewIntegrationTest(NewIntegrationTestArgs // Cherry-picked commit is empty t.Views().Main().Content(DoesNotContain("diff --git")) } else { + // Older git versions drop the commit that became empty t.Views().Commits(). - // We have a bug with how the selection is updated in this case; normally you would - // expect the "two changes in one commit" commit to be selected because it was - // selected before pasting, and we try to maintain that selection. This is broken - // for two reasons: - // 1. We increment the selected line index after pasting by the number of pasted - // commits; this is wrong because we skipped the commit that became empty. So - // according to this bug, the "base" commit should be selected. - // 2. We only update the selected line index after pasting if the currently selected - // commit is not a rebase TODO commit, on the assumption that if it is, we are in a - // rebase and the cherry-picked commits end up below the selection. In this case, - // however, we still think we are cherry-picking because the final refresh after the - // CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet; - // so the "unrelated change" still has a "pick" action. - // - // Since this only happens for older git versions, we don't bother fixing it. Lines( - Contains("unrelated change").IsSelected(), - Contains("two changes in one commit"), + Contains("unrelated change"), + Contains("two changes in one commit").IsSelected(), Contains("base"), ) } diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go index b135bfd7f..7468f921c 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go @@ -78,11 +78,11 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Focus(). TopLines( - Contains("second-change-branch unrelated change").IsSelected(), + Contains("second-change-branch unrelated change"), Contains("second change"), - Contains("first change"), + Contains("first change").IsSelected(), ). - SelectNextItem(). + SelectPreviousItem(). Tap(func() { // because we picked 'Second change' when resolving the conflict, // we now see this commit as having replaced First Change with Second Change, diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go index ff9efda3c..7af67791f 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go @@ -69,23 +69,8 @@ var CherryPickConflictsEmptyCommitAfterResolving = NewIntegrationTest(NewIntegra t.Views().Commits(). Focus(). TopLines( - // We have a bug with how the selection is updated in this case; normally you would - // expect the "first change" commit to be selected because it was selected before - // pasting, and we try to maintain that selection. This is broken for two reasons: - // 1. We increment the selected line index after pasting by the number of pasted - // commits; this is wrong because we skipped the commit that became empty. So - // according to this bug, the "original" commit should be selected. - // 2. We only update the selected line index after pasting if the currently selected - // commit is not a rebase TODO commit, on the assumption that if it is, we are in a - // rebase and the cherry-picked commits end up below the selection. In this case, - // however, we still think we are cherry-picking because the final refresh after the - // CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet; - // so the "second-change-branch unrelated change" still has a "pick" action. - // - // We don't bother fixing it for now because it's a pretty niche case, and the - // nature of the problem is only cosmetic. - Contains("second-change-branch unrelated change").IsSelected(), - Contains("first change"), + Contains("second-change-branch unrelated change"), + Contains("first change").IsSelected(), Contains("original"), ) }, diff --git a/pkg/integration/tests/commit/keep_selected_commit_after_external_commit.go b/pkg/integration/tests/commit/keep_selected_commit_after_external_commit.go new file mode 100644 index 000000000..82cc66ab2 --- /dev/null +++ b/pkg/integration/tests/commit/keep_selected_commit_after_external_commit.go @@ -0,0 +1,46 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var KeepSelectedCommitAfterExternalCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Keep the same commit selected after an external commit is created", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file", "first content") + shell.Commit("first commit") + shell.UpdateFile("file", "second content") + shell.GitAddAll() + shell.Commit("second commit") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("second commit"), + Contains("first commit"), + ). + NavigateToLine(Contains("first commit")) + + t.Views().Main().Content(Contains("+first content")) + + t.GlobalPress(keys.Universal.ExecuteShellCommand) + t.ExpectPopup().Prompt(). + Title(Equals("Shell command:")). + Type("git commit --allow-empty -m 'external commit'"). + Confirm() + + t.Views().Commits(). + Lines( + Contains("external commit"), + Contains("second commit"), + Contains("first commit").IsSelected(), + ) + + t.Views().Main().Content(Contains("+first content")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 1b264e50d..fa7b7e26b 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -139,6 +139,7 @@ var tests = []*components.IntegrationTest{ commit.Highlight, commit.History, commit.HistoryComplex, + commit.KeepSelectedCommitAfterExternalCommit, commit.NewBranch, commit.PasteCommitMessage, commit.PasteCommitMessageOverExisting, From b6063cff5b4dad85223af69675d1c095b53dbce2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 16:33:13 +0200 Subject: [PATCH 8/9] 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 { From 7f96c8ff4f106f92ef78b4ac9b770059b695cd29 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 22 Jun 2026 16:36:23 +0200 Subject: [PATCH 9/9] Remove bespoke commit selection restoration when starting a rebase Starting an interactive rebase (the `edit` command and quick-start) used to capture the selected commit range by hash before starting the rebase and restore it afterwards, because new update-ref lines for stacked branches can shift the commits' positions in the list. The generic keep-selection-by-hash mechanism now does exactly this for every refresh, including these, so the bespoke code is redundant. This relies on the previous commit, which taught the generic matcher to handle the case where the selected commit turns into a rebase TODO entry while it's being edited - something the bespoke code handled implicitly by matching on hash alone. --- .../controllers/local_commits_controller.go | 42 +------------------ 1 file changed, 1 insertion(+), 41 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 083da4f4d..80f01fc03 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -8,7 +8,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gocui" "github.com/jesseduffield/lazygit/pkg/gui/context" - "github.com/jesseduffield/lazygit/pkg/gui/context/traits" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" @@ -590,15 +589,9 @@ func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, start commits := self.c.Model().Commits if !commits[endIdx].IsMerge() { - selectionRangeAndMode := self.getSelectionRangeAndMode() err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit, "") return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( - err, - types.RefreshOptions{ - Mode: types.BLOCK_UI, Then: func() { - self.restoreSelectionRangeAndMode(selectionRangeAndMode) - }, - }) + err, types.RefreshOptions{Mode: types.BLOCK_UI}) } return self.startInteractiveRebaseWithEdit(selectedCommits) @@ -618,7 +611,6 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( ) error { return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.EditCommit) - selectionRangeAndMode := self.getSelectionRangeAndMode() err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash()) return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( err, @@ -636,42 +628,10 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit( self.c.Log.Errorf("error when updating todos: %v", err) } } - - self.restoreSelectionRangeAndMode(selectionRangeAndMode) }}) }) } -type SelectionRangeAndMode struct { - selectedHash string - rangeStartHash string - mode traits.RangeSelectMode -} - -func (self *LocalCommitsController) getSelectionRangeAndMode() SelectionRangeAndMode { - selectedIdx, rangeStartIdx, rangeSelectMode := self.context().GetSelectionRangeAndMode() - commits := self.c.Model().Commits - selectedHash := commits[selectedIdx].Hash() - rangeStartHash := commits[rangeStartIdx].Hash() - return SelectionRangeAndMode{selectedHash, rangeStartHash, rangeSelectMode} -} - -func (self *LocalCommitsController) restoreSelectionRangeAndMode(selectionRangeAndMode SelectionRangeAndMode) { - // We need to select the same commit range again because after starting a rebase, - // new lines can be added for update-ref commands in the TODO file, due to - // stacked branches. So the selected commits may be in different positions in the list. - _, newSelectedIdx, ok1 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool { - return c.Hash() == selectionRangeAndMode.selectedHash - }) - _, newRangeStartIdx, ok2 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool { - return c.Hash() == selectionRangeAndMode.rangeStartHash - }) - if ok1 && ok2 { - self.context().SetSelectionRangeAndMode(newSelectedIdx, newRangeStartIdx, selectionRangeAndMode.mode) - self.context().HandleFocus(types.OnFocusOpts{}) - } -} - func (self *LocalCommitsController) findCommitForQuickStartInteractiveRebase() (*models.Commit, error) { commit, index, ok := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool { return c.IsMerge() || c.Status == models.StatusMerged