Keep selected commits stable across refreshes (#5717)

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/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
+}

pkg/commands/models/commit_test.go

@@ -0,0 +1,81 @@
+package models
+
+import (
+	"testing"
+
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/stefanhaller/git-todo-parser/todo"
+	"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),
+		makeTestCommit("a"),
+		makeTestCommit("b"),
+	}
+
+	assert.False(t, IsHeadCommit(commits, 0))
+	assert.True(t, IsHeadCommit(commits, 1))
+	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})
+}
+
+func makeTestTodoCommit(action todo.TodoCommand) *Commit {
+	return NewCommit(&utils.StringPool{}, NewCommitOpts{Action: action})
+}

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
 }
 

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

pkg/gui/controllers/helpers/gpg_helper.go

@@ -19,11 +19,45 @@ func NewGpgHelper(c *HelperCommon) *GpgHelper {
 	}
 }
 
+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)
+}
+
+// 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
 // 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,
+	failureRefreshOptions types.RefreshOptions,
+	successRefreshOptions types.RefreshOptions,
+) error {
 	useSubprocess := self.c.Git().Config.NeedsGpgSubprocess(configKey)
 	if useSubprocess {
 		success, err := self.c.RunSubprocess(cmdObj)
@@ -32,18 +66,29 @@ func (self *GpgHelper) WithGpgHandling(cmdObj *oscommands.CmdObj, configKey git_
 				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) error {
+func (self *GpgHelper) runAndStream(
+	cmdObj *oscommands.CmdObj,
+	waitingStatus string,
+	onSuccess func() error,
+	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,
 			)
@@ -55,7 +100,7 @@ func (self *GpgHelper) runAndStream(cmdObj *oscommands.CmdObj, waitingStatus str
 			}
 		}
 
-		self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC, Scope: refreshScope})
+		self.c.Refresh(successRefreshOptions)
 		return nil
 	})
 }

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,15 +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),
-		)
-	}
-	result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command)
-	if err := self.CheckMergeOrRebase(result); err != nil {
+		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
 	}
-	return nil
+	result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command)
+	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 {
@@ -169,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
@@ -492,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)
 	}
 }
 

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,110 @@ 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 := findCommitByHashPreferringTODOStatus(
+		commits, selectionRange.selectedHash, selectionRange.selectedIsTODO)
+	rangeStartIdx, foundRangeStart := findCommitByHashPreferringTODOStatus(
+		commits, selectionRange.rangeStartHash, selectionRange.rangeStartIsTODO)
+	if !foundSelected || !foundRangeStart {
+		return 0, 0, false, false
+	}
+
+	didMove := selectedIdx != selectionRange.selectedIdx || rangeStartIdx != selectionRange.rangeStartIdx
+	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() != ""
+}
+
 func (self *RefreshHelper) refreshSubCommitsWithLimit() error {
 	if self.c.Contexts().SubCommits.GetRef() == nil {
 		return nil

pkg/gui/controllers/helpers/refresh_helper_test.go

@@ -5,10 +5,161 @@ 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,
+			},
+		},
+		{
+			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 {
+		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 +273,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})
+}

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
 }
 

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 {

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
@@ -767,7 +727,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 +742,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 +755,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 +770,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 +930,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 +975,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 +1075,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
 				})

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)

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 {

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

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"),
 				)
 		}

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,

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"),
 			)
 	},

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"))
+	},
+})

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"),

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,