Drop update-ref commands at the top of the rebase-todo file

The rebase.updateRefs feature of git is very useful to rebase a stack of
branches and keep everything nicely stacked; however, it is usually in the way
when you make a copy of a branch and want to rebase it "away" from the original
branch in some way or other. For example, the original branch might sit on main,
and you want to rebase the copy onto devel to see if things still compile there.
Or you want to do some heavy history rewriting experiments on the copy, but keep
the original branch in case the experiments fail. Or you want to split a branch
in two because it contains two unrelated sets of changes; so you make a copy,
and drop half of the commits from the copy, then check out the original branch
and drop the other half of the commits from it.

In all these cases, git's updateRefs feature insists on moving the original
branch along with the copy in the first rebase that you make on the copy. I
think this is a bug in git, it should create update-ref todos only for branches
that point into the middle of your branch (because only then do they form a
stack), not when they point at the head (because then it's a copy). I had a long
discussion about this on the git mailing list [1], but people either don't agree
or don't care enough.

So we fix this on our side: whenever we start a rebase for whatever reason, be
it interactive, non-interactive, or behind-the-scenes, we drop any update-ref
todos that are at the very top of the todo list, which fixes all the
above-mentioned scenarios nicely.

I will admit that there's one scenario where git's behavior is the desired one,
and the fix in this PR makes it worse: when you create a new branch off of an
existing one, with the intention of creating a stack of branches, but before you
make the first commit on the new branch you realize some problem with the first
branch (e.g. a commit that needs to be reworded or dropped). It this case you do
want both branches to be affected by the change. In my experience this scenario
is much rarer than the other ones that I described above, and it's also much
easier to recover from: just check out the other branch again and hard-reset it
to the rebased one.

[1]
https://public-inbox.org/git/354f9fed-567f-42c8-9da9-148a5e223022@haller-berlin.de/

7 files changed, 68 insertions, 15 deletions

diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go
index 16dcce4a2..045491fce 100644
--- a/pkg/app/daemon/daemon.go
+++ b/pkg/app/daemon/daemon.go
@@ -33,6 +33,7 @@ const (
 	DaemonKindUnknown DaemonKind = iota
 
 	DaemonKindExitImmediately
+	DaemonKindRemoveUpdateRefsForCopiedBranch
 	DaemonKindCherryPick
 	DaemonKindMoveTodosUp
 	DaemonKindMoveTodosDown
@@ -53,14 +54,15 @@ func getInstruction() Instruction {
 	jsonData := os.Getenv(DaemonInstructionEnvKey)
 
 	mapping := map[DaemonKind]func(string) Instruction{
-		DaemonKindExitImmediately:     deserializeInstruction[*ExitImmediatelyInstruction],
-		DaemonKindCherryPick:          deserializeInstruction[*CherryPickCommitsInstruction],
-		DaemonKindChangeTodoActions:   deserializeInstruction[*ChangeTodoActionsInstruction],
-		DaemonKindMoveFixupCommitDown: deserializeInstruction[*MoveFixupCommitDownInstruction],
-		DaemonKindMoveTodosUp:         deserializeInstruction[*MoveTodosUpInstruction],
-		DaemonKindMoveTodosDown:       deserializeInstruction[*MoveTodosDownInstruction],
-		DaemonKindInsertBreak:         deserializeInstruction[*InsertBreakInstruction],
-		DaemonKindWriteRebaseTodo:     deserializeInstruction[*WriteRebaseTodoInstruction],
+		DaemonKindExitImmediately:                 deserializeInstruction[*ExitImmediatelyInstruction],
+		DaemonKindRemoveUpdateRefsForCopiedBranch: deserializeInstruction[*RemoveUpdateRefsForCopiedBranchInstruction],
+		DaemonKindCherryPick:                      deserializeInstruction[*CherryPickCommitsInstruction],
+		DaemonKindChangeTodoActions:               deserializeInstruction[*ChangeTodoActionsInstruction],
+		DaemonKindMoveFixupCommitDown:             deserializeInstruction[*MoveFixupCommitDownInstruction],
+		DaemonKindMoveTodosUp:                     deserializeInstruction[*MoveTodosUpInstruction],
+		DaemonKindMoveTodosDown:                   deserializeInstruction[*MoveTodosDownInstruction],
+		DaemonKindInsertBreak:                     deserializeInstruction[*InsertBreakInstruction],
+		DaemonKindWriteRebaseTodo:                 deserializeInstruction[*WriteRebaseTodoInstruction],
 	}
 
 	return mapping[getDaemonKind()](jsonData)
@@ -157,6 +159,26 @@ func NewExitImmediatelyInstruction() Instruction {
 	return &ExitImmediatelyInstruction{}
 }
 
+type RemoveUpdateRefsForCopiedBranchInstruction struct{}
+
+func (self *RemoveUpdateRefsForCopiedBranchInstruction) Kind() DaemonKind {
+	return DaemonKindRemoveUpdateRefsForCopiedBranch
+}
+
+func (self *RemoveUpdateRefsForCopiedBranchInstruction) SerializedInstructions() string {
+	return serializeInstruction(self)
+}
+
+func (self *RemoveUpdateRefsForCopiedBranchInstruction) run(common *common.Common) error {
+	return handleInteractiveRebase(common, func(path string) error {
+		return nil
+	})
+}
+
+func NewRemoveUpdateRefsForCopiedBranchInstruction() Instruction {
+	return &RemoveUpdateRefsForCopiedBranchInstruction{}
+}
+
 type CherryPickCommitsInstruction struct {
 	Todo string
 }
diff --git a/pkg/app/daemon/rebase.go b/pkg/app/daemon/rebase.go
index 1ab91de8e..8cc16d3b1 100644
--- a/pkg/app/daemon/rebase.go
+++ b/pkg/app/daemon/rebase.go
@@ -8,6 +8,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/common"
 	"github.com/jesseduffield/lazygit/pkg/env"
+	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/samber/lo"
 	"github.com/stefanhaller/git-todo-parser/todo"
 )
@@ -44,6 +45,10 @@ func handleInteractiveRebase(common *common.Common, f func(path string) error) e
 	path := os.Args[1]
 
 	if strings.HasSuffix(path, "git-rebase-todo") {
+		err := utils.RemoveUpdateRefsForCopiedBranch(path, getCommentChar())
+		if err != nil {
+			return err
+		}
 		return f(path)
 	} else if strings.HasSuffix(path, filepath.Join(gitDir(), "COMMIT_EDITMSG")) { // TODO: test
 		// if we are rebasing and squashing, we'll see a COMMIT_EDITMSG
diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go
index ff7ecec09..040df0070 100644
--- a/pkg/commands/git_commands/rebase.go
+++ b/pkg/commands/git_commands/rebase.go
@@ -233,7 +233,7 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract
 	if opts.instruction != nil {
 		cmdObj.AddEnvVars(daemon.ToEnvVars(opts.instruction)...)
 	} else {
-		gitSequenceEditor = "true"
+		cmdObj.AddEnvVars(daemon.ToEnvVars(daemon.NewRemoveUpdateRefsForCopiedBranchInstruction())...)
 	}
 
 	cmdObj.AddEnvVars(
diff --git a/pkg/integration/tests/branch/rebase_copied_branch.go b/pkg/integration/tests/branch/rebase_copied_branch.go
index 162c69af7..faa31093e 100644
--- a/pkg/integration/tests/branch/rebase_copied_branch.go
+++ b/pkg/integration/tests/branch/rebase_copied_branch.go
@@ -48,7 +48,7 @@ var RebaseCopiedBranch = NewIntegrationTest(NewIntegrationTestArgs{
 			})
 
 		t.Views().Commits().Lines(
-			Contains("CI * branch 2"), // wrong, don't want a star here
+			Contains("CI branch 2"),
 			Contains("CI branch 1"),
 			Contains("CI master 2"),
 			Contains("CI master 1"),
@@ -60,9 +60,8 @@ var RebaseCopiedBranch = NewIntegrationTest(NewIntegrationTestArgs{
 			PressPrimaryAction()
 
 		t.Views().Commits().Lines(
-			Contains("CI * branch 2"), // wrong, don't want a star here
+			Contains("CI branch 2"),
 			Contains("CI branch 1"),
-			Contains("CI master 2"), // wrong, don't want this commit
 			Contains("CI master 1"),
 		)
 	},
diff --git a/pkg/integration/tests/interactive_rebase/drop_commit_in_copied_branch_with_update_ref.go b/pkg/integration/tests/interactive_rebase/drop_commit_in_copied_branch_with_update_ref.go
index 567e746a5..8ea13c6e2 100644
--- a/pkg/integration/tests/interactive_rebase/drop_commit_in_copied_branch_with_update_ref.go
+++ b/pkg/integration/tests/interactive_rebase/drop_commit_in_copied_branch_with_update_ref.go
@@ -38,7 +38,7 @@ var DropCommitInCopiedBranchWithUpdateRef = NewIntegrationTest(NewIntegrationTes
 					Confirm()
 			}).
 			Lines(
-				Contains("CI * commit 03"), // don't want a star here because branch1 should no longer be pointing to it
+				Contains("CI commit 03"), // no start on this commit because branch1 is no longer pointing to it
 				Contains("CI commit 01"),
 			)
 
@@ -48,7 +48,8 @@ var DropCommitInCopiedBranchWithUpdateRef = NewIntegrationTest(NewIntegrationTes
 			PressPrimaryAction()
 
 		t.Views().Commits().Lines(
-			Contains("CI * commit 03"), // branch1 has changed like branch2, but shouldn't have
+			Contains("CI commit 03"),
+			Contains("CI commit 02"),
 			Contains("CI commit 01"),
 		)
 	},
diff --git a/pkg/integration/tests/interactive_rebase/interactive_rebase_of_copied_branch.go b/pkg/integration/tests/interactive_rebase/interactive_rebase_of_copied_branch.go
index d29cbe102..4bb3b86c7 100644
--- a/pkg/integration/tests/interactive_rebase/interactive_rebase_of_copied_branch.go
+++ b/pkg/integration/tests/interactive_rebase/interactive_rebase_of_copied_branch.go
@@ -32,7 +32,7 @@ var InteractiveRebaseOfCopiedBranch = NewIntegrationTest(NewIntegrationTestArgs{
 			NavigateToLine(Contains("commit 01")).
 			Press(keys.Universal.Edit).
 			Lines(
-				Contains("update-ref").Contains("branch1"), // we don't want this
+				// No update-ref todo for branch1 here, even though command-line git would have added it
 				Contains("pick").Contains("CI commit 03"),
 				Contains("pick").Contains("CI commit 02"),
 				Contains("CI <-- YOU ARE HERE --- commit 01"),
diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go
index 20c2e750e..e678a8f4e 100644
--- a/pkg/utils/rebase_todo.go
+++ b/pkg/utils/rebase_todo.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"fmt"
 	"os"
+	"slices"
 	"strings"
 
 	"github.com/samber/lo"
@@ -262,6 +263,31 @@ func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash strin
 	return newTodos, nil
 }
 
+func RemoveUpdateRefsForCopiedBranch(fileName string, commentChar byte) error {
+	todos, err := ReadRebaseTodoFile(fileName, commentChar)
+	if err != nil {
+		return err
+	}
+
+	// Filter out comments
+	todos = lo.Filter(todos, func(t todo.Todo, _ int) bool {
+		return t.Command != todo.Comment
+	})
+
+	// Delete any update-ref todos at the end of the todo list. These are not
+	// part of a stack of branches, and so shouldn't be updated. This makes it
+	// possible to create a copy of a branch and rebase the copy without
+	// affecting the original branch.
+	if _, i, found := lo.FindLastIndexOf(todos, func(t todo.Todo) bool {
+		return t.Command != todo.UpdateRef
+	}); found && i < len(todos)-1 {
+		todos = slices.Delete(todos, i+1, len(todos))
+		return WriteRebaseTodoFile(fileName, todos, commentChar)
+	}
+
+	return nil
+}
+
 // We render a todo in the commits view if it's a commit or if it's an
 // update-ref. We don't render label, reset, or comment lines.
 func isRenderedTodo(t todo.Todo) bool {