fix(rebase): correct parent commit selection logic when git.log.showWholeGraph=true

- Able to accurately select the parent commit now, preventing incorrect
operations on merge commits and complex histories.
- Updated method signatures to accept a parent commit index for precise
targeting.
- Improved english confirmation prompts and translations to clarify
actions affect the parent commit below.

- Fixed: squash "s", drop "d", reword "r/R", discard "d",
edit/interactive-rebase "e", fixup "f", amend "a/A, patch "Ctrl-P"
- Not yet fixed: interative-rebase "i"

fix: GetParentCommit edge cases

Author: otakenz

pkg/commands/git_commands/patch.go

@@ -88,8 +88,8 @@ func (self *PatchCommands) SaveTemporaryPatch(patch string) (string, error) {
 }
 
 // DeletePatchesFromCommit applies a patch in reverse for a commit
-func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, commitIndex int) error {
-	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIndex, false); err != nil {
+func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, commitIndex int, parentIdx int) error {
+	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIndex, parentIdx, false); err != nil {
 		return err
 	}
 
@@ -113,12 +113,12 @@ func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, com
 	return self.rebase.ContinueRebase()
 }
 
-func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, sourceCommitIdx int, destinationCommitIdx int) error {
+func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, sourceCommitIdx int, destinationCommitIdx int, parentIdx int) error {
 	if sourceCommitIdx < destinationCommitIdx {
 		// Passing true for keepCommitsThatBecomeEmpty: if the moved-from
 		// commit becomes empty, we want to keep it, mainly for consistency with
 		// moving the patch to a *later* commit, which behaves the same.
-		if err := self.rebase.BeginInteractiveRebaseForCommit(commits, destinationCommitIdx, true); err != nil {
+		if err := self.rebase.BeginInteractiveRebaseForCommit(commits, destinationCommitIdx, parentIdx, true); err != nil {
 			return err
 		}
 
@@ -217,14 +217,14 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s
 	return self.rebase.ContinueRebase()
 }
 
-func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitIdx int, stash bool) error {
+func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitIdx int, parentIdx int, stash bool) error {
 	if stash {
 		if err := self.stash.Push(fmt.Sprintf(self.Tr.AutoStashForMovingPatchToIndex, commits[commitIdx].ShortHash())); err != nil {
 			return err
 		}
 	}
 
-	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, false); err != nil {
+	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, parentIdx, false); err != nil {
 		return err
 	}
 
@@ -275,10 +275,11 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
 func (self *PatchCommands) PullPatchIntoNewCommit(
 	commits []*models.Commit,
 	commitIdx int,
+	parentIdx int,
 	commitSummary string,
 	commitDescription string,
 ) error {
-	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, false); err != nil {
+	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, parentIdx, false); err != nil {
 		return err
 	}
 
@@ -318,10 +319,11 @@ func (self *PatchCommands) PullPatchIntoNewCommit(
 func (self *PatchCommands) PullPatchIntoNewCommitBefore(
 	commits []*models.Commit,
 	commitIdx int,
+	parentIdx int,
 	commitSummary string,
 	commitDescription string,
 ) error {
-	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx+1, true); err != nil {
+	if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx+1, parentIdx, true); err != nil {
 		return err
 	}
 

pkg/commands/git_commands/rebase.go

@@ -34,14 +34,14 @@ func NewRebaseCommands(
 	}
 }
 
-func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, summary string, description string) error {
+func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, parentIdx int, summary string, description string) error {
 	// This check is currently unreachable (handled in LocalCommitsController.reword),
 	// but kept as a safeguard in case this method is used elsewhere.
 	if self.config.NeedsGpgSubprocessForCommit() {
 		return errors.New(self.Tr.DisabledForGPG)
 	}
 
-	err := self.BeginInteractiveRebaseForCommit(commits, index, false)
+	err := self.BeginInteractiveRebaseForCommit(commits, index, parentIdx, false)
 	if err != nil {
 		return err
 	}
@@ -55,44 +55,44 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, su
 	return self.ContinueRebase()
 }
 
-func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (*oscommands.CmdObj, error) {
+func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int, parentIdx int) (*oscommands.CmdObj, error) {
 	changes := []daemon.ChangeTodoAction{{
 		Hash:      commits[index].Hash(),
 		NewAction: todo.Reword,
 	}}
 	self.os.LogCommand(logTodoChanges(changes), false)
 
 	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
-		baseHashOrRoot: getBaseHashOrRoot(commits, index+1),
+		baseHashOrRoot: getBaseHashOrRoot(commits, parentIdx),
 		instruction:    daemon.NewChangeTodoActionsInstruction(changes),
 	}), nil
 }
 
-func (self *RebaseCommands) ResetCommitAuthor(commits []*models.Commit, start, end int) error {
-	return self.GenericAmend(commits, start, end, func(_ *models.Commit) error {
+func (self *RebaseCommands) ResetCommitAuthor(commits []*models.Commit, start, end int, parentIdx int) error {
+	return self.GenericAmend(commits, start, end, parentIdx, func(_ *models.Commit) error {
 		return self.commit.ResetAuthor()
 	})
 }
 
-func (self *RebaseCommands) SetCommitAuthor(commits []*models.Commit, start, end int, value string) error {
-	return self.GenericAmend(commits, start, end, func(_ *models.Commit) error {
+func (self *RebaseCommands) SetCommitAuthor(commits []*models.Commit, start, end int, parentIdx int, value string) error {
+	return self.GenericAmend(commits, start, end, parentIdx, func(_ *models.Commit) error {
 		return self.commit.SetAuthor(value)
 	})
 }
 
-func (self *RebaseCommands) AddCommitCoAuthor(commits []*models.Commit, start, end int, value string) error {
-	return self.GenericAmend(commits, start, end, func(commit *models.Commit) error {
+func (self *RebaseCommands) AddCommitCoAuthor(commits []*models.Commit, start, end int, parentIdx int, value string) error {
+	return self.GenericAmend(commits, start, end, parentIdx, func(commit *models.Commit) error {
 		return self.commit.AddCoAuthor(commit.Hash(), value)
 	})
 }
 
-func (self *RebaseCommands) GenericAmend(commits []*models.Commit, start, end int, f func(commit *models.Commit) error) error {
+func (self *RebaseCommands) GenericAmend(commits []*models.Commit, start, end int, parentIdx int, f func(commit *models.Commit) error) error {
 	if start == end && models.IsHeadCommit(commits, start) {
 		// we've selected the top commit so no rebase is required
 		return f(commits[start])
 	}
 
-	err := self.BeginInteractiveRebaseForCommitRange(commits, start, end, false)
+	err := self.BeginInteractiveRebaseForCommitRange(commits, start, end, parentIdx, false)
 	if err != nil {
 		return err
 	}
@@ -139,13 +139,8 @@ func (self *RebaseCommands) MoveCommitsUp(commits []*models.Commit, startIdx int
 	}).Run()
 }
 
-func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx int, endIdx int, action todo.TodoCommand) error {
-	baseIndex := endIdx + 1
-	if action == todo.Squash || action == todo.Fixup {
-		baseIndex++
-	}
-
-	baseHashOrRoot := getBaseHashOrRoot(commits, baseIndex)
+func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx int, endIdx int, parentIdx int, action todo.TodoCommand) error {
+	baseHashOrRoot := getBaseHashOrRoot(commits, parentIdx)
 
 	changes := lo.FilterMap(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) (daemon.ChangeTodoAction, bool) {
 		return daemon.ChangeTodoAction{
@@ -294,7 +289,7 @@ func (self *RebaseCommands) getHashOfLastCommitMade() (string, error) {
 }
 
 // AmendTo amends the given commit with whatever files are staged
-func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error {
+func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int, parentIdx int) error {
 	commit := commits[commitIndex]
 
 	if err := self.commit.CreateFixupCommit(commit.Hash()); err != nil {
@@ -307,7 +302,7 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
 	}
 
 	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
-		baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex+1),
+		baseHashOrRoot: getBaseHashOrRoot(commits, parentIdx),
 		overrideEditor: true,
 		instruction:    daemon.NewMoveFixupCommitDownInstruction(commit.Hash(), fixupHash, true),
 	}).Run()
@@ -401,7 +396,7 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er
 // BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current
 // commit and pick all others. After this you'll want to call `self.ContinueRebase()
 func (self *RebaseCommands) BeginInteractiveRebaseForCommit(
-	commits []*models.Commit, commitIndex int, keepCommitsThatBecomeEmpty bool,
+	commits []*models.Commit, commitIndex int, parentIdx int, keepCommitsThatBecomeEmpty bool,
 ) error {
 	if commitIndex < len(commits) && commits[commitIndex].IsMerge() {
 		if self.config.NeedsGpgSubprocessForCommit() {
@@ -415,11 +410,11 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommit(
 		}).Run()
 	}
 
-	return self.BeginInteractiveRebaseForCommitRange(commits, commitIndex, commitIndex, keepCommitsThatBecomeEmpty)
+	return self.BeginInteractiveRebaseForCommitRange(commits, commitIndex, commitIndex, parentIdx, keepCommitsThatBecomeEmpty)
 }
 
 func (self *RebaseCommands) BeginInteractiveRebaseForCommitRange(
-	commits []*models.Commit, start, end int, keepCommitsThatBecomeEmpty bool,
+	commits []*models.Commit, start, end int, parentIdx int, keepCommitsThatBecomeEmpty bool,
 ) error {
 	if len(commits)-1 < end {
 		return errors.New("index outside of range of commits")
@@ -442,7 +437,7 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommitRange(
 	self.os.LogCommand(logTodoChanges(changes), false)
 
 	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
-		baseHashOrRoot:             getBaseHashOrRoot(commits, end+1),
+		baseHashOrRoot:             getBaseHashOrRoot(commits, parentIdx),
 		overrideEditor:             true,
 		keepCommitsThatBecomeEmpty: keepCommitsThatBecomeEmpty,
 		instruction:                daemon.NewChangeTodoActionsInstruction(changes),
@@ -515,8 +510,8 @@ func (self *RebaseCommands) runSkipEditorCommand(cmdObj *oscommands.CmdObj) erro
 }
 
 // DiscardOldFileChanges discards changes to a file from an old commit
-func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, commitIndex int, filePaths []string) error {
-	if err := self.BeginInteractiveRebaseForCommit(commits, commitIndex, false); err != nil {
+func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, commitIndex int, parentIdx int, filePaths []string) error {
+	if err := self.BeginInteractiveRebaseForCommit(commits, commitIndex, parentIdx, false); err != nil {
 		return err
 	}
 

pkg/commands/git_commands/rebase_test.go

@@ -100,6 +100,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 		gitConfigMockResponses map[string]string
 		commitOpts             []models.NewCommitOpts
 		commitIndex            int
+		parentIndex            int
 		fileName               []string
 		runner                 *oscommands.FakeCmdObjRunner
 		test                   func(error)
@@ -111,6 +112,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 			gitConfigMockResponses: nil,
 			commitOpts:             []models.NewCommitOpts{},
 			commitIndex:            0,
+			parentIndex:            0,
 			fileName:               []string{"test999.txt"},
 			runner:                 oscommands.NewFakeRunner(t),
 			test: func(err error) {
@@ -122,6 +124,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 			gitConfigMockResponses: map[string]string{"commit.gpgSign": "true"},
 			commitOpts:             []models.NewCommitOpts{{Name: "commit", Hash: "123456"}},
 			commitIndex:            0,
+			parentIndex:            0,
 			fileName:               []string{"test999.txt"},
 			runner:                 oscommands.NewFakeRunner(t),
 			test: func(err error) {
@@ -136,6 +139,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 				{Name: "commit2", Hash: "abcdef"},
 			},
 			commitIndex: 0,
+			parentIndex: 1,
 			fileName:    []string{"test999.txt"},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"rebase", "--interactive", "--autostash", "--keep-empty", "--no-autosquash", "--rebase-merges", "abcdef"}, "", nil).
@@ -163,7 +167,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
 			commits := lo.Map(s.commitOpts,
 				func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) })
 
-			s.test(instance.DiscardOldFileChanges(commits, s.commitIndex, s.fileName))
+			s.test(instance.DiscardOldFileChanges(commits, s.commitIndex, s.parentIndex, s.fileName))
 			s.runner.CheckForMissingCalls()
 		})
 	}

pkg/gui/controllers/commits_files_controller.go

@@ -323,7 +323,10 @@ func (self *CommitFilesController) discard(selectedNodes []*filetree.CommitFileN
 					})
 				}
 
-				err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), filePaths)
+				selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+				_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 1)
+
+				err := self.c.Git().Rebase.DiscardOldFileChanges(self.c.Model().Commits, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), parentIdx, filePaths)
 				if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
 					return err
 				}

pkg/gui/controllers/custom_patch_options_menu_action.go

@@ -145,8 +145,10 @@ func (self *CustomPatchOptionsMenuAction) handleDeletePatchFromCommit() error {
 
 	return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 		commitIndex := self.getPatchCommitIndex()
+		selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+		_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 1)
 		self.c.LogAction(self.c.Tr.Actions.RemovePatchFromCommit)
-		err := self.c.Git().Patch.DeletePatchesFromCommit(self.c.Model().Commits, commitIndex)
+		err := self.c.Git().Patch.DeletePatchesFromCommit(self.c.Model().Commits, commitIndex, parentIdx)
 		return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err)
 	})
 }
@@ -160,8 +162,10 @@ func (self *CustomPatchOptionsMenuAction) handleMovePatchToSelectedCommit() erro
 
 	return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 		commitIndex := self.getPatchCommitIndex()
+		selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+		_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 1)
 		self.c.LogAction(self.c.Tr.Actions.MovePatchToSelectedCommit)
-		err := self.c.Git().Patch.MovePatchToSelectedCommit(self.c.Model().Commits, commitIndex, self.c.Contexts().LocalCommits.GetSelectedLineIdx())
+		err := self.c.Git().Patch.MovePatchToSelectedCommit(self.c.Model().Commits, commitIndex, self.c.Contexts().LocalCommits.GetSelectedLineIdx(), parentIdx)
 		return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err)
 	})
 }
@@ -180,8 +184,10 @@ func (self *CustomPatchOptionsMenuAction) handleMovePatchIntoWorkingTree() error
 		HandleConfirm: func() error {
 			return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 				commitIndex := self.getPatchCommitIndex()
+				selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+				_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 1)
 				self.c.LogAction(self.c.Tr.Actions.MovePatchIntoIndex)
-				err := self.c.Git().Patch.MovePatchIntoIndex(self.c.Model().Commits, commitIndex, mustStash)
+				err := self.c.Git().Patch.MovePatchIntoIndex(self.c.Model().Commits, commitIndex, parentIdx, mustStash)
 				return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err)
 			})
 		},
@@ -196,6 +202,8 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error {
 	self.returnFocusFromPatchExplorerIfNecessary()
 
 	commitIndex := self.getPatchCommitIndex()
+	selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+	_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 1)
 	self.c.Helpers().Commits.OpenCommitMessagePanel(
 		&helpers.OpenCommitMessagePanelOpts{
 			// Pass a commit index of one less than the moved-from commit, so that
@@ -209,7 +217,7 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommit() error {
 				return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 					self.c.Helpers().Commits.CloseCommitMessagePanel()
 					self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit)
-					err := self.c.Git().Patch.PullPatchIntoNewCommit(self.c.Model().Commits, commitIndex, summary, description)
+					err := self.c.Git().Patch.PullPatchIntoNewCommit(self.c.Model().Commits, commitIndex, parentIdx, summary, description)
 					if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
 						return err
 					}
@@ -231,6 +239,8 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e
 	self.returnFocusFromPatchExplorerIfNecessary()
 
 	commitIndex := self.getPatchCommitIndex()
+	selectedCommits, _, endIdx := self.c.Contexts().LocalCommits.GetSelectedItems()
+	_, parentIdx := self.c.Helpers().Commits.GetParentCommit(selectedCommits, endIdx, 2)
 	self.c.Helpers().Commits.OpenCommitMessagePanel(
 		&helpers.OpenCommitMessagePanelOpts{
 			// Pass a commit index of one less than the moved-from commit, so that
@@ -244,7 +254,7 @@ func (self *CustomPatchOptionsMenuAction) handlePullPatchIntoNewCommitBefore() e
 				return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 					self.c.Helpers().Commits.CloseCommitMessagePanel()
 					self.c.LogAction(self.c.Tr.Actions.MovePatchIntoNewCommit)
-					err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(self.c.Model().Commits, commitIndex, summary, description)
+					err := self.c.Git().Patch.PullPatchIntoNewCommitBefore(self.c.Model().Commits, commitIndex, parentIdx, summary, description)
 					if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err); err != nil {
 						return err
 					}

pkg/gui/controllers/helpers/commits_helper.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/samber/lo"
 )
@@ -232,6 +233,39 @@ func (self *CommitsHelper) OpenCommitMenu(suggestionFunc func(string) []*types.S
 	})
 }
 
+func (self *CommitsHelper) GetParentCommit(selectedCommits []*models.Commit, endIdx int, depth int) (*models.Commit, int) {
+	commits := self.c.Model().Commits
+
+	currentCommit := selectedCommits[len(selectedCommits)-1]
+	parentIndex := endIdx
+	var parentCommit *models.Commit
+
+	for d := 0; d < depth; d++ {
+		selectedParents := currentCommit.Parents()
+		if len(selectedParents) == 0 {
+			self.c.Log.Warn("Selected commit has no parents, the nearest commit is used")
+			return commits[parentIndex], parentIndex + 1
+		}
+		parentHash := selectedParents[0]
+		found := false
+		for i, commit := range commits {
+			if commit.Hash() == parentHash {
+				parentIndex = i
+				parentCommit = commits[parentIndex]
+				currentCommit = parentCommit
+				found = true
+				break
+			}
+		}
+		if !found {
+			self.c.Log.Warnf("Parent commit %s not found in visible commit list, the nearest commit is used ", parentHash)
+			return commits[parentIndex], parentIndex + 1
+		}
+	}
+
+	return parentCommit, parentIndex
+}
+
 func (self *CommitsHelper) addCoAuthor(suggestionFunc func(string) []*types.Suggestion) error {
 	self.c.Prompt(types.PromptOpts{
 		Title:               self.c.Tr.AddCoAuthorPromptTitle,