Use "git cherry-pick" for implementing copy/paste of commits (#4443)

- **PR Description**

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

With this PR we reimplement copy/paste of commits to use `git
cherry-pick` under the hood. We do this because
- it's closer to what you would do on the command line
- it simplifies the code a bit
- it allows us to support cherry-picking merge commits.

Fixes #1374
Fixes #3317

Author: stefanhaller

pkg/app/daemon/daemon.go

@@ -8,7 +8,6 @@ import (
 	"os/exec"
 	"strconv"
 
-	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/common"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/samber/lo"
@@ -33,7 +32,6 @@ const (
 
 	DaemonKindExitImmediately
 	DaemonKindRemoveUpdateRefsForCopiedBranch
-	DaemonKindCherryPick
 	DaemonKindMoveTodosUp
 	DaemonKindMoveTodosDown
 	DaemonKindInsertBreak
@@ -56,7 +54,6 @@ func getInstruction() Instruction {
 	mapping := map[DaemonKind]func(string) Instruction{
 		DaemonKindExitImmediately:                 deserializeInstruction[*ExitImmediatelyInstruction],
 		DaemonKindRemoveUpdateRefsForCopiedBranch: deserializeInstruction[*RemoveUpdateRefsForCopiedBranchInstruction],
-		DaemonKindCherryPick:                      deserializeInstruction[*CherryPickCommitsInstruction],
 		DaemonKindChangeTodoActions:               deserializeInstruction[*ChangeTodoActionsInstruction],
 		DaemonKindDropMergeCommit:                 deserializeInstruction[*DropMergeCommitInstruction],
 		DaemonKindMoveFixupCommitDown:             deserializeInstruction[*MoveFixupCommitDownInstruction],
@@ -180,39 +177,6 @@ func NewRemoveUpdateRefsForCopiedBranchInstruction() Instruction {
 	return &RemoveUpdateRefsForCopiedBranchInstruction{}
 }
 
-type CherryPickCommitsInstruction struct {
-	Todo string
-}
-
-func NewCherryPickCommitsInstruction(commits []*models.Commit) Instruction {
-	todoLines := lo.Map(commits, func(commit *models.Commit, _ int) TodoLine {
-		return TodoLine{
-			Action: "pick",
-			Commit: commit,
-		}
-	})
-
-	todo := TodoLinesToString(todoLines)
-
-	return &CherryPickCommitsInstruction{
-		Todo: todo,
-	}
-}
-
-func (self *CherryPickCommitsInstruction) Kind() DaemonKind {
-	return DaemonKindCherryPick
-}
-
-func (self *CherryPickCommitsInstruction) SerializedInstructions() string {
-	return serializeInstruction(self)
-}
-
-func (self *CherryPickCommitsInstruction) run(common *common.Common) error {
-	return handleInteractiveRebase(common, func(path string) error {
-		return utils.PrependStrToTodoFile(path, []byte(self.Todo))
-	})
-}
-
 type ChangeTodoActionsInstruction struct {
 	Changes []ChangeTodoAction
 }

pkg/commands/git_commands/rebase.go

@@ -533,35 +533,15 @@ func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, comm
 
 // CherryPickCommits begins an interactive rebase with the given hashes being cherry picked onto HEAD
 func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error {
-	commitLines := lo.Map(commits, func(commit *models.Commit, _ int) string {
-		return fmt.Sprintf("%s %s", utils.ShortHash(commit.Hash), commit.Name)
-	})
-	msg := utils.ResolvePlaceholderString(
-		self.Tr.Log.CherryPickCommits,
-		map[string]string{
-			"commitLines": strings.Join(commitLines, "\n"),
-		},
-	)
-	self.os.LogCommand(msg, false)
-
-	return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
-		baseHashOrRoot: "HEAD",
-		instruction:    daemon.NewCherryPickCommitsInstruction(commits),
-	}).Run()
-}
-
-// CherryPickCommitsDuringRebase simply prepends the given commits to the existing git-rebase-todo file
-func (self *RebaseCommands) CherryPickCommitsDuringRebase(commits []*models.Commit) error {
-	todoLines := lo.Map(commits, func(commit *models.Commit, _ int) daemon.TodoLine {
-		return daemon.TodoLine{
-			Action: "pick",
-			Commit: commit,
-		}
-	})
+	hasMergeCommit := lo.SomeBy(commits, func(c *models.Commit) bool { return c.IsMerge() })
+	cmdArgs := NewGitCmd("cherry-pick").
+		Arg("--allow-empty").
+		ArgIf(self.version.IsAtLeast(2, 45, 0), "--empty=keep", "--keep-redundant-commits").
+		ArgIf(hasMergeCommit, "-m1").
+		Arg(lo.Reverse(lo.Map(commits, func(c *models.Commit, _ int) string { return c.Hash }))...).
+		ToArgv()
 
-	todo := daemon.TodoLinesToString(todoLines)
-	filePath := filepath.Join(self.repoPaths.worktreeGitDirPath, "rebase-merge/git-rebase-todo")
-	return utils.PrependStrToTodoFile(filePath, []byte(todo))
+	return self.cmd.New(cmdArgs).Run()
 }
 
 func (self *RebaseCommands) DropMergeCommit(commits []*models.Commit, commitIndex int) error {

pkg/gui/controllers/basic_commits_controller.go

@@ -366,10 +366,6 @@ func (self *BasicCommitsController) canCopyCommits(selectedCommits []*models.Com
 		if commit.Hash == "" {
 			return &types.DisabledReason{Text: self.c.Tr.CannotCherryPickNonCommit, ShowErrorInPanel: true}
 		}
-
-		if commit.IsMerge() {
-			return &types.DisabledReason{Text: self.c.Tr.CannotCherryPickMergeCommit, ShowErrorInPanel: true}
-		}
 	}
 
 	return nil

pkg/gui/controllers/helpers/cherry_pick_helper.go

@@ -77,41 +77,23 @@ func (self *CherryPickHelper) Paste() error {
 				"numCommits": strconv.Itoa(len(self.getData().CherryPickedCommits)),
 			}),
 		HandleConfirm: func() error {
-			isInRebase, err := self.c.Git().Status.IsInRebase()
-			if err != nil {
-				return err
-			}
-			if isInRebase {
-				if err := self.c.Git().Rebase.CherryPickCommitsDuringRebase(self.getData().CherryPickedCommits); err != nil {
-					return err
-				}
-				err = self.c.Refresh(types.RefreshOptions{
-					Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS},
-				})
-				if err != nil {
-					return err
-				}
-
-				return self.Reset()
-			}
-
 			return self.c.WithWaitingStatus(self.c.Tr.CherryPickingStatus, func(gocui.Task) error {
 				self.c.LogAction(self.c.Tr.Actions.CherryPick)
-				err := self.c.Git().Rebase.CherryPickCommits(self.getData().CherryPickedCommits)
-				err = self.rebaseHelper.CheckMergeOrRebase(err)
+				result := self.c.Git().Rebase.CherryPickCommits(self.getData().CherryPickedCommits)
+				err := self.rebaseHelper.CheckMergeOrRebase(result)
 				if err != nil {
-					return err
+					return result
 				}
 
-				// If we're in an interactive rebase at this point, it must
+				// If we're in the cherry-picking state at this point, it must
 				// be because there were conflicts. Don't clear the copied
-				// commits in this case, since we might want to abort and
-				// try pasting them again.
-				isInRebase, err = self.c.Git().Status.IsInRebase()
-				if err != nil {
-					return err
+				// commits in this case, since we might want to abort and try
+				// pasting them again.
+				isInCherryPick, result := self.c.Git().Status.IsInCherryPick()
+				if result != nil {
+					return result
 				}
-				if !isInRebase {
+				if !isInCherryPick {
 					self.getData().DidPaste = true
 					self.rerender()
 				}

pkg/gui/modes/cherrypicking/cherry_picking.go

@@ -59,11 +59,7 @@ func (self *CherryPicking) Remove(selectedCommit *models.Commit, commitsList []*
 }
 
 func (self *CherryPicking) update(selectedHashSet *set.Set[string], commitsList []*models.Commit) {
-	cherryPickedCommits := lo.Filter(commitsList, func(commit *models.Commit, _ int) bool {
+	self.CherryPickedCommits = lo.Filter(commitsList, func(commit *models.Commit, _ int) bool {
 		return selectedHashSet.Includes(commit.Hash)
 	})
-
-	self.CherryPickedCommits = lo.Map(cherryPickedCommits, func(commit *models.Commit, _ int) *models.Commit {
-		return &models.Commit{Name: commit.Name, Hash: commit.Hash}
-	})
 }

pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go

@@ -69,7 +69,7 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{
 			SelectNextItem().
 			PressPrimaryAction()
 
-		t.Common().ContinueOnConflictsResolved("rebase")
+		t.Common().ContinueOnConflictsResolved("cherry-pick")
 
 		t.Views().Files().IsEmpty()
 

pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go

@@ -75,8 +75,8 @@ var CherryPickDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
 			}).
 			Lines(
 				Contains("pick  CI two"),
-				Contains("pick  CI three"),
-				Contains("      CI <-- YOU ARE HERE --- one"),
+				Contains("      CI <-- YOU ARE HERE --- three"),
+				Contains("      CI one"),
 				Contains("      CI base"),
 			).
 			Tap(func() {

pkg/integration/tests/cherry_pick/cherry_pick_merge.go

@@ -0,0 +1,80 @@
+package cherry_pick
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CherryPickMerge = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Cherry pick a merge commit",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			EmptyCommit("base").
+			NewBranch("first-branch").
+			NewBranch("second-branch").
+			Checkout("first-branch").
+			Checkout("second-branch").
+			CreateFileAndAdd("file1.txt", "content").
+			Commit("one").
+			CreateFileAndAdd("file2.txt", "content").
+			Commit("two").
+			Checkout("master").
+			Merge("second-branch").
+			Checkout("first-branch")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Branches().
+			Focus().
+			Lines(
+				Contains("first-branch"),
+				Contains("master"),
+				Contains("second-branch"),
+			).
+			SelectNextItem().
+			PressEnter()
+
+		t.Views().SubCommits().
+			IsFocused().
+			Lines(
+				Contains("⏣─╮ Merge branch 'second-branch'").IsSelected(),
+				Contains("│ ◯ two"),
+				Contains("│ ◯ one"),
+				Contains("◯ ╯ base"),
+			).
+			// copy the merge commit
+			Press(keys.Commits.CherryPickCopy)
+
+		t.Views().Information().Content(Contains("1 commit copied"))
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("base").IsSelected(),
+			).
+			Press(keys.Commits.PasteCommits).
+			Tap(func() {
+				t.ExpectPopup().Alert().
+					Title(Equals("Cherry-pick")).
+					Content(Contains("Are you sure you want to cherry-pick the 1 copied commit(s) onto this branch?")).
+					Confirm()
+			}).
+			Tap(func() {
+				t.Views().Information().Content(DoesNotContain("commit copied"))
+			}).
+			Lines(
+				Contains("Merge branch 'second-branch'").IsSelected(),
+				Contains("base"),
+			)
+
+		t.Views().Main().ContainsLines(
+			Contains("Merge branch 'second-branch'"),
+			Contains("---"),
+			Contains("file1.txt | 1 +"),
+			Contains("file2.txt | 1 +"),
+			Contains("2 files changed, 2 insertions(+)"),
+		)
+	},
+})

pkg/integration/tests/test_list.go

@@ -82,6 +82,7 @@ var tests = []*components.IntegrationTest{
 	cherry_pick.CherryPick,
 	cherry_pick.CherryPickConflicts,
 	cherry_pick.CherryPickDuringRebase,
+	cherry_pick.CherryPickMerge,
 	cherry_pick.CherryPickRange,
 	commit.AddCoAuthor,
 	commit.AddCoAuthorRange,