Continue/abort a conflicted cherry-pick or revert (#4441)

- **PR Description**

This is part one of a four part series of PRs that improve the
cherry-pick and revert experience.

With this first PR we make it possible to continue or abort a
cherry-pick or revert operation, in the same way you can continue or
abort a rebase or merge. Currently this is only relevant for revert,
because lazygit doesn't use git cherry-pick for copying/pasting commits.
This will change in a later PR in this series though, so here we are
already preparing for that.

Fixes #1807

Author: stefanhaller

pkg/commands/git.go

@@ -136,7 +136,7 @@ func NewGitCommandAux(
 
 	branchLoader := git_commands.NewBranchLoader(cmn, gitCommon, cmd, branchCommands.CurrentBranchInfo, configCommands)
 	commitFileLoader := git_commands.NewCommitFileLoader(cmn, cmd)
-	commitLoader := git_commands.NewCommitLoader(cmn, cmd, statusCommands.RebaseMode, gitCommon)
+	commitLoader := git_commands.NewCommitLoader(cmn, cmd, statusCommands.WorkingTreeState, gitCommon)
 	reflogCommitLoader := git_commands.NewReflogCommitLoader(cmn, cmd)
 	remoteLoader := git_commands.NewRemoteLoader(cmn, cmd, repo.Remotes)
 	worktreeLoader := git_commands.NewWorktreeLoader(gitCommon)

pkg/commands/git_commands/commit_loader.go

@@ -13,7 +13,6 @@ import (
 
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
-	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
 	"github.com/jesseduffield/lazygit/pkg/common"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/samber/lo"
@@ -31,27 +30,27 @@ type CommitLoader struct {
 	*common.Common
 	cmd oscommands.ICmdObjBuilder
 
-	getRebaseMode func() (enums.RebaseMode, error)
-	readFile      func(filename string) ([]byte, error)
-	walkFiles     func(root string, fn filepath.WalkFunc) error
-	dotGitDir     string
+	getWorkingTreeState func() models.WorkingTreeState
+	readFile            func(filename string) ([]byte, error)
+	walkFiles           func(root string, fn filepath.WalkFunc) error
+	dotGitDir           string
 	*GitCommon
 }
 
 // making our dependencies explicit for the sake of easier testing
 func NewCommitLoader(
 	cmn *common.Common,
 	cmd oscommands.ICmdObjBuilder,
-	getRebaseMode func() (enums.RebaseMode, error),
+	getWorkingTreeState func() models.WorkingTreeState,
 	gitCommon *GitCommon,
 ) *CommitLoader {
 	return &CommitLoader{
-		Common:        cmn,
-		cmd:           cmd,
-		getRebaseMode: getRebaseMode,
-		readFile:      os.ReadFile,
-		walkFiles:     filepath.Walk,
-		GitCommon:     gitCommon,
+		Common:              cmn,
+		cmd:                 cmd,
+		getWorkingTreeState: getWorkingTreeState,
+		readFile:            os.ReadFile,
+		walkFiles:           filepath.Walk,
+		GitCommon:           gitCommon,
 	}
 }
 
@@ -172,17 +171,12 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod
 		}
 	}
 
-	rebaseMode, err := self.getRebaseMode()
-	if err != nil {
-		return nil, err
-	}
-
-	if rebaseMode == enums.REBASE_MODE_NONE {
+	if !self.getWorkingTreeState().Rebasing {
 		// not in rebase mode so return original commits
 		return result, nil
 	}
 
-	rebasingCommits, err := self.getHydratedRebasingCommits(rebaseMode)
+	rebasingCommits, err := self.getHydratedRebasingCommits()
 	if err != nil {
 		return nil, err
 	}
@@ -248,8 +242,8 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool
 	}
 }
 
-func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode) ([]*models.Commit, error) {
-	commits := self.getRebasingCommits(rebaseMode)
+func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) {
+	commits := self.getRebasingCommits()
 
 	if len(commits) == 0 {
 		return nil, nil
@@ -310,11 +304,7 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode
 // git-rebase-todo example:
 // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae
 // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931
-func (self *CommitLoader) getRebasingCommits(rebaseMode enums.RebaseMode) []*models.Commit {
-	if rebaseMode != enums.REBASE_MODE_INTERACTIVE {
-		return nil
-	}
-
+func (self *CommitLoader) getRebasingCommits() []*models.Commit {
 	bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"))
 	if err != nil {
 		self.Log.Error(fmt.Sprintf("error occurred reading git-rebase-todo: %s", err.Error()))

pkg/commands/git_commands/commit_loader_test.go

@@ -8,7 +8,6 @@ import (
 	"github.com/go-errors/errors"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
-	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
 	"github.com/jesseduffield/lazygit/pkg/config"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/stefanhaller/git-todo-parser/todo"
@@ -33,17 +32,15 @@ func TestGetCommits(t *testing.T) {
 		expectedCommits []*models.Commit
 		expectedError   error
 		logOrder        string
-		rebaseMode      enums.RebaseMode
 		opts            GetCommitsOptions
 		mainBranches    []string
 	}
 
 	scenarios := []scenario{
 		{
-			testName:   "should return no commits if there are none",
-			logOrder:   "topo-order",
-			rebaseMode: enums.REBASE_MODE_NONE,
-			opts:       GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
+			testName: "should return no commits if there are none",
+			logOrder: "topo-order",
+			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
@@ -52,10 +49,9 @@ func TestGetCommits(t *testing.T) {
 			expectedError:   nil,
 		},
 		{
-			testName:   "should use proper upstream name for branch",
-			logOrder:   "topo-order",
-			rebaseMode: enums.REBASE_MODE_NONE,
-			opts:       GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false},
+			testName: "should use proper upstream name for branch",
+			logOrder: "topo-order",
+			opts:     GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
@@ -66,7 +62,6 @@ func TestGetCommits(t *testing.T) {
 		{
 			testName:     "should return commits if they are present",
 			logOrder:     "topo-order",
-			rebaseMode:   enums.REBASE_MODE_NONE,
 			opts:         GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			mainBranches: []string{"master", "main", "develop"},
 			runner: oscommands.NewFakeRunner(t).
@@ -203,7 +198,6 @@ func TestGetCommits(t *testing.T) {
 		{
 			testName:     "should not call merge-base for mainBranches if none exist",
 			logOrder:     "topo-order",
-			rebaseMode:   enums.REBASE_MODE_NONE,
 			opts:         GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			mainBranches: []string{"master", "main"},
 			runner: oscommands.NewFakeRunner(t).
@@ -240,7 +234,6 @@ func TestGetCommits(t *testing.T) {
 		{
 			testName:     "should call merge-base for all main branches that exist",
 			logOrder:     "topo-order",
-			rebaseMode:   enums.REBASE_MODE_NONE,
 			opts:         GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			mainBranches: []string{"master", "main", "develop", "1.0-hotfixes"},
 			runner: oscommands.NewFakeRunner(t).
@@ -277,10 +270,9 @@ func TestGetCommits(t *testing.T) {
 			expectedError: nil,
 		},
 		{
-			testName:   "should not specify order if `log.order` is `default`",
-			logOrder:   "default",
-			rebaseMode: enums.REBASE_MODE_NONE,
-			opts:       GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
+			testName: "should not specify order if `log.order` is `default`",
+			logOrder: "default",
+			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
@@ -289,10 +281,9 @@ func TestGetCommits(t *testing.T) {
 			expectedError:   nil,
 		},
 		{
-			testName:   "should set filter path",
-			logOrder:   "default",
-			rebaseMode: enums.REBASE_MODE_NONE,
-			opts:       GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"},
+			testName: "should set filter path",
+			logOrder: "default",
+			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"},
 			runner: oscommands.NewFakeRunner(t).
 				ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
 				ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil),
@@ -310,10 +301,10 @@ func TestGetCommits(t *testing.T) {
 			cmd := oscommands.NewDummyCmdObjBuilder(scenario.runner)
 
 			builder := &CommitLoader{
-				Common:        common,
-				cmd:           cmd,
-				getRebaseMode: func() (enums.RebaseMode, error) { return scenario.rebaseMode, nil },
-				dotGitDir:     ".git",
+				Common:              common,
+				cmd:                 cmd,
+				getWorkingTreeState: func() models.WorkingTreeState { return models.WorkingTreeState{} },
+				dotGitDir:           ".git",
 				readFile: func(filename string) ([]byte, error) {
 					return []byte(""), nil
 				},
@@ -493,10 +484,10 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
 			common := utils.NewDummyCommon()
 
 			builder := &CommitLoader{
-				Common:        common,
-				cmd:           oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)),
-				getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_INTERACTIVE, nil },
-				dotGitDir:     ".git",
+				Common:              common,
+				cmd:                 oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)),
+				getWorkingTreeState: func() models.WorkingTreeState { return models.WorkingTreeState{Rebasing: true} },
+				dotGitDir:           ".git",
 				readFile: func(filename string) ([]byte, error) {
 					return []byte(""), nil
 				},

pkg/commands/git_commands/patch.go

@@ -8,7 +8,6 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/app/daemon"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/commands/patch"
-	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
 	"github.com/stefanhaller/git-todo-parser/todo"
 )
 
@@ -229,7 +228,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
 	}
 
 	if err := self.ApplyCustomPatch(true, true); err != nil {
-		if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
+		if self.status.WorkingTreeState().Rebasing {
 			_ = self.rebase.AbortRebase()
 		}
 		return err
@@ -253,7 +252,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
 	self.rebase.onSuccessfulContinue = func() error {
 		// add patches to index
 		if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil {
-			if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING {
+			if self.status.WorkingTreeState().Rebasing {
 				_ = self.rebase.AbortRebase()
 			}
 			return err

pkg/commands/git_commands/status.go

@@ -5,7 +5,7 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
 )
 
 type StatusCommands struct {
@@ -20,50 +20,68 @@ func NewStatusCommands(
 	}
 }
 
-// RebaseMode returns "" for non-rebase mode, "normal" for normal rebase
-// and "interactive" for interactive rebase
-func (self *StatusCommands) RebaseMode() (enums.RebaseMode, error) {
-	ok, err := self.IsInNormalRebase()
-	if err == nil && ok {
-		return enums.REBASE_MODE_NORMAL, nil
-	}
-	ok, err = self.IsInInteractiveRebase()
-	if err == nil && ok {
-		return enums.REBASE_MODE_INTERACTIVE, err
-	}
-
-	return enums.REBASE_MODE_NONE, err
-}
-
-func (self *StatusCommands) WorkingTreeState() enums.RebaseMode {
-	rebaseMode, _ := self.RebaseMode()
-	if rebaseMode != enums.REBASE_MODE_NONE {
-		return enums.REBASE_MODE_REBASING
-	}
-	merging, _ := self.IsInMergeState()
-	if merging {
-		return enums.REBASE_MODE_MERGING
-	}
-	return enums.REBASE_MODE_NONE
+func (self *StatusCommands) WorkingTreeState() models.WorkingTreeState {
+	result := models.WorkingTreeState{}
+	result.Rebasing, _ = self.IsInRebase()
+	result.Merging, _ = self.IsInMergeState()
+	result.CherryPicking, _ = self.IsInCherryPick()
+	result.Reverting, _ = self.IsInRevert()
+	return result
 }
 
 func (self *StatusCommands) IsBareRepo() bool {
 	return self.repoPaths.isBareRepo
 }
 
-func (self *StatusCommands) IsInNormalRebase() (bool, error) {
+func (self *StatusCommands) IsInRebase() (bool, error) {
+	exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge"))
+	if err == nil && exists {
+		return true, nil
+	}
 	return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-apply"))
 }
 
-func (self *StatusCommands) IsInInteractiveRebase() (bool, error) {
-	return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge"))
-}
-
 // IsInMergeState states whether we are still mid-merge
 func (self *StatusCommands) IsInMergeState() (bool, error) {
 	return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "MERGE_HEAD"))
 }
 
+func (self *StatusCommands) IsInCherryPick() (bool, error) {
+	exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD"))
+	if err != nil || !exists {
+		return exists, err
+	}
+	// Sometimes, CHERRY_PICK_HEAD is present during rebases even if no
+	// cherry-pick is in progress. I suppose this is because rebase used to be
+	// implemented as a series of cherry-picks, so this could be remnants of
+	// code that is shared between cherry-pick and rebase, or something. The way
+	// to tell if this is the case is to check for the presence of the
+	// stopped-sha file, which records the sha of the last pick that was
+	// executed before the rebase stopped, and seeing if the sha in that file is
+	// the same as the one in CHERRY_PICK_HEAD.
+	cherryPickHead, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD"))
+	if err != nil {
+		return false, err
+	}
+	stoppedSha, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge", "stopped-sha"))
+	if err != nil {
+		// If we get an error we assume the file doesn't exist
+		return true, nil
+	}
+	cherryPickHeadStr := strings.TrimSpace(string(cherryPickHead))
+	stoppedShaStr := strings.TrimSpace(string(stoppedSha))
+	// Need to use HasPrefix here because the cherry-pick HEAD is a full sha1,
+	// but stopped-sha is an abbreviated sha1
+	if strings.HasPrefix(cherryPickHeadStr, stoppedShaStr) {
+		return false, nil
+	}
+	return true, nil
+}
+
+func (self *StatusCommands) IsInRevert() (bool, error) {
+	return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD"))
+}
+
 // Full ref (e.g. "refs/heads/mybranch") of the branch that is currently
 // being rebased, or empty string when we're not in a rebase
 func (self *StatusCommands) BranchBeingRebased() string {