Skip to content

Commit e0a2d97

Browse files
committed
Put subject last in git log's prettyFormat
We are going to truncate overly long lines returned from git log, and the most likely field that is going to make the line too long is the subject; so we must put it last, otherwise we'd end up with not enough fields to split when it's too long. It might not be obvious from the diff what's happening to the mock command output in the test: it didn't have the divergence field (">") at all, which was kind of a bug. It didn't matter for these tests though, because we are not testing the divergence here, and our production code happens to be resilient against it missing. But now we must add the ">" field before the subject.
1 parent 66d0ce8 commit e0a2d97

File tree

2 files changed

+19
-19
lines changed

2 files changed

+19
-19
lines changed

pkg/commands/git_commands/commit_loader.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,11 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool
211211
authorEmail := split[3]
212212
extraInfo := strings.TrimSpace(split[4])
213213
parentHashes := split[5]
214-
message := split[6]
215214
divergence := models.DivergenceNone
216215
if showDivergence {
217-
divergence = lo.Ternary(split[7] == "<", models.DivergenceLeft, models.DivergenceRight)
216+
divergence = lo.Ternary(split[6] == "<", models.DivergenceLeft, models.DivergenceRight)
218217
}
218+
message := split[7]
219219

220220
tags := []string{}
221221

@@ -605,4 +605,4 @@ func (self *CommitLoader) getLogCmd(opts GetCommitsOptions) oscommands.ICmdObj {
605605
return self.cmd.New(cmdArgs).DontLog()
606606
}
607607

608-
const prettyFormat = `--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m`
608+
const prettyFormat = `--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s`

pkg/commands/git_commands/commit_loader_test.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,16 @@ import (
1515
"github.com/stretchr/testify/assert"
1616
)
1717

18-
var commitsOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|better typing for rebase mode
19-
b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|[email protected]|origin/better-tests|e94e8fc5b6fab4cb755f|fix logging
20-
e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|[email protected]|tag: 123, tag: 456|d8084cd558925eb7c9c3|refactor
21-
d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|[email protected]||65f910ebd85283b5cce9|WIP
22-
65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|[email protected]||26c07b1ab33860a1a759|WIP
23-
26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|[email protected]||3d4470a6c072208722e5|WIP
24-
3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|[email protected]||053a66a7be3da43aacdc|WIP
25-
053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|[email protected]||985fe482e806b172aea4|refactoring the config struct`, "|", "\x00", -1)
18+
var commitsOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode
19+
b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164|1640824515|Jesse Duffield|[email protected]|origin/better-tests|e94e8fc5b6fab4cb755f|>|fix logging
20+
e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c|1640823749|Jesse Duffield|[email protected]|tag: 123, tag: 456|d8084cd558925eb7c9c3|>|refactor
21+
d8084cd558925eb7c9c38afeed5725c21653ab90|1640821426|Jesse Duffield|[email protected]||65f910ebd85283b5cce9|>|WIP
22+
65f910ebd85283b5cce9bf67d03d3f1a9ea3813a|1640821275|Jesse Duffield|[email protected]||26c07b1ab33860a1a759|>|WIP
23+
26c07b1ab33860a1a7591a0638f9925ccf497ffa|1640750752|Jesse Duffield|[email protected]||3d4470a6c072208722e5|>|WIP
24+
3d4470a6c072208722e5ae9a54bcb9634959a1c5|1640748818|Jesse Duffield|[email protected]||053a66a7be3da43aacdc|>|WIP
25+
053a66a7be3da43aacdc7aa78e1fe757b82c4dd2|1640739815|Jesse Duffield|[email protected]||985fe482e806b172aea4|>|refactoring the config struct`, "|", "\x00", -1)
2626

27-
var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|better typing for rebase mode`, "|", "\x00", -1)
27+
var singleCommitOutput = strings.Replace(`0eea75e8c631fba6b58135697835d58ba4c18dbc|1640826609|Jesse Duffield|[email protected]|HEAD -> better-tests|b21997d6b4cbdf84b149|>|better typing for rebase mode`, "|", "\x00", -1)
2828

2929
func TestGetCommits(t *testing.T) {
3030
type scenario struct {
@@ -46,7 +46,7 @@ func TestGetCommits(t *testing.T) {
4646
opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
4747
runner: oscommands.NewFakeRunner(t).
4848
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
49-
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
49+
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
5050

5151
expectedCommits: []*models.Commit{},
5252
expectedError: nil,
@@ -58,7 +58,7 @@ func TestGetCommits(t *testing.T) {
5858
opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false},
5959
runner: oscommands.NewFakeRunner(t).
6060
ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
61-
ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
61+
ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
6262

6363
expectedCommits: []*models.Commit{},
6464
expectedError: nil,
@@ -73,7 +73,7 @@ func TestGetCommits(t *testing.T) {
7373
// here it's seeing which commits are yet to be pushed
7474
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
7575
// here it's actually getting all the commits in a formatted form, one per line
76-
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil).
76+
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil).
7777
// here it's testing which of the configured main branches have an upstream
7878
ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil). // this one does
7979
ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")). // this one doesn't, so it checks origin instead
@@ -210,7 +210,7 @@ func TestGetCommits(t *testing.T) {
210210
// here it's seeing which commits are yet to be pushed
211211
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
212212
// here it's actually getting all the commits in a formatted form, one per line
213-
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
213+
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
214214
// here it's testing which of the configured main branches exist; neither does
215215
ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "", errors.New("error")).
216216
ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/master"}, "", errors.New("error")).
@@ -247,7 +247,7 @@ func TestGetCommits(t *testing.T) {
247247
// here it's seeing which commits are yet to be pushed
248248
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
249249
// here it's actually getting all the commits in a formatted form, one per line
250-
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
250+
ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil).
251251
// here it's testing which of the configured main branches exist
252252
ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "master@{u}"}, "refs/remotes/origin/master", nil).
253253
ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "main@{u}"}, "", errors.New("error")).
@@ -283,7 +283,7 @@ func TestGetCommits(t *testing.T) {
283283
opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false},
284284
runner: oscommands.NewFakeRunner(t).
285285
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
286-
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
286+
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil),
287287

288288
expectedCommits: []*models.Commit{},
289289
expectedError: nil,
@@ -295,7 +295,7 @@ func TestGetCommits(t *testing.T) {
295295
opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"},
296296
runner: oscommands.NewFakeRunner(t).
297297
ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil).
298-
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%s%x00%m", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil),
298+
ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil),
299299

300300
expectedCommits: []*models.Commit{},
301301
expectedError: nil,

0 commit comments

Comments
 (0)