Create fixup commit at end of its branch when there's a stack of branches

Author: stefanhaller

pkg/commands/git_commands/rebase.go

@@ -309,6 +309,19 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
 	}).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()
+}
+
 func todoFromCommit(commit *models.Commit) utils.Todo {
 	if commit.Action == todo.UpdateRef {
 		return utils.Todo{Ref: commit.Name, Action: commit.Action}

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,