With stacked branches, create fixup commit at the end of the branch it belongs to

pkg/app/daemon/daemon.go

@@ -246,16 +246,18 @@ func (self *ChangeTodoActionsInstruction) run(common *common.Common) error {
 
 // Takes the hash of some commit, and the hash of a fixup commit that was created
 // at the end of the branch, then moves the fixup commit down to right after the
-// original commit, changing its type to "fixup"
+// original commit, changing its type to "fixup" (only if ChangeToFixup is true)
 type MoveFixupCommitDownInstruction struct {
-	OriginalHash string
-	FixupHash    string
+	OriginalHash  string
+	FixupHash     string
+	ChangeToFixup bool
 }
 
-func NewMoveFixupCommitDownInstruction(originalHash string, fixupHash string) Instruction {
+func NewMoveFixupCommitDownInstruction(originalHash string, fixupHash string, changeToFixup bool) Instruction {
 	return &MoveFixupCommitDownInstruction{
-		OriginalHash: originalHash,
-		FixupHash:    fixupHash,
+		OriginalHash:  originalHash,
+		FixupHash:     fixupHash,
+		ChangeToFixup: changeToFixup,
 	}
 }
 
@@ -269,7 +271,7 @@ func (self *MoveFixupCommitDownInstruction) SerializedInstructions() string {
 
 func (self *MoveFixupCommitDownInstruction) run(common *common.Common) error {
 	return handleInteractiveRebase(common, func(path string) error {
-		return utils.MoveFixupCommitDown(path, self.OriginalHash, self.FixupHash, getCommentChar())
+		return utils.MoveFixupCommitDown(path, self.OriginalHash, self.FixupHash, self.ChangeToFixup, getCommentChar())
 	})
 }
 

pkg/commands/git_commands/rebase.go

@@ -284,6 +284,11 @@ func (self *RebaseCommands) GitRebaseEditTodo(todosFileContent []byte) error {
 	return cmdObj.Run()
 }
 
+func (self *RebaseCommands) getHashOfLastCommitMade() (string, error) {
+	cmdArgs := NewGitCmd("rev-parse").Arg("--verify", "HEAD").ToArgv()
+	return self.cmd.New(cmdArgs).RunWithOutput()
+}
+
 // AmendTo amends the given commit with whatever files are staged
 func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error {
 	commit := commits[commitIndex]
@@ -292,17 +297,28 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
 		return err
 	}
 
-	// Get the hash of the commit we just created
-	cmdArgs := NewGitCmd("rev-parse").Arg("--verify", "HEAD").ToArgv()
-	fixupHash, err := self.cmd.New(cmdArgs).RunWithOutput()
+	fixupHash, err := self.getHashOfLastCommitMade()
 	if err != nil {
 		return err
 	}
 
 	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
 		baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex+1),
 		overrideEditor: true,
-		instruction:    daemon.NewMoveFixupCommitDownInstruction(commit.Hash, fixupHash),
+		instruction:    daemon.NewMoveFixupCommitDownInstruction(commit.Hash, fixupHash, true),
+	}).Run()
+}
+
+func (self *RebaseCommands) MoveFixupCommitDown(commits []*models.Commit, targetCommitIndex int) error {
+	fixupHash, err := self.getHashOfLastCommitMade()
+	if err != nil {
+		return err
+	}
+
+	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
+		baseHashOrRoot: getBaseHashOrRoot(commits, targetCommitIndex+1),
+		overrideEditor: true,
+		instruction:    daemon.NewMoveFixupCommitDownInstruction(commits[targetCommitIndex].Hash, fixupHash, false),
 	}).Run()
 }
 

pkg/gui/controllers/local_commits_controller.go

@@ -895,6 +895,10 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
 								return err
 							}
 
+							if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil {
+								return err
+							}
+
 							self.context().MoveSelectedLine(1)
 							return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC})
 						})
@@ -924,6 +928,50 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
 	})
 }
 
+func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCommit *models.Commit) error {
+	if self.c.Git().Version.IsOlderThan(2, 38, 0) {
+		// Git 2.38.0 introduced the `rebase.updateRefs` config option. Don't
+		// move the commit down with older versions, as it would break the stack.
+		return nil
+	}
+
+	if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
+		// Can't move commits while rebasing
+		return nil
+	}
+
+	if targetCommit.Status == models.StatusMerged {
+		// Target commit is already on main. It's a bit questionable that we
+		// allow creating a fixup commit for it in the first place, but we
+		// always did, so why restrict that now; however, it doesn't make sense
+		// to move the created fixup commit down in that case.
+		return nil
+	}
+
+	if !self.c.Git().Config.GetRebaseUpdateRefs() {
+		// If the user has disabled rebase.updateRefs, we don't move the fixup
+		// because this would break the stack of branches (presumably they like
+		// to manage it themselves manually, or something).
+		return nil
+	}
+
+	headOfOwnerBranchIdx := -1
+	for i := self.context().GetSelectedLineIdx(); i > 0; i-- {
+		if lo.SomeBy(self.c.Model().Branches, func(b *models.Branch) bool {
+			return b.CommitHash == self.c.Model().Commits[i].Hash
+		}) {
+			headOfOwnerBranchIdx = i
+			break
+		}
+	}
+
+	if headOfOwnerBranchIdx == -1 {
+		return nil
+	}
+
+	return self.c.Git().Rebase.MoveFixupCommitDown(self.c.Model().Commits, headOfOwnerBranchIdx)
+}
+
 func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, includeFileChanges bool) error {
 	commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash)
 	if err != nil {
@@ -947,6 +995,10 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc
 						return err
 					}
 
+					if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil {
+						return err
+					}
+
 					self.context().MoveSelectedLine(1)
 					return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC})
 				})

pkg/integration/tests/commit/create_fixup_commit_in_branch_stack.go

@@ -0,0 +1,53 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CreateFixupCommitInBranchStack = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Create a fixup commit in a stack of branches, verify that it is created at the end of the branch it belongs to",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	GitVersion:   AtLeast("2.38.0"),
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.NewBranch("branch1")
+		shell.EmptyCommit("branch1 commit 1")
+		shell.EmptyCommit("branch1 commit 2")
+		shell.EmptyCommit("branch1 commit 3")
+		shell.NewBranch("branch2")
+		shell.EmptyCommit("branch2 commit 1")
+		shell.EmptyCommit("branch2 commit 2")
+		shell.CreateFileAndAdd("fixup-file", "fixup content")
+
+		shell.SetConfig("rebase.updateRefs", "true")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("CI ◯ branch2 commit 2"),
+				Contains("CI ◯ branch2 commit 1"),
+				Contains("CI ◯ * branch1 commit 3"),
+				Contains("CI ◯ branch1 commit 2"),
+				Contains("CI ◯ branch1 commit 1"),
+			).
+			NavigateToLine(Contains("branch1 commit 2")).
+			Press(keys.Commits.CreateFixupCommit).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("Create fixup commit")).
+					Select(Contains("fixup! commit")).
+					Confirm()
+			}).
+			Lines(
+				Contains("CI ◯ branch2 commit 2"),
+				Contains("CI ◯ branch2 commit 1"),
+				Contains("CI ◯ * fixup! branch1 commit 2"),
+				Contains("CI ◯ branch1 commit 3"),
+				Contains("CI ◯ branch1 commit 2"),
+				Contains("CI ◯ branch1 commit 1"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -87,6 +87,7 @@ var tests = []*components.IntegrationTest{
 	commit.CommitWithNonMatchingBranchName,
 	commit.CommitWithPrefix,
 	commit.CreateAmendCommit,
+	commit.CreateFixupCommitInBranchStack,
 	commit.CreateTag,
 	commit.DiscardOldFileChanges,
 	commit.FindBaseCommitForFixup,

pkg/utils/rebase_todo.go

@@ -221,21 +221,21 @@ func moveTodosUp(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) {
 	return todos, nil
 }
 
-func MoveFixupCommitDown(fileName string, originalHash string, fixupHash string, commentChar byte) error {
+func MoveFixupCommitDown(fileName string, originalHash string, fixupHash string, changeToFixup bool, commentChar byte) error {
 	todos, err := ReadRebaseTodoFile(fileName, commentChar)
 	if err != nil {
 		return err
 	}
 
-	newTodos, err := moveFixupCommitDown(todos, originalHash, fixupHash)
+	newTodos, err := moveFixupCommitDown(todos, originalHash, fixupHash, changeToFixup)
 	if err != nil {
 		return err
 	}
 
 	return WriteRebaseTodoFile(fileName, newTodos, commentChar)
 }
 
-func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash string) ([]todo.Todo, error) {
+func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash string, changeToFixup bool) ([]todo.Todo, error) {
 	isOriginal := func(t todo.Todo) bool {
 		return (t.Command == todo.Pick || t.Command == todo.Merge) && equalHash(t.Commit, originalHash)
 	}
@@ -259,7 +259,9 @@ func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash strin
 
 	newTodos := MoveElement(todos, fixupIndex, originalIndex+1)
 
-	newTodos[originalIndex+1].Command = todo.Fixup
+	if changeToFixup {
+		newTodos[originalIndex+1].Command = todo.Fixup
+	}
 
 	return newTodos, nil
 }

pkg/utils/rebase_todo_test.go

@@ -266,32 +266,50 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 		todos         []todo.Todo
 		originalHash  string
 		fixupHash     string
+		changeToFixup bool
 		expectedTodos []todo.Todo
 		expectedErr   error
 	}{
 		{
-			name: "fixup commit is the last commit",
+			name: "fixup commit is the last commit (change to fixup)",
 			todos: []todo.Todo{
 				{Command: todo.Pick, Commit: "original"},
 				{Command: todo.Pick, Commit: "fixup"},
 			},
-			originalHash: "original",
-			fixupHash:    "fixup",
+			originalHash:  "original",
+			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: []todo.Todo{
 				{Command: todo.Pick, Commit: "original"},
 				{Command: todo.Fixup, Commit: "fixup"},
 			},
 			expectedErr: nil,
 		},
+		{
+			name: "fixup commit is the last commit (don't change to fixup)",
+			todos: []todo.Todo{
+				{Command: todo.Pick, Commit: "original"},
+				{Command: todo.Pick, Commit: "fixup"},
+			},
+			originalHash:  "original",
+			fixupHash:     "fixup",
+			changeToFixup: false,
+			expectedTodos: []todo.Todo{
+				{Command: todo.Pick, Commit: "original"},
+				{Command: todo.Pick, Commit: "fixup"},
+			},
+			expectedErr: nil,
+		},
 		{
 			name: "fixup commit is separated from original commit",
 			todos: []todo.Todo{
 				{Command: todo.Pick, Commit: "original"},
 				{Command: todo.Pick, Commit: "other"},
 				{Command: todo.Pick, Commit: "fixup"},
 			},
-			originalHash: "original",
-			fixupHash:    "fixup",
+			originalHash:  "original",
+			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: []todo.Todo{
 				{Command: todo.Pick, Commit: "original"},
 				{Command: todo.Fixup, Commit: "fixup"},
@@ -306,8 +324,9 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 				{Command: todo.Pick, Commit: "other"},
 				{Command: todo.Pick, Commit: "fixup"},
 			},
-			originalHash: "original",
-			fixupHash:    "fixup",
+			originalHash:  "original",
+			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: []todo.Todo{
 				{Command: todo.Merge, Commit: "original"},
 				{Command: todo.Fixup, Commit: "fixup"},
@@ -324,6 +343,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 			},
 			originalHash:  "original",
 			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: nil,
 			expectedErr:   errors.New("Expected exactly one original hash, found 2"),
 		},
@@ -336,6 +356,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 			},
 			originalHash:  "original",
 			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: nil,
 			expectedErr:   errors.New("Expected exactly one fixup hash, found 2"),
 		},
@@ -346,6 +367,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 			},
 			originalHash:  "original",
 			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: nil,
 			expectedErr:   errors.New("Expected exactly one fixup hash, found 0"),
 		},
@@ -356,14 +378,15 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
 			},
 			originalHash:  "original",
 			fixupHash:     "fixup",
+			changeToFixup: true,
 			expectedTodos: nil,
 			expectedErr:   errors.New("Expected exactly one original hash, found 0"),
 		},
 	}
 
 	for _, scenario := range scenarios {
 		t.Run(scenario.name, func(t *testing.T) {
-			actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalHash, scenario.fixupHash)
+			actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalHash, scenario.fixupHash, scenario.changeToFixup)
 
 			if scenario.expectedErr == nil {
 				assert.NoError(t, actualErr)