feat: enhance cherry-pick functionality with pre-paste head hash tracking and add integration tests for empty intermediate commits

Author: kosiew

pkg/gui/controllers/helpers/cherry_pick_helper.go

@@ -2,7 +2,9 @@ package helpers
 
 import (
 	"strconv"
+	"strings"
 
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
 	"github.com/jesseduffield/lazygit/pkg/gui/modes/cherrypicking"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
@@ -19,6 +21,11 @@ type CherryPickHelper struct {
 	postPasteShouldMarkDidPaste bool
 
 	postPasteSelection *postPasteSelection
+
+	prePasteHeadHash     string
+	pasteProducedCommits bool
+
+	deferPostPasteCleanup bool
 }
 
 type postPasteSelection struct {
@@ -104,6 +111,7 @@ func (self *CherryPickHelper) Paste() error {
 
 				cherryPickedCommits := self.getData().CherryPickedCommits
 				self.preparePostPasteSelection(cherryPickedCommits)
+				self.capturePrePasteHeadHash()
 
 				self.setPostPasteCleanup(func(markDidPaste bool) error {
 					if markDidPaste {
@@ -131,6 +139,8 @@ func (self *CherryPickHelper) Paste() error {
 					return err
 				}
 
+				self.markPasteProducedCommitsIfHeadChanged()
+
 				// 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
@@ -163,6 +173,9 @@ func (self *CherryPickHelper) Reset() error {
 	self.postPasteCleanup = nil
 	self.postPasteShouldMarkDidPaste = false
 	self.postPasteSelection = nil
+	self.prePasteHeadHash = ""
+	self.pasteProducedCommits = false
+	self.deferPostPasteCleanup = false
 
 	self.rerender()
 	return nil
@@ -253,6 +266,9 @@ func (self *CherryPickHelper) runPostPasteCleanup(markDidPaste bool) error {
 	defer func() {
 		self.postPasteShouldMarkDidPaste = false
 		self.postPasteSelection = nil
+		self.prePasteHeadHash = ""
+		self.pasteProducedCommits = false
+		self.deferPostPasteCleanup = false
 	}()
 
 	return cleanup(markDidPaste && self.postPasteShouldMarkDidPaste)
@@ -265,3 +281,80 @@ func (self *CherryPickHelper) setPostPasteShouldMarkDidPaste(mark bool) {
 
 	self.postPasteShouldMarkDidPaste = mark
 }
+
+func (self *CherryPickHelper) capturePrePasteHeadHash() {
+	headHash, err := self.getHeadHash()
+	if err != nil {
+		self.prePasteHeadHash = ""
+		return
+	}
+
+	self.prePasteHeadHash = headHash
+	self.pasteProducedCommits = false
+}
+
+func (self *CherryPickHelper) PasteProducedCommits() bool {
+	if self.pasteProducedCommits {
+		return true
+	}
+
+	if self.prePasteHeadHash == "" {
+		return true
+	}
+
+	headHash, err := self.getHeadHash()
+	if err != nil {
+		return true
+	}
+
+	if headHash != self.prePasteHeadHash {
+		self.pasteProducedCommits = true
+		return true
+	}
+
+	return false
+}
+
+func (self *CherryPickHelper) DeferPostPasteCleanup() {
+	self.deferPostPasteCleanup = true
+}
+
+func (self *CherryPickHelper) ClearDeferredPostPasteCleanup() {
+	self.deferPostPasteCleanup = false
+}
+
+func (self *CherryPickHelper) ShouldDeferPostPasteCleanup() bool {
+	return self.deferPostPasteCleanup
+}
+
+func (self *CherryPickHelper) markPasteProducedCommitsIfHeadChanged() {
+	if self.pasteProducedCommits {
+		return
+	}
+
+	if self.prePasteHeadHash == "" {
+		self.pasteProducedCommits = true
+		return
+	}
+
+	headHash, err := self.getHeadHash()
+	if err != nil {
+		self.pasteProducedCommits = true
+		return
+	}
+
+	if headHash != self.prePasteHeadHash {
+		self.pasteProducedCommits = true
+	}
+}
+
+func (self *CherryPickHelper) getHeadHash() (string, error) {
+	output, err := self.c.OS().Cmd.New(
+		git_commands.NewGitCmd("rev-parse").Arg("HEAD").ToArgv(),
+	).DontLog().RunWithOutput()
+	if err != nil {
+		return "", err
+	}
+
+	return strings.TrimSpace(output), nil
+}

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -121,6 +121,10 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error {
 		return err
 	}
 
+	if self.cherryPickHelper != nil {
+		self.cherryPickHelper.markPasteProducedCommitsIfHeadChanged()
+	}
+
 	if err := self.finalizeCherryPickIfDone(); err != nil {
 		return err
 	}
@@ -215,7 +219,7 @@ func (self *MergeAndRebaseHelper) handleEmptyCherryPick() error {
 				OnPress: func() error {
 					if self.cherryPickHelper != nil {
 						self.cherryPickHelper.DisablePostPasteReselect()
-						self.cherryPickHelper.setPostPasteShouldMarkDidPaste(false)
+						self.cherryPickHelper.DeferPostPasteCleanup()
 					}
 					if err := self.genericMergeCommand(REBASE_OPTION_SKIP); err != nil {
 						return err
@@ -266,6 +270,8 @@ func (self *MergeAndRebaseHelper) completeCherryPickAfterEmptyResolution(preserv
 		return nil
 	}
 
+	defer self.cherryPickHelper.ClearDeferredPostPasteCleanup()
+
 	if !preservePostPasteReselect {
 		self.cherryPickHelper.DisablePostPasteReselect()
 	}
@@ -274,7 +280,13 @@ func (self *MergeAndRebaseHelper) completeCherryPickAfterEmptyResolution(preserv
 		return nil
 	}
 
-	if err := self.cherryPickHelper.runPostPasteCleanup(preservePostPasteReselect); err != nil {
+	self.cherryPickHelper.markPasteProducedCommitsIfHeadChanged()
+
+	if !self.cherryPickHelper.PasteProducedCommits() {
+		self.cherryPickHelper.setPostPasteShouldMarkDidPaste(false)
+	}
+
+	if err := self.cherryPickHelper.runPostPasteCleanup(true); err != nil {
 		return err
 	}
 
@@ -286,6 +298,10 @@ func (self *MergeAndRebaseHelper) finalizeCherryPickIfDone() error {
 		return nil
 	}
 
+	if self.cherryPickHelper.ShouldDeferPostPasteCleanup() {
+		return nil
+	}
+
 	hasTodos, err := self.c.Git().Status.HasPendingSequencerTodos()
 	if err != nil {
 		return err
@@ -304,6 +320,8 @@ func (self *MergeAndRebaseHelper) finalizeCherryPickIfDone() error {
 		return nil
 	}
 
+	self.cherryPickHelper.markPasteProducedCommitsIfHeadChanged()
+
 	if !self.cherryPickHelper.ShouldRestorePostPasteSelection() {
 		self.cherryPickHelper.DisablePostPasteReselect()
 	}

pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go

@@ -100,7 +100,7 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{
 					Content(Contains("-First Change")).
 					Content(Contains("+Second Change"))
 
-				t.Views().Information().Content(Contains("2 commits copied"))
+				t.Views().Information().Content(DoesNotContain("commits copied"))
 			}).
 			PressEscape().
 			Tap(func() {

pkg/integration/tests/cherry_pick/cherry_pick_range_empty_intermediate_skip.go

@@ -0,0 +1,97 @@
+package cherry_pick
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CherryPickRangeEmptyIntermediateSkip = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Cherry-picking a range with an intermediate empty commit and skipping it",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+		config.GetUserConfig().Git.LocalBranchSortOrder = "recency"
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateFileAndAdd("shared.txt", "original\n").Commit("add shared file on master").
+			NewBranch("target").
+			UpdateFileAndAdd("shared.txt", "target change\n").Commit("update shared file on target").
+			Checkout("master").
+			NewBranch("source").
+			CreateFileAndAdd("unique1.txt", "content1\n").Commit("add unique1 on source").
+			UpdateFileAndAdd("shared.txt", "target change\n").Commit("match target change").
+			CreateFileAndAdd("unique2.txt", "content2\n").Commit("add unique2 on source").
+			Checkout("target")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Branches().
+			Focus().
+			Lines(
+				Contains("target"),
+				Contains("source"),
+				Contains("master"),
+			).
+			SelectNextItem().
+			PressEnter()
+
+		t.Views().SubCommits().
+			IsFocused().
+			Lines(
+				Contains("add unique2 on source").IsSelected(),
+				Contains("match target change"),
+				Contains("add unique1 on source"),
+				Contains("add shared file on master"),
+			).
+			Press(keys.Universal.RangeSelectDown).
+			Lines(
+				Contains("add unique2 on source").IsSelected(),
+				Contains("match target change").IsSelected(),
+				Contains("add unique1 on source"),
+				Contains("add shared file on master"),
+			).
+			Press(keys.Universal.RangeSelectDown).
+			Lines(
+				Contains("add unique2 on source").IsSelected(),
+				Contains("match target change").IsSelected(),
+				Contains("add unique1 on source").IsSelected(),
+				Contains("add shared file on master"),
+			).
+			Press(keys.Commits.CherryPickCopy)
+
+		t.Views().Information().Content(Contains("3 commits copied"))
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("update shared file on target").IsSelected(),
+				Contains("add shared file on master"),
+			).
+			Press(keys.Commits.PasteCommits).
+			Tap(func() {
+				t.ExpectPopup().Alert().
+					Title(Equals("Cherry-pick")).
+					Content(Contains("Are you sure you want to cherry-pick the 3 copied commit(s) onto this branch?")).
+					Confirm()
+			}).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("Cherry-pick produced no changes")).
+					ContainsLines(
+						Contains("Skip this cherry-pick"),
+						Contains("Create empty commit and continue"),
+						Contains("Cancel"),
+					).
+					Select(Contains("Skip this cherry-pick")).
+					Confirm()
+			}).
+			Tap(func() {
+				t.Views().Information().Content(DoesNotContain("commits copied"))
+			}).
+			TopLines(
+				Contains("add unique2 on source").IsSelected(),
+				Contains("add unique1 on source"),
+				Contains("update shared file on target"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -95,6 +95,7 @@ var tests = []*components.IntegrationTest{
 	cherry_pick.CherryPickMerge,
 	cherry_pick.CherryPickRange,
 	cherry_pick.CherryPickRangeEmptyIntermediate,
+	cherry_pick.CherryPickRangeEmptyIntermediateSkip,
 	commit.AddCoAuthor,
 	commit.AddCoAuthorRange,
 	commit.AddCoAuthorWhileCommitting,