From 4e24ef025b24a146a39f78d3a32322eb7527411e Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 7 Nov 2026 09:48:08 +0100 Subject: [PATCH] Show a cherry-pick or revert as in progress if only .git/sequencer exists If a cherry-pick or revert of several commits stops before git starts to apply one of them, git doesn't write CHERRY_PICK_HEAD or REVERT_HEAD. It only leaves the todos behind in .git/sequencer. Look at the first todo there, like git status does. If it is a "pick" or "revert", consider a cherry-pick or revert to be in progress. Once we show such an operation, two more things need care. The same state occurs if the user resolves a conflict and commits the result themselves. Git tells the two cases apart by whether HEAD has moved since it last tried to apply a commit, so do the same. Mark the commit that git stopped at as conflicted only if CHERRY_PICK_HEAD or REVERT_HEAD exists, and don't show it as pending if the user has committed it. Also, "git cherry-pick --continue" skips the commit that git stopped at, even if git never applied it. Before continuing in that case, put a copy of its todo in front of it, so that git skips the copy instead. Co-Authored-By: Claude Opus 5.5 --- pkg/commands/git_commands/commit_loader.go | 11 +- pkg/commands/git_commands/rebase.go | 11 ++ pkg/commands/git_commands/sequencer.go | 101 ++++++++++++ pkg/commands/git_commands/sequencer_test.go | 145 ++++++++++++++++++ pkg/commands/git_commands/status.go | 31 +++- pkg/commands/git_commands/status_test.go | 53 +++++++ ...cherry_pick_conflicts_committed_by_user.go | 84 ++++++++++ .../cherry_pick_stopped_by_untracked_file.go | 12 -- pkg/integration/tests/test_list.go | 1 + 9 files changed, 434 insertions(+), 15 deletions(-) create mode 100644 pkg/commands/git_commands/sequencer.go create mode 100644 pkg/commands/git_commands/sequencer_test.go create mode 100644 pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index b4a938880..c0b6cf580 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -259,8 +259,15 @@ func (self *CommitLoader) getHydratedSequencerCommits(hashPool *utils.StringPool commits := self.getSequencerCommits(hashPool) if len(commits) > 0 { // If we have any commits in .git/sequencer/todo, then the last one of - // those is the conflicting one. - commits[len(commits)-1].Status = models.StatusConflicted + // those is the one that git stopped at. + switch getCurrentSequencerTodoState(self.cmd, self.repoPaths.WorktreeGitDirPath()) { + case sequencerTodoStarted: + commits[len(commits)-1].Status = models.StatusConflicted + case sequencerTodoNotStarted: + // It is still pending like the other ones + case sequencerTodoCommitted: + commits = commits[:len(commits)-1] + } } else { // For single-commit cherry-picks and reverts, git apparently doesn't // use the sequencer; in that case, CHERRY_PICK_HEAD or REVERT_HEAD is diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index bce778472..6098f9493 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -491,8 +491,19 @@ func (self *RebaseCommands) AbortRebase() error { // GenericMerge takes a commandType of "merge" or "rebase" and a command of "abort", "skip" or "continue" // By default we skip the editor in the case where a commit will be made func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, command string) error { + undoRepeatingTodo := func() {} + if command == "continue" && (commandType == "cherry-pick" || commandType == "revert") { + var err error + undoRepeatingTodo, err = repeatCurrentSequencerTodo( + self.cmd, self.repoPaths.WorktreeGitDirPath(), self.config.GetCoreCommentChar()) + if err != nil { + return err + } + } + err := skipEditor(self.GenericMergeOrRebaseActionCmdObj(commandType, command)).Run() if err != nil { + undoRepeatingTodo() if !strings.Contains(err.Error(), "no rebase in progress") { return err } diff --git a/pkg/commands/git_commands/sequencer.go b/pkg/commands/git_commands/sequencer.go new file mode 100644 index 000000000..b216cc21e --- /dev/null +++ b/pkg/commands/git_commands/sequencer.go @@ -0,0 +1,101 @@ +package git_commands + +import ( + "bytes" + "os" + "path/filepath" + "strings" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/samber/lo" +) + +// sequencerTodoState tells what has happened so far with the todo that a +// cherry-pick or revert of several commits stopped at. This is the first todo +// in .git/sequencer/todo. +type sequencerTodoState int + +const ( + // Git started to apply the todo, and stopped because of conflicts (or + // because it couldn't commit the result). CHERRY_PICK_HEAD or REVERT_HEAD + // exists, and continuing commits the changes. + sequencerTodoStarted sequencerTodoState = iota + // Git stopped before it started to apply the todo, for example because the + // commit would overwrite an untracked file. + sequencerTodoNotStarted + // The user has committed the result of the todo themselves, for example + // after resolving its conflicts. Continuing skips it. + sequencerTodoCommitted +) + +// getCurrentSequencerTodoState returns the state of the todo that the +// cherry-pick or revert of several commits that is in progress in gitDir +// stopped at. +// +// Without CHERRY_PICK_HEAD or REVERT_HEAD, we can tell whether the user has +// committed the todo by whether HEAD has moved since git last tried to apply a +// commit, which git records in .git/sequencer/abort-safety. Git does the same +// when it decides whether "--skip" is possible (see rollback_is_safe in git's +// sequencer.c). +func getCurrentSequencerTodoState(cmd oscommands.ICmdObjBuilder, gitDir string) sequencerTodoState { + for _, file := range []string{"CHERRY_PICK_HEAD", "REVERT_HEAD"} { + if _, err := os.Stat(filepath.Join(gitDir, file)); err == nil { + return sequencerTodoStarted + } + } + + // If the file doesn't exist, git compares HEAD to the null hash; an empty + // string does the same for us + abortSafety, _ := os.ReadFile(filepath.Join(gitDir, "sequencer", "abort-safety")) + + // If HEAD is unborn, this fails and returns an empty string + head, _ := cmd.New(NewGitCmd("rev-parse").Arg("--verify", "--quiet", "HEAD").ToArgv()).DontLog().RunWithOutput() + + if strings.TrimSpace(head) == strings.TrimSpace(string(abortSafety)) { + return sequencerTodoNotStarted + } + return sequencerTodoCommitted +} + +// repeatCurrentSequencerTodo prepares the cherry-pick or revert of several +// commits that is in progress in gitDir for "--continue". If git stopped it +// before it started to apply the current todo, "--continue" would skip that +// todo, because git can't tell it apart from a todo that the user has +// committed themselves. So we put a copy of the todo in front of it, and git +// skips the copy instead. +// +// The returned function removes the copy again if git hasn't got as far as +// skipping it, for example because it refused to continue. +func repeatCurrentSequencerTodo(cmd oscommands.ICmdObjBuilder, gitDir string, commentChar byte) (func(), error) { + noop := func() {} + + todoPath := filepath.Join(gitDir, "sequencer", "todo") + content, err := os.ReadFile(todoPath) + if err != nil { + // No cherry-pick or revert of several commits is in progress + return noop, nil + } + + if getCurrentSequencerTodoState(cmd, gitDir) != sequencerTodoNotStarted { + return noop, nil + } + + currentTodo, found := lo.Find(strings.Split(string(content), "\n"), func(line string) bool { + line = strings.TrimSpace(line) + return line != "" && line[0] != commentChar + }) + if !found { + return noop, nil + } + + newContent := append([]byte(currentTodo+"\n"), content...) + if err := os.WriteFile(todoPath, newContent, 0o644); err != nil { + return nil, err + } + + return func() { + if currentContent, err := os.ReadFile(todoPath); err == nil && bytes.Equal(currentContent, newContent) { + _ = os.WriteFile(todoPath, content, 0o644) + } + }, nil +} diff --git a/pkg/commands/git_commands/sequencer_test.go b/pkg/commands/git_commands/sequencer_test.go new file mode 100644 index 000000000..0ee54e3bb --- /dev/null +++ b/pkg/commands/git_commands/sequencer_test.go @@ -0,0 +1,145 @@ +package git_commands + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/stretchr/testify/assert" +) + +func TestGetCurrentSequencerTodoState(t *testing.T) { + revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"} + + scenarios := []struct { + testName string + files map[string]string + runner *oscommands.FakeCmdObjRunner + expectedState sequencerTodoState + }{ + { + testName: "CHERRY_PICK_HEAD exists", + files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"}, + runner: oscommands.NewFakeRunner(t), + expectedState: sequencerTodoStarted, + }, + { + testName: "REVERT_HEAD exists", + files: map[string]string{"REVERT_HEAD": "1234567\n"}, + runner: oscommands.NewFakeRunner(t), + expectedState: sequencerTodoStarted, + }, + { + testName: "HEAD hasn't moved since git tried to apply a commit", + files: map[string]string{"sequencer/abort-safety": "1234567\n"}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "1234567\n", nil), + expectedState: sequencerTodoNotStarted, + }, + { + testName: "HEAD has moved since git tried to apply a commit", + files: map[string]string{"sequencer/abort-safety": "1234567\n"}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "2345678\n", nil), + expectedState: sequencerTodoCommitted, + }, + { + testName: "HEAD is unborn, and there is no abort-safety file", + files: map[string]string{}, + runner: oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "", errors.New("error")), + expectedState: sequencerTodoNotStarted, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + gitDir := t.TempDir() + for path, content := range s.files { + fullPath := filepath.Join(gitDir, path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + + state := getCurrentSequencerTodoState(oscommands.NewDummyCmdObjBuilder(s.runner), gitDir) + + assert.Equal(t, s.expectedState, state) + s.runner.CheckForMissingCalls() + }) + } +} + +func TestRepeatCurrentSequencerTodo(t *testing.T) { + revParseArgs := []string{"rev-parse", "--verify", "--quiet", "HEAD"} + todos := "pick 1234567 one\npick 2345678 two\n" + repeatedTodos := "pick 1234567 one\npick 1234567 one\npick 2345678 two\n" + + setUp := func(t *testing.T, files map[string]string) string { + t.Helper() + gitDir := t.TempDir() + for path, content := range files { + fullPath := filepath.Join(gitDir, path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + return gitDir + } + + readTodos := func(t *testing.T, gitDir string) string { + t.Helper() + content, err := os.ReadFile(filepath.Join(gitDir, "sequencer", "todo")) + assert.NoError(t, err) + return string(content) + } + + t.Run("no cherry-pick or revert of several commits in progress", func(t *testing.T) { + gitDir := setUp(t, map[string]string{}) + runner := oscommands.NewFakeRunner(t) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + undo() + assert.NoFileExists(t, filepath.Join(gitDir, "sequencer", "todo")) + runner.CheckForMissingCalls() + }) + + t.Run("git started to apply the current todo", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "CHERRY_PICK_HEAD": "1234567\n"}) + runner := oscommands.NewFakeRunner(t) + + _, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + assert.Equal(t, todos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) + + t.Run("git stopped before it started to apply the current todo", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"}) + runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + + assert.NoError(t, err) + assert.Equal(t, repeatedTodos, readTodos(t, gitDir)) + + // If git didn't touch the todos, undo restores them + undo() + assert.Equal(t, todos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) + + t.Run("undo leaves the todos alone if git has changed them", func(t *testing.T) { + gitDir := setUp(t, map[string]string{"sequencer/todo": todos, "sequencer/abort-safety": "abcdef0\n"}) + runner := oscommands.NewFakeRunner(t).ExpectGitArgs(revParseArgs, "abcdef0\n", nil) + + undo, err := repeatCurrentSequencerTodo(oscommands.NewDummyCmdObjBuilder(runner), gitDir, '#') + assert.NoError(t, err) + gitsTodos := "pick 2345678 two\n" + assert.NoError(t, os.WriteFile(filepath.Join(gitDir, "sequencer", "todo"), []byte(gitsTodos), 0o644)) + + undo() + assert.Equal(t, gitsTodos, readTodos(t, gitDir)) + runner.CheckForMissingCalls() + }) +} diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index d9120d87d..cdec190c4 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -7,7 +7,9 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/spf13/afero" + "github.com/stefanhaller/git-todo-parser/todo" ) type StatusCommands struct { @@ -49,6 +51,14 @@ func (self *StatusCommands) IsInMergeState() (bool, error) { } func (self *StatusCommands) IsInCherryPick() (bool, error) { + hasCherryPickHead, err := self.hasCherryPickHead() + if err != nil || hasCherryPickHead { + return hasCherryPickHead, err + } + return self.sequencerCommand() == todo.Pick, nil +} + +func (self *StatusCommands) hasCherryPickHead() (bool, error) { exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) if err != nil || !exists { return exists, err @@ -81,7 +91,26 @@ func (self *StatusCommands) IsInCherryPick() (bool, error) { } func (self *StatusCommands) IsInRevert() (bool, error) { - return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD")) + hasRevertHead, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD")) + if err != nil || hasRevertHead { + return hasRevertHead, err + } + return self.sequencerCommand() == todo.Revert, nil +} + +// sequencerCommand returns the command of the first todo of a cherry-pick or +// revert of several commits that is in progress, or 0 if there is none. Git +// keeps the todos of such an operation in .git/sequencer. If it stops before it +// starts to apply a commit (for example because the commit would overwrite an +// untracked file), it doesn't write CHERRY_PICK_HEAD or REVERT_HEAD. Like git +// status, we still consider the cherry-pick or revert to be in progress then. +func (self *StatusCommands) sequencerCommand() todo.TodoCommand { + todos, err := utils.ReadRebaseTodoFile( + filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer", "todo"), self.config.GetCoreCommentChar()) + if err != nil || len(todos) == 0 { + return 0 + } + return todos[0].Command } // RefsSnapshot returns a string fingerprint of the current state of local diff --git a/pkg/commands/git_commands/status_test.go b/pkg/commands/git_commands/status_test.go index dc6b2d558..0ec71123a 100644 --- a/pkg/commands/git_commands/status_test.go +++ b/pkg/commands/git_commands/status_test.go @@ -1,6 +1,8 @@ package git_commands import ( + "os" + "path/filepath" "testing" "github.com/go-errors/errors" @@ -89,3 +91,54 @@ func TestStatusRefsSnapshot(t *testing.T) { }) } } + +func TestStatusWorkingTreeStateOfCherryPickOrRevert(t *testing.T) { + scenarios := []struct { + testName string + files map[string]string + expectedCherryPicking bool + expectedReverting bool + }{ + { + testName: "nothing in progress", + files: map[string]string{}, + }, + { + testName: "CHERRY_PICK_HEAD", + files: map[string]string{"CHERRY_PICK_HEAD": "1234567\n"}, + expectedCherryPicking: true, + }, + { + testName: "REVERT_HEAD", + files: map[string]string{"REVERT_HEAD": "1234567\n"}, + expectedReverting: true, + }, + { + testName: "cherry-pick of several commits that stopped before applying one", + files: map[string]string{"sequencer/todo": "pick 1234567 one\npick 2345678 two\n"}, + expectedCherryPicking: true, + }, + { + testName: "revert of several commits that stopped before applying one", + files: map[string]string{"sequencer/todo": "revert 1234567 one\nrevert 2345678 two\n"}, + expectedReverting: true, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + repoDir := t.TempDir() + for path, content := range s.files { + fullPath := filepath.Join(repoDir, ".git", path) + assert.NoError(t, os.MkdirAll(filepath.Dir(fullPath), 0o755)) + assert.NoError(t, os.WriteFile(fullPath, []byte(content), 0o644)) + } + instance := buildStatusCommands(commonDeps{repoPaths: MockRepoPaths(repoDir)}) + + state := instance.WorkingTreeState() + + assert.Equal(t, s.expectedCherryPicking, state.CherryPicking) + assert.Equal(t, s.expectedReverting, state.Reverting) + }) + } +} diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go new file mode 100644 index 000000000..be986de05 --- /dev/null +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts_committed_by_user.go @@ -0,0 +1,84 @@ +package cherry_pick + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var CherryPickConflictsCommittedByUser = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Cherry pick two commits, the first of which conflicts; the user commits the resolution themselves before continuing", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.LocalBranchSortOrder = "recency" + }, + SetupRepo: func(shell *Shell) { + shared.MergeConflictsSetup(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Lines( + Contains("first-change-branch"), + Contains("second-change-branch"), + Contains("original-branch"), + ). + SelectNextItem(). + PressEnter() + + t.Views().SubCommits(). + IsFocused(). + TopLines( + Contains("second-change-branch unrelated change"), + Contains("second change"), + ). + Press(keys.Commits.CherryPickCopy). + SelectNextItem(). + Press(keys.Commits.CherryPickCopy) + + t.Views().Information().Content(Contains("2 commits copied")) + + t.Views().Commits(). + Focus(). + TopLines( + Contains("first change").IsSelected(), + ). + Press(keys.Commits.PasteCommits) + + t.ExpectPopup().Alert(). + Title(Equals("Cherry-pick")). + Content(Contains("Are you sure you want to cherry-pick the 2 copied commit(s) onto this branch?")). + Confirm() + + t.Common().AcknowledgeConflicts() + + t.Shell(). + UpdateFileAndAdd("file", shared.SecondChangeFileContent). + RunCommand([]string{"git", "commit", "--no-edit"}) + + t.GlobalPress(keys.Universal.Refresh) + + t.Views().Status().Content(Equals("(cherry-picking) repo → first-change-branch")) + + t.Views().Commits(). + TopLines( + Contains("─── Pending cherry-picks"), + Contains("pick").DoesNotContain("CONFLICT").Contains("second-change-branch unrelated change"), + Contains("─── Commits"), + Contains("second change"), + Contains("first change"), + ) + + t.Common().ContinueOnConflictsResolved("cherry-pick") + + t.Views().Status().Content(Equals("repo → first-change-branch")) + + t.Views().Commits(). + TopLines( + Contains("second-change-branch unrelated change"), + Contains("second change"), + Contains("first change"), + ) + }, +}) diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go index 54e7dca0b..40c148cf6 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_stopped_by_untracked_file.go @@ -65,7 +65,6 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Content(Contains("The following untracked working tree files would be overwritten by merge")). Confirm() }). - /* EXPECTED: Lines( Contains("─── Pending cherry-picks"), Contains("pick").DoesNotContain("CONFLICT").Contains("two"), @@ -73,22 +72,13 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Contains("one"), Contains("base"), ) - ACTUAL: */ - Lines( - Contains("one"), - Contains("base"), - ) t.Views().Status(). - /* EXPECTED: Content(Equals("(cherry-picking) repo → master")) - ACTUAL: */ - Content(Equals("repo → master")) t.Shell().DeleteFile("file2") t.GlobalPress(keys.Universal.CreateRebaseOptionsMenu) - /* EXPECTED: t.ExpectPopup().Menu(). Title(Equals("Cherry-pick options")). Select(Contains("continue")). @@ -102,7 +92,5 @@ var CherryPickStoppedByUntrackedFile = NewIntegrationTest(NewIntegrationTestArgs Contains("one"), Contains("base"), ) - ACTUAL: */ - t.ExpectToast(Equals("Disabled: You are currently neither rebasing nor merging")) }, }) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 6da04eae1..501e9a86b 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -110,6 +110,7 @@ var tests = []*components.IntegrationTest{ cherry_pick.CherryPickAcrossWorktrees, cherry_pick.CherryPickCommitThatBecomesEmpty, cherry_pick.CherryPickConflicts, + cherry_pick.CherryPickConflictsCommittedByUser, cherry_pick.CherryPickConflictsEmptyCommitAfterResolving, cherry_pick.CherryPickDuringRebase, cherry_pick.CherryPickMerge,