STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Use null byte separators when fetching comments from git

Browse source
This commit is contained in:
William Martin 2024-02-27 17:38:57 +01:00
parent fb353caf19
commit 2b0484f5aa
3 changed files with 29 additions and 24 deletions
Download patch file Download diff file Expand all files Collapse all files

11
git/client.go
View file

@ -234,7 +234,12 @@ func isGitSha(s string) bool {
} }
func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commit, error) { func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commit, error) {
args := []string{"-c", "log.ShowSignature=false", "log", "--pretty=format:%H,%s,%b", "--cherry", fmt.Sprintf("%s...%s", baseRef, headRef)} // The formatting directive %x00 indicates that git should include the null byte as a separator.
// We use this because it is not a valid character to include in a commit message. Previously,
// commas were used here but when we Split on them, we would get incorrect results if commit titles
// happened to contain them.
// https://git-scm.com/docs/pretty-formats#Documentation/pretty-formats.txt-emx00em
args := []string{"-c", "log.ShowSignature=false", "log", "--pretty=format:%H%x00%s%x00%b", "--cherry", fmt.Sprintf("%s...%s", baseRef, headRef)}
cmd, err := c.Command(ctx, args...) cmd, err := c.Command(ctx, args...)
if err != nil { if err != nil {
return nil, err return nil, err
@ -249,7 +254,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi
body := 2 body := 2
lines := outputLines(out) lines := outputLines(out)
for i := 0; i < len(lines); i++ { for i := 0; i < len(lines); i++ {
split := strings.SplitN(lines[i], ",", 3) split := strings.SplitN(lines[i], "\u0000", 3)
if isGitSha(split[sha]) { if isGitSha(split[sha]) {
c := &Commit{ c := &Commit{
Sha: split[sha], Sha: split[sha],
@ -264,7 +269,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi
break break
} }
possibleSplit := strings.SplitN(lines[i], ",", 3) possibleSplit := strings.SplitN(lines[i], "\u0000", 3)
if len(possibleSplit) > 2 && isGitSha(possibleSplit[sha]) { if len(possibleSplit) > 2 && isGitSha(possibleSplit[sha]) {
i-- i--
break break

18
git/client_test.go
View file

@ -493,7 +493,7 @@ func TestClientCommits(t *testing.T) {
}, },
}, },
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantCommits: []*Commit{{ wantCommits: []*Commit{{
Sha: "6a6872b918c601a0e730710ad8473938a7516d30", Sha: "6a6872b918c601a0e730710ad8473938a7516d30",
Title: "testing testability test", Title: "testing testability test",
@ -510,7 +510,7 @@ func TestClientCommits(t *testing.T) {
}, },
}, },
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantCommits: []*Commit{{ wantCommits: []*Commit{{
Sha: "6a6872b918c601a0e730710ad8473938a7516d30", Sha: "6a6872b918c601a0e730710ad8473938a7516d30",
Title: "testing testability test", Title: "testing testability test",
@ -533,7 +533,7 @@ func TestClientCommits(t *testing.T) {
}, },
}, },
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantCommits: []*Commit{ wantCommits: []*Commit{
{ {
Sha: "6a6872b918c601a0e730710ad8473938a7516d30", Sha: "6a6872b918c601a0e730710ad8473938a7516d30",
@ -562,7 +562,7 @@ func TestClientCommits(t *testing.T) {
}, },
}, },
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantCommits: []*Commit{ wantCommits: []*Commit{
{ {
Sha: "6a6872b918c601a0e730710ad8473938a7516d30", Sha: "6a6872b918c601a0e730710ad8473938a7516d30",
@ -591,7 +591,7 @@ func TestClientCommits(t *testing.T) {
}, },
}, },
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantCommits: []*Commit{ wantCommits: []*Commit{
{ {
Sha: "6a6872b918c601a0e730710ad8473938a7516d30", Sha: "6a6872b918c601a0e730710ad8473938a7516d30",
@ -610,7 +610,7 @@ func TestClientCommits(t *testing.T) {
testData: stubbedCommitsCommandData{ testData: stubbedCommitsCommandData{
Commits: []stubbedCommit{}, Commits: []stubbedCommit{},
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantErrorMsg: "could not find any commits between SHA1 and SHA2", wantErrorMsg: "could not find any commits between SHA1 and SHA2",
}, },
{ {
@ -619,7 +619,7 @@ func TestClientCommits(t *testing.T) {
ErrMsg: "git error message", ErrMsg: "git error message",
ExitStatus: 1, ExitStatus: 1,
}, },
wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry SHA1...SHA2`, wantCmdArgs: `path/to/git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry SHA1...SHA2`,
wantErrorMsg: "failed to run git: git error message", wantErrorMsg: "failed to run git: git error message",
}, },
} }
@ -656,9 +656,9 @@ func TestCommitsHelperProcess(t *testing.T) {
var sb strings.Builder var sb strings.Builder
for _, commit := range td.Commits { for _, commit := range td.Commits {
sb.WriteString(commit.Sha) sb.WriteString(commit.Sha)
sb.WriteString(",") sb.WriteString("\u0000")
sb.WriteString(commit.Title) sb.WriteString(commit.Title)
sb.WriteString(",") sb.WriteString("\u0000")
sb.WriteString(commit.Body) sb.WriteString(commit.Body)
sb.WriteString("\n") sb.WriteString("\n")
} }

24
pkg/cmd/pr/create/create_test.go
View file

@ -627,7 +627,7 @@ func Test_createRun(t *testing.T) {
})) }))
}, },
cmdStubs: func(cs *run.CommandStubber) { cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "d3476a1,commit 0\n7a6ea13,commit 1") cs.Register(`git( .+)? log( .+)? origin/master\.\.\.feature`, 0, "d3476a1\u0000commit 0\u0000\n7a6ea13\u0000commit 1\u0000")
}, },
promptStubs: func(pm *prompter.PrompterMock) { promptStubs: func(pm *prompter.PrompterMock) {
pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) { pm.MarkdownEditorFunc = func(p, d string, ba bool) (string, error) {
@ -851,7 +851,7 @@ func Test_createRun(t *testing.T) {
})) }))
}, },
cmdStubs: func(cs *run.CommandStubber) { cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry origin/master...feature`, 0, "") cs.Register(`git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry origin/master...feature`, 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`git rev-parse --show-toplevel`, 0, "")
}, },
promptStubs: func(pm *prompter.PrompterMock) { promptStubs: func(pm *prompter.PrompterMock) {
@ -1004,9 +1004,9 @@ func Test_createRun(t *testing.T) {
}, },
cmdStubs: func(cs *run.CommandStubber) { cmdStubs: func(cs *run.CommandStubber) {
cs.Register( cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry origin/master...feature", "git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry origin/master...feature",
0, 0,
"3a9b48085046d156c5acce8f3b3a0532cd706a4a,first commit of pr,first commit description\n", "3a9b48085046d156c5acce8f3b3a0532cd706a4a\u0000first commit of pr\u0000first commit description\n",
) )
cs.Register(`git rev-parse --show-toplevel`, 0, "") cs.Register(`git rev-parse --show-toplevel`, 0, "")
}, },
@ -1079,10 +1079,10 @@ func Test_createRun(t *testing.T) {
}, },
cmdStubs: func(cs *run.CommandStubber) { cmdStubs: func(cs *run.CommandStubber) {
cs.Register( cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry origin/master...feature", "git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry origin/master...feature",
0, 0,
"56b6f8bb7c9e3a30093cd17e48934ce354148e80,second commit of pr\n"+ "56b6f8bb7c9e3a30093cd17e48934ce354148e80\u0000second commit of pr\u0000\n"+
"3a9b48085046d156c5acce8f3b3a0532cd706a4a,first commit of pr,first commit description\n", "3a9b48085046d156c5acce8f3b3a0532cd706a4a\u0000first commit of pr\u0000first commit description\n",
) )
}, },
httpStubs: func(reg *httpmock.Registry, t *testing.T) { httpStubs: func(reg *httpmock.Registry, t *testing.T) {
@ -1115,20 +1115,20 @@ func Test_createRun(t *testing.T) {
}, },
cmdStubs: func(cs *run.CommandStubber) { cmdStubs: func(cs *run.CommandStubber) {
cs.Register( cs.Register(
"git -c log.ShowSignature=false log --pretty=format:%H,%s,%b --cherry origin/master...feature", "git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b --cherry origin/master...feature",
0, 0,
"56b6f8bb7c9e3a30093cd17e48934ce354148e80,second commit of pr,second commit description\n"+ "56b6f8bb7c9e3a30093cd17e48934ce354148e80\u0000second commit of pr\u0000second commit description\n"+
"3a9b48085046d156c5acce8f3b3a0532cd706a4a,first commit of pr,first commit with super long description, with super long description, with super long description, with super long description.\n", "3a9b48085046d156c5acce8f3b3a0532cd706a4a\u0000first commit of pr\u0000first commit with super long description, with super long description, with super long description, with super long description.\n",
) )
}, },
httpStubs: func(reg *httpmock.Registry, t *testing.T) { httpStubs: func(reg *httpmock.Registry, t *testing.T) {
reg.Register( reg.Register(
httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQL(`mutation PullRequestCreate\b`),
httpmock.GraphQLMutation(` httpmock.GraphQLMutation(`
{ {
"data": { "createPullRequest": { "pullRequest": { "data": { "createPullRequest": { "pullRequest": {
"URL": "https://github.com/OWNER/REPO/pull/12" "URL": "https://github.com/OWNER/REPO/pull/12"
} } } } } }
} }
`, `,
func(input map[string]interface{}) { func(input map[string]interface{}) {