Fix GPG status parsing for hydrated commits

Pass the GPG parsing mode explicitly so rebasing and sequencer commits keep their subject lines when signature status is enabled. Add regression tests for commit loading, TODO hydration, and commit row rendering.

Author: Xyerophyte

pkg/commands/git_commands/commit_loader.go

@@ -91,8 +91,9 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit,
 		defer wg.Done()
 
 		var realCommits []*models.Commit
+		showGpg := self.UserConfig().Gui.ShowGpgSigningStatus
 		realCommits, logErr = loadCommits(self.getLogCmd(opts), opts.FilterPath, func(line string) (*models.Commit, bool) {
-			return self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != ""), false
+			return self.extractCommitFromLine(opts.HashPool, line, opts.RefToShowDivergenceFrom != "", showGpg), false
 		})
 		if logErr == nil {
 			commits = append(commits, realCommits...)
@@ -189,8 +190,7 @@ func (self *CommitLoader) MergeRebasingCommits(hashPool *utils.StringPool, commi
 // then puts them into a commit object
 // example input:
 // 8ad01fe32fcc20f07bc6693f87aa4977c327f1e1|10 hours ago|Jesse Duffield| (HEAD -> master, tag: v0.15.2)|refresh commits when adding a tag
-func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line string, showDivergence bool) *models.Commit {
-	showGpg := self.UserConfig().Gui.ShowGpgSigningStatus
+func (self *CommitLoader) extractCommitFromLine(hashPool *utils.StringPool, line string, showDivergence bool, showGpg bool) *models.Commit {
 	numFields := 8
 	if showGpg {
 		numFields = 9
@@ -299,10 +299,16 @@ func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, tod
 
 	// note that we're not filtering these as we do non-rebasing commits just because
 	// I suspect that will cause some damage
+	showGpg := self.UserConfig().Gui.ShowGpgSigningStatus
+	format := prettyFormat
+	if showGpg {
+		format = prettyFormatWithGpg
+	}
+
 	cmdObj := self.cmd.New(
 		NewGitCmd("show").
 			Config("log.showSignature=false").
-			Arg("--no-patch", "--oneline", "--abbrev=20", prettyFormat).
+			Arg("--no-patch", "--oneline", "--abbrev=20", format).
 			Arg(commitHashes...).
 			ToArgv(),
 	).DontLog()
@@ -312,7 +318,10 @@ func (self *CommitLoader) getHydratedTodoCommits(hashPool *utils.StringPool, tod
 		if line == "" || line[0] != '+' {
 			return false, nil
 		}
-		commit := self.extractCommitFromLine(hashPool, line[1:], false)
+		commit := self.extractCommitFromLine(hashPool, line[1:], false, showGpg)
+		if commit == nil {
+			return false, nil
+		}
 		fullCommits[commit.Hash()] = commit
 		return false, nil
 	})

pkg/commands/git_commands/commit_loader_test.go

@@ -27,13 +27,16 @@ var commitsOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18db
 
 var singleCommitOutput = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|b21997d6b4cbdf84b149|>|HEAD -> better-tests|better typing for rebase mode`, "|", "\x00")
 
+var singleCommitOutputWithGpg = strings.ReplaceAll(`+0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|jessedduffield@gmail.com|b21997d6b4cbdf84b149|>|HEAD -> better-tests|G|better typing for rebase mode`, "|", "\x00")
+
 func TestGetCommits(t *testing.T) {
 	type scenario struct {
 		testName           string
 		runner             *oscommands.FakeCmdObjRunner
 		expectedCommitOpts []models.NewCommitOpts
 		expectedError      error
 		logOrder           string
+		showGpg            bool
 		opts               GetCommitsOptions
 		mainBranches       []string
 	}
@@ -61,6 +64,30 @@ func TestGetCommits(t *testing.T) {
 			expectedCommitOpts: []models.NewCommitOpts{},
 			expectedError:      nil,
 		},
+		{
+			testName: "should return commits with gpg status when enabled",
+			logOrder: "topo-order",
+			showGpg:  true,
+			opts:     GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false},
+			runner: oscommands.NewFakeRunner(t).
+				ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil).
+				ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%G?%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutputWithGpg, nil),
+
+			expectedCommitOpts: []models.NewCommitOpts{{
+				Hash:          "0eea75e8c631fba6b58135697835d58ba4c18dbc",
+				Name:          "better typing for rebase mode",
+				Status:        models.StatusUnpushed,
+				Action:        models.ActionNone,
+				Tags:          nil,
+				ExtraInfo:     "(HEAD -> better-tests)",
+				AuthorName:    "Jesse Duffield",
+				AuthorEmail:   "jessedduffield@gmail.com",
+				UnixTimestamp: 1640826609,
+				Parents:       []string{"b21997d6b4cbdf84b149"},
+				GpgStatus:     "G",
+			}},
+			expectedError: nil,
+		},
 		{
 			testName:     "should return commits if they are present",
 			logOrder:     "topo-order",
@@ -300,6 +327,7 @@ func TestGetCommits(t *testing.T) {
 		t.Run(scenario.testName, func(t *testing.T) {
 			common := common.NewDummyCommon()
 			common.UserConfig().Git.Log.Order = scenario.logOrder
+			common.UserConfig().Gui.ShowGpgSigningStatus = scenario.showGpg
 			cmd := oscommands.NewDummyCmdObjBuilder(scenario.runner)
 
 			builder := &CommitLoader{
@@ -333,6 +361,42 @@ func TestGetCommits(t *testing.T) {
 	}
 }
 
+func TestCommitLoader_getHydratedTodoCommitsWithGpgStatus(t *testing.T) {
+	common := common.NewDummyCommon()
+	common.UserConfig().Gui.ShowGpgSigningStatus = true
+	runner := oscommands.NewFakeRunner(t).
+		ExpectGitArgs([]string{"-c", "log.showSignature=false", "show", "--no-patch", "--oneline", "--abbrev=20", prettyFormatWithGpg, "0eea75e8c631fba6b58135697835d58ba4c18dbc"}, singleCommitOutputWithGpg, nil)
+
+	hashPool := &utils.StringPool{}
+	loader := &CommitLoader{
+		Common: common,
+		cmd:    oscommands.NewDummyCmdObjBuilder(runner),
+	}
+
+	todoCommits := []*models.Commit{models.NewCommit(hashPool, models.NewCommitOpts{
+		Hash:   "0eea75e8c631fba6b58135697835d58ba4c18dbc",
+		Status: models.StatusRebasing,
+		Action: todo.Pick,
+	})}
+
+	commits, err := loader.getHydratedTodoCommits(hashPool, todoCommits, false)
+
+	assert.NoError(t, err)
+	assert.Equal(t, []*models.Commit{models.NewCommit(hashPool, models.NewCommitOpts{
+		Hash:          "0eea75e8c631fba6b58135697835d58ba4c18dbc",
+		Name:          "better typing for rebase mode",
+		Status:        models.StatusRebasing,
+		Action:        todo.Pick,
+		ExtraInfo:     "(HEAD -> better-tests)",
+		AuthorName:    "Jesse Duffield",
+		AuthorEmail:   "jessedduffield@gmail.com",
+		UnixTimestamp: 1640826609,
+		Parents:       []string{"b21997d6b4cbdf84b149"},
+		GpgStatus:     "G",
+	})}, commits)
+	runner.CheckForMissingCalls()
+}
+
 func TestCommitLoader_getConflictedCommitImpl(t *testing.T) {
 	hashPool := &utils.StringPool{}
 
@@ -605,6 +669,7 @@ func TestCommitLoader_extractCommitFromLine(t *testing.T) {
 		testName       string
 		line           string
 		showDivergence bool
+		showGpg        bool
 		expectedCommit *models.Commit
 	}{
 		{
@@ -759,6 +824,24 @@ func TestCommitLoader_extractCommitFromLine(t *testing.T) {
 				Divergence:    models.DivergenceLeft,
 			}),
 		},
+		{
+			testName:       "valid line with gpg status",
+			line:           "hash\x00timestamp\x00author\x00email\x00parents\x00<\x00extraInfo\x00G\x00message",
+			showDivergence: true,
+			showGpg:        true,
+			expectedCommit: models.NewCommit(hashPool, models.NewCommitOpts{
+				Hash:          "hash",
+				Name:          "message",
+				Tags:          nil,
+				ExtraInfo:     "(extraInfo)",
+				UnixTimestamp: 0,
+				AuthorName:    "author",
+				AuthorEmail:   "email",
+				Parents:       []string{"parents"},
+				Divergence:    models.DivergenceLeft,
+				GpgStatus:     "G",
+			}),
+		},
 		{
 			testName:       "empty line",
 			line:           "",
@@ -785,7 +868,7 @@ func TestCommitLoader_extractCommitFromLine(t *testing.T) {
 
 	for _, scenario := range scenarios {
 		t.Run(scenario.testName, func(t *testing.T) {
-			result := loader.extractCommitFromLine(hashPool, scenario.line, scenario.showDivergence)
+			result := loader.extractCommitFromLine(hashPool, scenario.line, scenario.showDivergence, scenario.showGpg)
 			if scenario.expectedCommit == nil {
 				assert.Nil(t, result)
 			} else {

pkg/gui/presentation/commits_test.go

@@ -40,6 +40,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) {
 		selectedCommitHashPtr     *string
 		startIdx                  int
 		endIdx                    int
+		showGpg                   bool
 		showGraph                 bool
 		bisectInfo                *git_commands.BisectInfo
 		expected                  string
@@ -90,6 +91,24 @@ func TestGetCommitListDisplayStrings(t *testing.T) {
 		hash2 commit2
 						`),
 		},
+		{
+			testName: "show gpg status when enabled",
+			commitOpts: []models.NewCommitOpts{
+				{Name: "commit1", Hash: "hash1", GpgStatus: "G"},
+				{Name: "commit2", Hash: "hash2", GpgStatus: "N"},
+			},
+			startIdx:                  0,
+			endIdx:                    2,
+			showGpg:                   true,
+			showGraph:                 false,
+			bisectInfo:                git_commands.NewNullBisectInfo(),
+			cherryPickedCommitHashSet: set.New[string](),
+			now:                       time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC),
+			expected: formatExpected(`
+		hash1 ✓ commit1
+		hash2 - commit2
+						`),
+		},
 		{
 			testName: "show local branch head, except the current branch, main branches, or merged branches",
 			commitOpts: []models.NewCommitOpts{
@@ -546,6 +565,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) {
 		if !focusing || s.focus {
 			t.Run(s.testName, func(t *testing.T) {
 				hashPool := &utils.StringPool{}
+				common.UserConfig().Gui.ShowGpgSigningStatus = s.showGpg
 
 				commits := lo.Map(s.commitOpts,
 					func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) })