Fix lazygit getting unresponsive when pasting commits that become empty (#4958)

Lazygit would become unresponsive when cherry-picking a commit that
becomes empty at the destination. There are two ways this can happen:
one where a cherry-picked commit simply becomes empty "by itself",
because the change is already present on the destination branch (this
was only a problem with git versions older than 2.45), and another where
the cherry-pick stops with conflicts, and the user resolves them such
that no changes are left, and then continues the cherry-pick. This would
still fail even with git 2.45 or later. The fix is the same for both
cases though.

Fixes #4763.

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -157,7 +157,7 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result er
 	} else if strings.Contains(result.Error(), "No changes - did you forget to use") {
 		return self.genericMergeCommand(REBASE_OPTION_SKIP)
 	} else if strings.Contains(result.Error(), "The previous cherry-pick is now empty") {
-		return self.genericMergeCommand(REBASE_OPTION_CONTINUE)
+		return self.genericMergeCommand(REBASE_OPTION_SKIP)
 	} else if strings.Contains(result.Error(), "No rebase in progress?") {
 		// assume in this case that we're already done
 		return nil

pkg/integration/tests/cherry_pick/cherry_pick_commit_that_becomes_empty.go

@@ -0,0 +1,99 @@
+package cherry_pick
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CherryPickCommitThatBecomesEmpty = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Cherry-pick a commit that becomes empty at the destination",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			EmptyCommit("base").
+			CreateFileAndAdd("file1", "change 1\n").
+			CreateFileAndAdd("file2", "change 2\n").
+			Commit("two changes in one commit").
+			NewBranchFrom("branch", "HEAD^").
+			CreateFileAndAdd("file1", "change 1\n").
+			Commit("single change").
+			CreateFileAndAdd("file3", "change 3\n").
+			Commit("unrelated change").
+			Checkout("master")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Branches().
+			Focus().
+			Lines(
+				Contains("master").IsSelected(),
+				Contains("branch"),
+			).
+			SelectNextItem().
+			PressEnter()
+
+		t.Views().SubCommits().
+			IsFocused().
+			Lines(
+				Contains("unrelated change").IsSelected(),
+				Contains("single change"),
+				Contains("base"),
+			).
+			Press(keys.Universal.RangeSelectDown).
+			Press(keys.Commits.CherryPickCopy).
+			Tap(func() {
+				t.Views().Information().Content(Contains("2 commits copied"))
+			})
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("two changes in one commit").IsSelected(),
+				Contains("base"),
+			).
+			Press(keys.Commits.PasteCommits).
+			Tap(func() {
+				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()
+			})
+
+		if t.Git().Version().IsAtLeast(2, 45, 0) {
+			t.Views().Commits().
+				Lines(
+					Contains("unrelated change"),
+					Contains("single change"),
+					Contains("two changes in one commit").IsSelected(),
+					Contains("base"),
+				).
+				SelectPreviousItem()
+
+			// Cherry-picked commit is empty
+			t.Views().Main().Content(DoesNotContain("diff --git"))
+		} else {
+			t.Views().Commits().
+				// We have a bug with how the selection is updated in this case; normally you would
+				// expect the "two changes in one commit" commit to be selected because it was
+				// selected before pasting, and we try to maintain that selection. This is broken
+				// for two reasons:
+				// 1. We increment the selected line index after pasting by the number of pasted
+				// commits; this is wrong because we skipped the commit that became empty. So
+				// according to this bug, the "base" commit should be selected.
+				// 2. We only update the selected line index after pasting if the currently selected
+				// commit is not a rebase TODO commit, on the assumption that if it is, we are in a
+				// rebase and the cherry-picked commits end up below the selection. In this case,
+				// however, we still think we are cherry-picking because the final refresh after the
+				// CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet;
+				// so the "unrelated change" still has a "pick" action.
+				//
+				// Since this only happens for older git versions, we don't bother fixing it.
+				Lines(
+					Contains("unrelated change").IsSelected(),
+					Contains("two changes in one commit"),
+					Contains("base"),
+				)
+		}
+	},
+})

pkg/integration/tests/cherry_pick/cherry_pick_conflicts_empty_commit_after_resolving.go

@@ -0,0 +1,92 @@
+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 CherryPickConflictsEmptyCommitAfterResolving = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Cherry pick commits with conflicts, resolve them so that the commit becomes empty",
+	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.Universal.RangeSelectDown).
+			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.Views().Files().
+			IsFocused().
+			SelectedLine(Contains("file")).
+			Press(keys.Universal.Remove)
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Discard changes")).
+			Select(Contains("Discard all changes")).
+			Confirm()
+
+		t.Common().ContinueOnConflictsResolved("cherry-pick")
+
+		t.Views().Files().IsEmpty()
+
+		t.Views().Commits().
+			Focus().
+			TopLines(
+				// We have a bug with how the selection is updated in this case; normally you would
+				// expect the "first change" commit to be selected because it was selected before
+				// pasting, and we try to maintain that selection. This is broken for two reasons:
+				// 1. We increment the selected line index after pasting by the number of pasted
+				// commits; this is wrong because we skipped the commit that became empty. So
+				// according to this bug, the "original" commit should be selected.
+				// 2. We only update the selected line index after pasting if the currently selected
+				// commit is not a rebase TODO commit, on the assumption that if it is, we are in a
+				// rebase and the cherry-picked commits end up below the selection. In this case,
+				// however, we still think we are cherry-picking because the final refresh after the
+				// CheckMergeOrRebase in CherryPickHelper.Paste is async and hasn't completed yet;
+				// so the "second-change-branch unrelated change" still has a "pick" action.
+				//
+				// We don't bother fixing it for now because it's a pretty niche case, and the
+				// nature of the problem is only cosmetic.
+				Contains("second-change-branch unrelated change").IsSelected(),
+				Contains("first change"),
+				Contains("original"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -86,7 +86,9 @@ var tests = []*components.IntegrationTest{
 	branch.Suggestions,
 	branch.UnsetUpstream,
 	cherry_pick.CherryPick,
+	cherry_pick.CherryPickCommitThatBecomesEmpty,
 	cherry_pick.CherryPickConflicts,
+	cherry_pick.CherryPickConflictsEmptyCommitAfterResolving,
 	cherry_pick.CherryPickDuringRebase,
 	cherry_pick.CherryPickMerge,
 	cherry_pick.CherryPickRange,