Disregard master commits when finding base commit for fixup

If exactly one candidate from inside the current branch is found, we return that
one even if there are also hunks belonging to master commits; we disregard those
in this case.

Author: stefanhaller

pkg/gui/controllers/helpers/fixup_helper.go

@@ -72,8 +72,49 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 		// This should never happen
 		return errors.New(self.c.Tr.NoBaseCommitsFound)
 	}
-	if len(hashes) > 1 {
-		subjects, err := self.c.Git().Commit.GetHashesAndCommitMessagesFirstLine(hashes)
+
+	// If a commit can't be found, and the last known commit is already merged,
+	// we know that the commit we're looking for is also merged. Otherwise we
+	// can't tell.
+	notFoundMeansMerged := len(commits) > 0 && commits[len(commits)-1].Status == models.StatusMerged
+
+	const (
+		MERGED int = iota
+		NOT_MERGED
+		CANNOT_TELL
+	)
+
+	// Group the hashes into buckets by merged status
+	hashGroups := lo.GroupBy(hashes, func(hash string) int {
+		commit, _, ok := self.findCommit(commits, hash)
+		if ok {
+			return lo.Ternary(commit.Status == models.StatusMerged, MERGED, NOT_MERGED)
+		}
+		return lo.Ternary(notFoundMeansMerged, MERGED, CANNOT_TELL)
+	})
+
+	if len(hashGroups[CANNOT_TELL]) > 0 {
+		// If we have any commits that we can't tell if they're merged, just
+		// show the generic "not in current view" error. This can only happen if
+		// a feature branch has more than 300 commits, or there is no main
+		// branch. Both are so unlikely that we don't bother returning a more
+		// detailed error message (e.g. we could say something about the commits
+		// that *are* in the current branch, but it's not worth it).
+		return errors.New(self.c.Tr.BaseCommitIsNotInCurrentView)
+	}
+
+	if len(hashGroups[NOT_MERGED]) == 0 {
+		// If all the commits are merged, show the "already on main branch"
+		// error. It isn't worth doing a detailed report of which commits we
+		// found.
+		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
+		// them in the error message. But only the candidates from the current
+		// branch, not including any that are already merged.
+		subjects, err := self.c.Git().Commit.GetHashesAndCommitMessagesFirstLine(hashGroups[NOT_MERGED])
 		if err != nil {
 			return err
 		}
@@ -83,20 +124,9 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 		return fmt.Errorf("%s\n\n%s", message, subjects)
 	}
 
-	commit, index, ok := self.findCommit(commits, hashes[0])
-	if !ok {
-		if commits[len(commits)-1].Status == models.StatusMerged {
-			// If the commit is not found, it's most likely because it's already
-			// merged, and more than 300 commits away. Check if the last known
-			// commit is already merged; if so, show the "already merged" error.
-			return errors.New(self.c.Tr.BaseCommitIsAlreadyOnMainBranch)
-		}
-		// If we get here, the current branch must have more then 300 commits. Unlikely...
-		return errors.New(self.c.Tr.BaseCommitIsNotInCurrentView)
-	}
-	if commit.Status == models.StatusMerged {
-		return errors.New(self.c.Tr.BaseCommitIsAlreadyOnMainBranch)
-	}
+	// 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])
 
 	doIt := func() error {
 		if !hasStagedChanges {

pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_main_branch.go

@@ -35,7 +35,6 @@ var FindBaseCommitForFixupDisregardMainBranch = NewIntegrationTest(NewIntegratio
 			Focus().
 			Press(keys.Files.FindBaseCommitForFixup)
 
-		/* EXPECTED:
 		t.Views().Commits().
 			IsFocused().
 			Lines(
@@ -44,13 +43,5 @@ var FindBaseCommitForFixupDisregardMainBranch = NewIntegrationTest(NewIntegratio
 				Contains("2nd commit"),
 				Contains("1st commit"),
 			)
-		*/
-
-		// ACTUAL:
-		t.ExpectPopup().Alert().Title(Equals("Error")).Content(
-			Contains("Multiple base commits found.").
-				Contains("2nd commit").
-				Contains("3rd commit"),
-		).Confirm()
 	},
 })