feat: Make Fixup Helper ignore fixup-commits, if multiple base commits are found.

pkg/gui/controllers/helpers/fixup_helper.go

@@ -111,37 +111,68 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 	}
 
 	if len(hashGroups[NOT_MERGED]) > 1 {
-		// If there are 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.
+		// If there are multiple commits that could be the base commit, check
+		// if there is only a single one that isn't a fixup commit itself.
+		// List the commits in the error message. But only the candidates from
+		// the current branch, not including any that are already merged.
+
+		unmerged_commits := self.getCommitsFromHashes(commits, hashGroups[NOT_MERGED])
+		non_fixup_commits := lo.Filter(unmerged_commits, func(c *models.Commit, index int) bool {
+			return !strings.HasPrefix(c.Name, "fixup! ") && !strings.HasPrefix(c.Name, "squash! ") && !strings.HasPrefix(c.Name, "amend! ")
+		})
 		subjects := self.getHashesAndSubjects(commits, hashGroups[NOT_MERGED])
-		message := lo.Ternary(hasStagedChanges,
-			self.c.Tr.MultipleBaseCommitsFoundStaged,
-			self.c.Tr.MultipleBaseCommitsFoundUnstaged)
-		return fmt.Errorf("%s\n\n%s", message, subjects)
+		if len(non_fixup_commits) > 1 || len(non_fixup_commits) == 0 {
+			message := lo.Ternary(hasStagedChanges,
+				self.c.Tr.MultipleBaseCommitsFoundStaged,
+				self.c.Tr.MultipleBaseCommitsFoundUnstaged)
+			return fmt.Errorf("%s\n\n%s", message, subjects)
+		}
+		// There is only a single non fixup commit found. Present it with a Confirmation dialog.
+		_, index, _ := self.findCommit(commits, non_fixup_commits[0].Hash())
+
+		return self.c.ConfirmIf(true, types.ConfirmOpts{
+			Title:         self.c.Tr.FindBaseCommitForFixup,
+			Prompt:        fmt.Sprintf("%s\n\n%s", self.c.Tr.MultipleBaseCommitsOnlyOneNonFixup, subjects),
+			HandleConfirm: self.getHandlerToStageAndSelectIndex(hasStagedChanges, index),
+		})
 	}
 
 	// 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])
 
 	return self.c.ConfirmIf(warnAboutAddedLines, types.ConfirmOpts{
-		Title:  self.c.Tr.FindBaseCommitForFixup,
-		Prompt: self.c.Tr.HunksWithOnlyAddedLinesWarning,
-		HandleConfirm: func() error {
-			if !hasStagedChanges {
-				if err := self.c.Git().WorkingTree.StageAll(true); err != nil {
-					return err
-				}
-				self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}})
+		Title:         self.c.Tr.FindBaseCommitForFixup,
+		Prompt:        self.c.Tr.HunksWithOnlyAddedLinesWarning,
+		HandleConfirm: self.getHandlerToStageAndSelectIndex(hasStagedChanges, index),
+	})
+}
+
+func (self *FixupHelper) getHandlerToStageAndSelectIndex(hasStagedChanges bool, index int) func() error {
+	return func() error {
+		if !hasStagedChanges {
+			if err := self.c.Git().WorkingTree.StageAll(true); err != nil {
+				return err
 			}
+			self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{types.FILES}})
+		}
 
-			self.c.Contexts().LocalCommits.SetSelection(index)
-			self.c.Contexts().LocalCommits.FocusLine(true)
-			self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{})
-			return nil
-		},
-	})
+		self.c.Contexts().LocalCommits.SetSelection(index)
+		self.c.Contexts().LocalCommits.FocusLine(true)
+		self.c.Context().Push(self.c.Contexts().LocalCommits, types.OnFocusOpts{})
+		return nil
+	}
+}
+
+func (self *FixupHelper) getCommitsFromHashes(commits []*models.Commit, hashes []string) []*models.Commit {
+	commitsSet := set.NewFromSlice(hashes)
+	found_commits := make([]*models.Commit, 0, len(hashes))
+	for _, c := range commits {
+		if commitsSet.Includes(c.Hash()) {
+			found_commits = append(found_commits, c)
+		}
+	}
+	return found_commits
 }
 
 func (self *FixupHelper) getHashesAndSubjects(commits []*models.Commit, hashes []string) string {

pkg/i18n/english.go

@@ -45,6 +45,7 @@ type TranslationSet struct {
 	NoBaseCommitsFound                    string
 	MultipleBaseCommitsFoundStaged        string
 	MultipleBaseCommitsFoundUnstaged      string
+	MultipleBaseCommitsOnlyOneNonFixup    string
 	BaseCommitIsAlreadyOnMainBranch       string
 	BaseCommitIsNotInCurrentView          string
 	HunksWithOnlyAddedLinesWarning        string
@@ -1140,6 +1141,7 @@ func EnglishTranslationSet() *TranslationSet {
 		NoBaseCommitsFound:                   "No base commits found",
 		MultipleBaseCommitsFoundStaged:       "Multiple base commits found. (Try staging fewer changes at once)",
 		MultipleBaseCommitsFoundUnstaged:     "Multiple base commits found. (Try staging some of the changes)",
+		MultipleBaseCommitsOnlyOneNonFixup:   "Multiple base commits found, however all but one of them were fixup commits",
 		BaseCommitIsAlreadyOnMainBranch:      "The base commit for this change is already on the main branch",
 		BaseCommitIsNotInCurrentView:         "Base commit is not in current view",
 		HunksWithOnlyAddedLinesWarning:       "There are ranges of only added lines in the diff; be careful to check that these belong in the found base commit.\n\nProceed?",

pkg/integration/tests/commit/find_base_commit_for_fixup_ignore_fixup_commits.go

@@ -0,0 +1,58 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var FindBaseCommitForFixupIgnoreFixupCommits = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Finds the base commit to create a fixup for, disregarding changes to a commit that is already on master",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			EmptyCommit("1st commit").
+			CreateFileAndAdd("file", "file1 content line 1\nfile1 content line 2\n").
+			Commit("2nd commit").
+			NewBranch("mybranch").
+			UpdateFileAndAdd("file", "file1 changed content line 1\nfile1 changed content line 2\n").
+			Commit("3rd commit").
+			EmptyCommit("4th commit").
+			UpdateFileAndAdd("file", "file1 1st fixup content line 1\nfile1 changed content line 2\n").
+			Commit("fixup! 3rd commit").
+			UpdateFile("file", "file1 2nd fixup content line 1\nfile1 2nd fixup content line 2\n")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Lines(
+				Contains("fixup! 3rd commit").IsSelected(),
+				Contains("4th commit"),
+				Contains("3rd commit"),
+				Contains("2nd commit"),
+				Contains("1st commit"),
+			)
+
+		t.Views().Files().
+			Focus().
+			Press(keys.Files.FindBaseCommitForFixup)
+		t.ExpectPopup().
+			Confirmation().
+			Title(Equals("Find base commit for fixup")).
+			Content(
+				Contains("all but one of them were fixup commits").
+					Contains("3rd commit").
+					Contains("fixup! 3rd commit"),
+			).
+			Confirm()
+		t.Views().Commits().
+			IsFocused().
+			Lines(
+				Contains("fixup! 3rd commit"),
+				Contains("4th commit"),
+				Contains("3rd commit").IsSelected(),
+				Contains("2nd commit"),
+				Contains("1st commit"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -131,6 +131,7 @@ var tests = []*components.IntegrationTest{
 	commit.FailHooksThenCommitNoHooks,
 	commit.FindBaseCommitForFixup,
 	commit.FindBaseCommitForFixupDisregardMainBranch,
+	commit.FindBaseCommitForFixupIgnoreFixupCommits,
 	commit.FindBaseCommitForFixupOnlyAddedLines,
 	commit.FindBaseCommitForFixupWarningForAddedLines,
 	commit.Highlight,