Fix deleting update-ref todosfix-deleting-update-ref-todos

It is a bad idea to read a git-rebase-todo file, remove some update-ref todos,
and write it back out behind git's back. This will cause git to actually remove
the branches referenced by those update-ref todos when the rebase is continued.

The reason is that git remembers the refs affected by update-ref todos at the
beginning of the rebase, and remembers information about them in the file
.git/rebase-merge/update-refs. Then, whenever the user performs a "git rebase
--edit-todo" command, it updates that file based on whether update-ref todos
were added or removed by that edit. If we rewrite the git-rebase-todo file
behind git's back, this updating doesn't happen.

Fix this by not updating the git-rebase-todo file directly in this case, but
performing a "git rebase --edit-todo" command where we set ourselves as the
editor and change the file in there. This makes git update the bookkeeping
information properly.

Ideally we would use this method for all cases where we change the
git-rebase-todo file (e.g. moving todos up/down, or changing the type of a
todo); this would be cleaner because we wouldn't mess with git's private
implementation details. I tried this, but unfortunately it isn't fast enough.
Right now, moving a todo up or down takes between 1 and 2ms on my machine;
changing it to do a "git rebase --edit-todo" slows it down to over 100ms, which
is unacceptable.

4 files changed, 77 insertions, 7 deletions

diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go
index 70490aef0..2f1cded7c 100644
--- a/pkg/app/daemon/daemon.go
+++ b/pkg/app/daemon/daemon.go
@@ -39,6 +39,7 @@ const (
 	DaemonKindInsertBreak
 	DaemonKindChangeTodoActions
 	DaemonKindMoveFixupCommitDown
+	DaemonKindWriteRebaseTodo
 )
 
 const (
@@ -59,6 +60,7 @@ func getInstruction() Instruction {
 		DaemonKindMoveTodosUp:         deserializeInstruction[*MoveTodosUpInstruction],
 		DaemonKindMoveTodosDown:       deserializeInstruction[*MoveTodosDownInstruction],
 		DaemonKindInsertBreak:         deserializeInstruction[*InsertBreakInstruction],
+		DaemonKindWriteRebaseTodo:     deserializeInstruction[*WriteRebaseTodoInstruction],
 	}
 
 	return mapping[getDaemonKind()](jsonData)
@@ -330,3 +332,27 @@ func (self *InsertBreakInstruction) run(common *common.Common) error {
 		return utils.PrependStrToTodoFile(path, []byte("break\n"))
 	})
 }
+
+type WriteRebaseTodoInstruction struct {
+	TodosFileContent []byte
+}
+
+func NewWriteRebaseTodoInstruction(todosFileContent []byte) Instruction {
+	return &WriteRebaseTodoInstruction{
+		TodosFileContent: todosFileContent,
+	}
+}
+
+func (self *WriteRebaseTodoInstruction) Kind() DaemonKind {
+	return DaemonKindWriteRebaseTodo
+}
+
+func (self *WriteRebaseTodoInstruction) SerializedInstructions() string {
+	return serializeInstruction(self)
+}
+
+func (self *WriteRebaseTodoInstruction) run(common *common.Common) error {
+	return handleInteractiveRebase(common, func(path string) error {
+		return os.WriteFile(path, self.TodosFileContent, 0o644)
+	})
+}
diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go
index e8d32de1c..c4396bbe4 100644
--- a/pkg/commands/git_commands/rebase.go
+++ b/pkg/commands/git_commands/rebase.go
@@ -251,6 +251,35 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract
 	return cmdObj
 }
 
+func (self *RebaseCommands) RewriteRebaseTodoFile(todosFileContent []byte) error {
+	ex := oscommands.GetLazygitPath()
+
+	cmdArgs := NewGitCmd("rebase").
+		Arg("--edit-todo").
+		ToArgv()
+
+	debug := "FALSE"
+	if self.Debug {
+		debug = "TRUE"
+	}
+
+	self.Log.WithField("command", cmdArgs).Debug("RunCommand")
+
+	cmdObj := self.cmd.New(cmdArgs)
+
+	cmdObj.AddEnvVars(daemon.ToEnvVars(daemon.NewWriteRebaseTodoInstruction(todosFileContent))...)
+
+	cmdObj.AddEnvVars(
+		"DEBUG="+debug,
+		"LANG=en_US.UTF-8",   // Force using EN as language
+		"LC_ALL=en_US.UTF-8", // Force using EN as language
+		"GIT_EDITOR="+ex,
+		"GIT_SEQUENCE_EDITOR="+ex,
+	)
+
+	return cmdObj.Run()
+}
+
 // AmendTo amends the given commit with whatever files are staged
 func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error {
 	commit := commits[commitIndex]
@@ -303,11 +332,16 @@ func (self *RebaseCommands) DeleteUpdateRefTodos(commits []*models.Commit) error
 		return todoFromCommit(commit)
 	})
 
-	return utils.DeleteTodos(
+	todosFileContent, err := utils.DeleteTodos(
 		filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"),
 		todosToDelete,
 		self.config.GetCoreCommentChar(),
 	)
+	if err != nil {
+		return err
+	}
+
+	return self.RewriteRebaseTodoFile(todosFileContent)
 }
 
 func (self *RebaseCommands) MoveTodosDown(commits []*models.Commit) error {
diff --git a/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go b/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go
index 1969be58e..9bb216cfe 100644
--- a/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go
+++ b/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go
@@ -66,9 +66,7 @@ var DeleteUpdateRefTodo = NewIntegrationTest(NewIntegrationTestArgs{
 		t.Views().Branches().
 			Lines(
 				Contains("branch2"),
-				/* branch1 was deleted, which is wrong:
 				Contains("branch1"),
-				*/
 			)
 	},
 })
diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go
index 3403eef97..f2b60a097 100644
--- a/pkg/utils/rebase_todo.go
+++ b/pkg/utils/rebase_todo.go
@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"bytes"
 	"fmt"
 	"os"
 	"strings"
@@ -96,6 +97,12 @@ func WriteRebaseTodoFile(fileName string, todos []todo.Todo, commentChar byte) e
 	return err
 }
 
+func todosToString(todos []todo.Todo, commentChar byte) ([]byte, error) {
+	buffer := bytes.Buffer{}
+	err := todo.Write(&buffer, todos, commentChar)
+	return buffer.Bytes(), err
+}
+
 func PrependStrToTodoFile(filePath string, linesToPrepend []byte) error {
 	existingContent, err := os.ReadFile(filePath)
 	if err != nil {
@@ -106,16 +113,21 @@ func PrependStrToTodoFile(filePath string, linesToPrepend []byte) error {
 	return os.WriteFile(filePath, linesToPrepend, 0o644)
 }
 
-func DeleteTodos(fileName string, todosToDelete []Todo, commentChar byte) error {
+// Unlike the other functions in this file, which write the changed todos file
+// back to disk, this one returns the new content as a byte slice. This is
+// because when deleting update-ref todos, we must perform a "git rebase
+// --edit-todo" command to pass the changed todos to git so that it can do some
+// housekeeping around the deleted todos. This can only be done by our caller.
+func DeleteTodos(fileName string, todosToDelete []Todo, commentChar byte) ([]byte, error) {
 	todos, err := ReadRebaseTodoFile(fileName, commentChar)
 	if err != nil {
-		return err
+		return nil, err
 	}
 	rearrangedTodos, err := deleteTodos(todos, todosToDelete)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	return WriteRebaseTodoFile(fileName, rearrangedTodos, commentChar)
+	return todosToString(rearrangedTodos, commentChar)
 }
 
 func deleteTodos(todos []todo.Todo, todosToDelete []Todo) ([]todo.Todo, error) {