Show fixup base commits in correct order in ctrl-f error message (#5073)

If the ctrl-f command ("Find base commit for fixup") finds multiple
candidates, it lists them all in an error message. Previously they were
listed in random order in the error message, which can be confusing;
it's nicer to see them in the same order in which they appear in the
commit log.

Fixes #5071.

Author: stefanhaller

pkg/commands/git_commands/commit.go

@@ -217,20 +217,6 @@ func (self *CommitCommands) GetCommitMessagesFirstLine(hashes []string) (string,
 	return self.cmd.New(cmdArgs).DontLog().RunWithOutput()
 }
 
-// Example output:
-//
-//	cd50c79ae Preserve the commit message correctly even if the description has blank lines
-//	3ebba5f32 Add test demonstrating a bug with preserving the commit message
-//	9a423c388 Remove unused function
-func (self *CommitCommands) GetHashesAndCommitMessagesFirstLine(hashes []string) (string, error) {
-	cmdArgs := NewGitCmd("show").
-		Arg("--no-patch", "--pretty=format:%h %s").
-		Arg(hashes...).
-		ToArgv()
-
-	return self.cmd.New(cmdArgs).DontLog().RunWithOutput()
-}
-
 func (self *CommitCommands) GetCommitsOneline(hashes []string) (string, error) {
 	cmdArgs := NewGitCmd("show").
 		Arg("--no-patch", "--oneline").

pkg/gui/controllers/helpers/fixup_helper.go

@@ -114,10 +114,7 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 		// 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
-		}
+		subjects := self.getHashesAndSubjects(commits, hashGroups[NOT_MERGED])
 		message := lo.Ternary(hasStagedChanges,
 			self.c.Tr.MultipleBaseCommitsFoundStaged,
 			self.c.Tr.MultipleBaseCommitsFoundUnstaged)
@@ -146,6 +143,23 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error {
 	})
 }
 
+func (self *FixupHelper) getHashesAndSubjects(commits []*models.Commit, hashes []string) string {
+	// This is called only for the NOT_MERGED commits, and we know that all of them are contained in
+	// the commits slice.
+	commitsSet := set.NewFromSlice(hashes)
+	subjects := make([]string, 0, len(hashes))
+	for _, c := range commits {
+		if commitsSet.Includes(c.Hash()) {
+			subjects = append(subjects, fmt.Sprintf("%s %s", c.ShortRefName(), c.Name))
+			commitsSet.Remove(c.Hash())
+			if commitsSet.Len() == 0 {
+				break
+			}
+		}
+	}
+	return strings.Join(subjects, "\n")
+}
+
 func (self *FixupHelper) getDiff() (string, bool, error) {
 	args := []string{"-U0", "--ignore-submodules=all", "HEAD", "--"}
 

pkg/integration/tests/commit/find_base_commit_for_fixup.go

@@ -36,9 +36,9 @@ var FindBaseCommitForFixup = NewIntegrationTest(NewIntegrationTestArgs{
 		t.ExpectPopup().Alert().
 			Title(Equals("Error")).
 			Content(
-				Contains("Multiple base commits found").
-					Contains("2nd commit").
-					Contains("3rd commit"),
+				MatchesRegexp("Multiple base commits found.*\n\n" +
+					".*3rd commit\n" +
+					".*2nd commit"),
 			).
 			Confirm()
 

pkg/integration/tests/commit/find_base_commit_for_fixup_only_added_lines.go

@@ -39,9 +39,11 @@ var FindBaseCommitForFixupOnlyAddedLines = NewIntegrationTest(NewIntegrationTest
 		t.ExpectPopup().Alert().
 			Title(Equals("Error")).
 			Content(
-				Contains("Multiple base commits found").
-					Contains("3rd commit").
-					Contains("4th commit"),
+				MatchesRegexp(
+					"Multiple base commits found.*\n\n" +
+						".*4th commit\n" +
+						".*3rd commit",
+				),
 			).
 			Confirm()