Show todo items for pending cherry-picks and reverts (#4442)

- **PR Description**

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

With this PR we include pending cherry-picks and reverts in the commit
list (like rebase todos) when a cherry-pick or revert stops with
conflicts; also, we show the conflicting item in the list like we do
with conflicting rebase todos.

As with the previous PR, this is not really very useful yet because you
can't revert a range of commits, and we don't use git cherry-pick for
copy/paste. Both of these will change in later PRs in this series, so
again this is preparation for that.

Author: stefanhaller

pkg/commands/git_commands/commit_loader.go

@@ -71,15 +71,13 @@ type GetCommitsOptions struct {
 // GetCommits obtains the commits of the current branch
 func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, error) {
 	commits := []*models.Commit{}
-	var rebasingCommits []*models.Commit
 
 	if opts.IncludeRebaseCommits && opts.FilterPath == "" {
 		var err error
-		rebasingCommits, err = self.MergeRebasingCommits(commits)
+		commits, err = self.MergeRebasingCommits(commits)
 		if err != nil {
 			return nil, err
 		}
-		commits = append(commits, rebasingCommits...)
 	}
 
 	wg := sync.WaitGroup{}
@@ -126,7 +124,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit,
 		if commit.Hash == firstPushedCommit {
 			passedFirstPushedCommit = true
 		}
-		if commit.Status != models.StatusRebasing {
+		if !commit.IsTODO() {
 			if passedFirstPushedCommit {
 				commit.Status = models.StatusPushed
 			} else {
@@ -171,19 +169,26 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod
 		}
 	}
 
-	if !self.getWorkingTreeState().Rebasing {
-		// not in rebase mode so return original commits
-		return result, nil
+	workingTreeState := self.getWorkingTreeState()
+	addConflictedRebasingCommit := true
+	if workingTreeState.CherryPicking || workingTreeState.Reverting {
+		sequencerCommits, err := self.getHydratedSequencerCommits(workingTreeState)
+		if err != nil {
+			return nil, err
+		}
+		result = append(sequencerCommits, result...)
+		addConflictedRebasingCommit = false
 	}
 
-	rebasingCommits, err := self.getHydratedRebasingCommits()
-	if err != nil {
-		return nil, err
-	}
-	if len(rebasingCommits) > 0 {
-		result = append(rebasingCommits, result...)
+	if workingTreeState.Rebasing {
+		rebasingCommits, err := self.getHydratedRebasingCommits(addConflictedRebasingCommit)
+		if err != nil {
+			return nil, err
+		}
+		if len(rebasingCommits) > 0 {
+			result = append(rebasingCommits, result...)
+		}
 	}
-
 	return result, nil
 }
 
@@ -242,14 +247,36 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool
 	}
 }
 
-func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) {
-	commits := self.getRebasingCommits()
+func (self *CommitLoader) getHydratedRebasingCommits(addConflictingCommit bool) ([]*models.Commit, error) {
+	todoFileHasShortHashes := self.version.IsOlderThan(2, 25, 2)
+	return self.getHydratedTodoCommits(self.getRebasingCommits(addConflictingCommit), todoFileHasShortHashes)
+}
 
-	if len(commits) == 0 {
+func (self *CommitLoader) getHydratedSequencerCommits(workingTreeState models.WorkingTreeState) ([]*models.Commit, error) {
+	commits := self.getSequencerCommits()
+	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
+	} 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
+		// our conflicting commit, so synthesize it here.
+		conflicedCommit := self.getConflictedSequencerCommit(workingTreeState)
+		if conflicedCommit != nil {
+			commits = append(commits, conflicedCommit)
+		}
+	}
+
+	return self.getHydratedTodoCommits(commits, true)
+}
+
+func (self *CommitLoader) getHydratedTodoCommits(todoCommits []*models.Commit, todoFileHasShortHashes bool) ([]*models.Commit, error) {
+	if len(todoCommits) == 0 {
 		return nil, nil
 	}
 
-	commitHashes := lo.FilterMap(commits, func(commit *models.Commit, _ int) (string, bool) {
+	commitHashes := lo.FilterMap(todoCommits, func(commit *models.Commit, _ int) (string, bool) {
 		return commit.Hash, commit.Hash != ""
 	})
 
@@ -273,7 +300,7 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error)
 		return nil, err
 	}
 
-	findFullCommit := lo.Ternary(self.version.IsOlderThan(2, 25, 2),
+	findFullCommit := lo.Ternary(todoFileHasShortHashes,
 		func(hash string) *models.Commit {
 			for s, c := range fullCommits {
 				if strings.HasPrefix(s, hash) {
@@ -286,8 +313,8 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error)
 			return fullCommits[hash]
 		})
 
-	hydratedCommits := make([]*models.Commit, 0, len(commits))
-	for _, rebasingCommit := range commits {
+	hydratedCommits := make([]*models.Commit, 0, len(todoCommits))
+	for _, rebasingCommit := range todoCommits {
 		if rebasingCommit.Hash == "" {
 			hydratedCommits = append(hydratedCommits, rebasingCommit)
 		} else if commit := findFullCommit(rebasingCommit.Hash); commit != nil {
@@ -304,7 +331,7 @@ func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error)
 // git-rebase-todo example:
 // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae
 // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931
-func (self *CommitLoader) getRebasingCommits() []*models.Commit {
+func (self *CommitLoader) getRebasingCommits(addConflictingCommit bool) []*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()))
@@ -322,13 +349,10 @@ func (self *CommitLoader) getRebasingCommits() []*models.Commit {
 
 	// See if the current commit couldn't be applied because it conflicted; if
 	// so, add a fake entry for it
-	if conflictedCommitHash := self.getConflictedCommit(todos); conflictedCommitHash != "" {
-		commits = append(commits, &models.Commit{
-			Hash:   conflictedCommitHash,
-			Name:   "",
-			Status: models.StatusRebasing,
-			Action: models.ActionConflict,
-		})
+	if addConflictingCommit {
+		if conflictedCommit := self.getConflictedCommit(todos); conflictedCommit != nil {
+			commits = append(commits, conflictedCommit)
+		}
 	}
 
 	for _, t := range todos {
@@ -351,36 +375,34 @@ func (self *CommitLoader) getRebasingCommits() []*models.Commit {
 	return commits
 }
 
-func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) string {
+func (self *CommitLoader) getConflictedCommit(todos []todo.Todo) *models.Commit {
 	bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/done"))
 	if err != nil {
 		self.Log.Error(fmt.Sprintf("error occurred reading rebase-merge/done: %s", err.Error()))
-		return ""
+		return nil
 	}
 
 	doneTodos, err := todo.Parse(bytes.NewBuffer(bytesContent), self.config.GetCoreCommentChar())
 	if err != nil {
 		self.Log.Error(fmt.Sprintf("error occurred while parsing rebase-merge/done file: %s", err.Error()))
-		return ""
+		return nil
 	}
 
-	amendFileExists := false
-	if _, err := os.Stat(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/amend")); err == nil {
-		amendFileExists = true
-	}
+	amendFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/amend"))
+	messageFileExists, _ := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/message"))
 
-	return self.getConflictedCommitImpl(todos, doneTodos, amendFileExists)
+	return self.getConflictedCommitImpl(todos, doneTodos, amendFileExists, messageFileExists)
 }
 
-func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool) string {
+func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos []todo.Todo, amendFileExists bool, messageFileExists bool) *models.Commit {
 	// Should never be possible, but just to be safe:
 	if len(doneTodos) == 0 {
 		self.Log.Error("no done entries in rebase-merge/done file")
-		return ""
+		return nil
 	}
 	lastTodo := doneTodos[len(doneTodos)-1]
 	if lastTodo.Command == todo.Break || lastTodo.Command == todo.Exec || lastTodo.Command == todo.Reword {
-		return ""
+		return nil
 	}
 
 	// In certain cases, git reschedules commands that failed. One example is if
@@ -391,7 +413,7 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [
 	// same, the command was rescheduled.
 	if len(doneTodos) > 0 && len(todos) > 0 && doneTodos[len(doneTodos)-1] == todos[0] {
 		// Command was rescheduled, no need to display it
-		return ""
+		return nil
 	}
 
 	// Older versions of git have a bug whereby, if a command is rescheduled,
@@ -416,26 +438,99 @@ func (self *CommitLoader) getConflictedCommitImpl(todos []todo.Todo, doneTodos [
 	if len(doneTodos) >= 3 && len(todos) > 0 && doneTodos[len(doneTodos)-2] == todos[0] &&
 		doneTodos[len(doneTodos)-1] == doneTodos[len(doneTodos)-3] {
 		// Command was rescheduled, no need to display it
-		return ""
+		return nil
 	}
 
 	if lastTodo.Command == todo.Edit {
 		if amendFileExists {
 			// Special case for "edit": if the "amend" file exists, the "edit"
 			// command was successful, otherwise it wasn't
-			return ""
+			return nil
+		}
+
+		if !messageFileExists {
+			// As an additional check, see if the "message" file exists; if it
+			// doesn't, it must be because a multi-commit cherry-pick or revert
+			// was performed in the meantime, which deleted both the amend file
+			// and the message file.
+			return nil
 		}
 	}
 
 	// I don't think this is ever possible, but again, just to be safe:
 	if lastTodo.Commit == "" {
 		self.Log.Error("last command in rebase-merge/done file doesn't have a commit")
-		return ""
+		return nil
 	}
 
 	// Any other todo that has a commit associated with it must have failed with
 	// a conflict, otherwise we wouldn't have stopped the rebase:
-	return lastTodo.Commit
+	return &models.Commit{
+		Hash:   lastTodo.Commit,
+		Action: lastTodo.Command,
+		Status: models.StatusConflicted,
+	}
+}
+
+func (self *CommitLoader) getSequencerCommits() []*models.Commit {
+	bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "sequencer/todo"))
+	if err != nil {
+		self.Log.Error(fmt.Sprintf("error occurred reading sequencer/todo: %s", err.Error()))
+		// we assume an error means the file doesn't exist so we just return
+		return nil
+	}
+
+	commits := []*models.Commit{}
+
+	todos, err := todo.Parse(bytes.NewBuffer(bytesContent), self.config.GetCoreCommentChar())
+	if err != nil {
+		self.Log.Error(fmt.Sprintf("error occurred while parsing sequencer/todo file: %s", err.Error()))
+		return nil
+	}
+
+	for _, t := range todos {
+		if t.Commit == "" {
+			// Command does not have a commit associated, skip
+			continue
+		}
+		commits = utils.Prepend(commits, &models.Commit{
+			Hash:   t.Commit,
+			Name:   t.Msg,
+			Status: models.StatusRebasing,
+			Action: t.Command,
+		})
+	}
+
+	return commits
+}
+
+func (self *CommitLoader) getConflictedSequencerCommit(workingTreeState models.WorkingTreeState) *models.Commit {
+	var shaFile string
+	var action todo.TodoCommand
+	if workingTreeState.CherryPicking {
+		shaFile = "CHERRY_PICK_HEAD"
+		action = todo.Pick
+	} else if workingTreeState.Reverting {
+		shaFile = "REVERT_HEAD"
+		action = todo.Revert
+	} else {
+		return nil
+	}
+	bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), shaFile))
+	if err != nil {
+		self.Log.Error(fmt.Sprintf("error occurred reading %s: %s", shaFile, err.Error()))
+		// we assume an error means the file doesn't exist so we just return
+		return nil
+	}
+	lines := strings.Split(string(bytesContent), "\n")
+	if len(lines) == 0 {
+		return nil
+	}
+	return &models.Commit{
+		Hash:   lines[0],
+		Status: models.StatusConflicted,
+		Action: action,
+	}
 }
 
 func setCommitMergedStatuses(ancestor string, commits []*models.Commit) {