Ignore fixup commits for a found base commit when doing ctrl-f

pkg/gui/controllers/helpers/fixup_helper.go

@@ -110,20 +110,24 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 		return errors.New(self.c.Tr.BaseCommitIsAlreadyOnMainBranch)
 	}
 
-	if len(hashGroups[NOT_MERGED]) > 1 {
-		// If there are multiple commits that could be the base commit, list
+	foundCommits := getCommitsForHashes(commits, hashGroups[NOT_MERGED])
+	// If there are multiple commits that could be the base commit, remove all
+	// those that are fixups for the last one.
+	foundCommits = removeFixupCommits(foundCommits)
+
+	if len(foundCommits) > 1 {
+		// If there are still multiple commits that could be the base commit, list
 		// them in the error message. But only the candidates from the current
 		// branch, not including any that are already merged.
-		subjects := getHashesAndSubjects(commits, hashGroups[NOT_MERGED])
+		subjects := getHashesAndSubjects(foundCommits)
 		message := lo.Ternary(hasStagedChanges,
 			self.c.Tr.MultipleBaseCommitsFoundStaged,
 			self.c.Tr.MultipleBaseCommitsFoundUnstaged)
 		return fmt.Errorf("%s\n\n%s", message, subjects)
 	}
 
-	// At this point we know that the NOT_MERGED bucket has exactly one commit,
-	// and that's the one we want to select.
-	_, index, _ := self.findCommit(commits, hashGroups[NOT_MERGED][0])
+	// Now we know that foundCommits has exactly one commit, so find its index
+	_, index, _ := self.findCommit(commits, foundCommits[0].Hash())
 
 	return self.c.ConfirmIf(warnAboutAddedLines, types.ConfirmOpts{
 		Title:  self.c.Tr.FindBaseCommitForFixup,
@@ -161,13 +165,35 @@ func getCommitsForHashes(commits []*models.Commit, hashes []string) []*models.Co
 	return result
 }
 
-func getHashesAndSubjects(commits []*models.Commit, hashes []string) string {
-	subjects := lo.Map(getCommitsForHashes(commits, hashes), func(c *models.Commit, _ int) string {
+func getHashesAndSubjects(commits []*models.Commit) string {
+	subjects := lo.Map(commits, func(c *models.Commit, _ int) string {
 		return fmt.Sprintf("%s %s", c.ShortRefName(), c.Name)
 	})
 	return strings.Join(subjects, "\n")
 }
 
+func removeFixupCommits(commits []*models.Commit) []*models.Commit {
+	if len(commits) <= 1 {
+		return commits
+	}
+
+	// If the last found commit is itself a fixup, don't eliminate anything.
+	baseSubject, lastIsFixup := IsFixupCommit(commits[len(commits)-1].Name)
+	if lastIsFixup {
+		return commits
+	}
+
+	// need to go backwards because we are mutating the slice as we go
+	for i := len(commits) - 2; i >= 0; i-- {
+		subject, isFixup := IsFixupCommit(commits[i].Name)
+		if isFixup && subject == baseSubject {
+			commits = utils.Remove(commits, i)
+		}
+	}
+
+	return commits
+}
+
 func (self *FixupHelper) getDiff() (string, bool, error) {
 	args := []string{"-U0", "--ignore-submodules=all", "HEAD", "--"}
 

pkg/gui/controllers/helpers/fixup_helper_test.go

@@ -3,6 +3,9 @@ package helpers
 import (
 	"testing"
 
+	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/utils"
+	"github.com/samber/lo"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -201,3 +204,138 @@ func TestFixupHelper_IsFixupCommit(t *testing.T) {
 		})
 	}
 }
+
+func TestFixupHelper_removeFixupCommits(t *testing.T) {
+	hashPool := &utils.StringPool{}
+
+	type commitDesc struct {
+		Hash string
+		Name string
+	}
+
+	scenarios := []struct {
+		name           string
+		commits        []commitDesc
+		expectedResult []commitDesc
+	}{
+		{
+			name:           "empty list",
+			commits:        []commitDesc{},
+			expectedResult: []commitDesc{},
+		},
+		{
+			name: "single commit",
+			commits: []commitDesc{
+				{"abc123", "Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"abc123", "Some feature"},
+			},
+		},
+		{
+			name: "two unrelated commits",
+			commits: []commitDesc{
+				{"abc123", "First feature"},
+				{"def456", "Second feature"},
+			},
+			expectedResult: []commitDesc{
+				{"abc123", "First feature"},
+				{"def456", "Second feature"},
+			},
+		},
+		{
+			name: "fixup commit for last commit",
+			commits: []commitDesc{
+				{"abc123", "fixup! Some feature"},
+				{"def456", "Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"def456", "Some feature"},
+			},
+		},
+		{
+			name: "amend and squash commits for last commit",
+			commits: []commitDesc{
+				{"abc123", "squash! Some feature"},
+				{"def456", "amend! Some feature"},
+				{"ghi789", "Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"ghi789", "Some feature"},
+			},
+		},
+		{
+			name: "fixup commit for different commit",
+			commits: []commitDesc{
+				{"abc123", "fixup! Other feature"},
+				{"def456", "Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"abc123", "fixup! Other feature"},
+				{"def456", "Some feature"},
+			},
+		},
+		{
+			name: "last commit is a fixup itself",
+			commits: []commitDesc{
+				{"abc123", "fixup! Some feature"},
+				{"def456", "fixup! Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"abc123", "fixup! Some feature"},
+				{"def456", "fixup! Some feature"},
+			},
+		},
+		{
+			name: "nested fixup commit",
+			commits: []commitDesc{
+				{"abc123", "fixup! fixup! Some feature"},
+				{"def456", "amend! squash! fixup! Some feature"},
+				{"ghi789", "Some feature"},
+			},
+			expectedResult: []commitDesc{
+				{"ghi789", "Some feature"},
+			},
+		},
+		{
+			name: "fixup commits mixed with unrelated commits",
+			commits: []commitDesc{
+				{Hash: "abc123", Name: "fixup! Base commit"},
+				{Hash: "def456", Name: "Unrelated commit"},
+				{Hash: "ghi789", Name: "fixup! Base commit"},
+				{Hash: "jkl012", Name: "Base commit"},
+			},
+			expectedResult: []commitDesc{
+				{Hash: "def456", Name: "Unrelated commit"},
+				{Hash: "jkl012", Name: "Base commit"},
+			},
+		},
+		{
+			name: "only fixup commits for last commit removed, others preserved",
+			commits: []commitDesc{
+				{Hash: "abc123", Name: "fixup! First feature"},
+				{Hash: "def456", Name: "fixup! Second feature"},
+				{Hash: "ghi789", Name: "Second feature"},
+				{Hash: "jkl012", Name: "First feature"},
+			},
+			expectedResult: []commitDesc{
+				{Hash: "def456", Name: "fixup! Second feature"},
+				{Hash: "ghi789", Name: "Second feature"},
+				{Hash: "jkl012", Name: "First feature"},
+			},
+		},
+	}
+
+	makeCommitFromDesc := func(desc commitDesc, _ int) *models.Commit {
+		return models.NewCommit(hashPool, models.NewCommitOpts{Hash: desc.Hash, Name: desc.Name})
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.name, func(t *testing.T) {
+			commits := lo.Map(s.commits, makeCommitFromDesc)
+			result := removeFixupCommits(commits)
+			expectedCommits := lo.Map(s.expectedResult, makeCommitFromDesc)
+			assert.Equal(t, expectedCommits, result)
+		})
+	}
+}

pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go

@@ -34,13 +34,13 @@ var FindBaseCommitForFixupDisregardFixupsForSameBaseCommit = NewIntegrationTest(
 			Focus().
 			Press(keys.Files.FindBaseCommitForFixup)
 
-		t.ExpectPopup().Alert().
-			Title(Equals("Error")).
-			Content(
-				MatchesRegexp("Multiple base commits found.*\n\n" +
-					".*fixup! 2nd commit\n" +
-					".*2nd commit"),
-			).
-			Confirm()
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains("3rd commit"),
+				Contains("fixup! 2nd commit"),
+				Contains("2nd commit").IsSelected(),
+				Contains("1st commit"),
+			)
 	},
 })