diff --git a/git/client.go b/git/client.go index 6db7a92f9..308491e1f 100644 --- a/git/client.go +++ b/git/client.go @@ -234,7 +234,12 @@ func isGitSha(s string) bool { } 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...) if err != nil { return nil, err @@ -249,7 +254,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi body := 2 lines := outputLines(out) for i := 0; i < len(lines); i++ { - split := strings.SplitN(lines[i], ",", 3) + split := strings.SplitN(lines[i], "\u0000", 3) if isGitSha(split[sha]) { c := &Commit{ Sha: split[sha], @@ -264,7 +269,7 @@ func (c *Client) Commits(ctx context.Context, baseRef, headRef string) ([]*Commi break } - possibleSplit := strings.SplitN(lines[i], ",", 3) + possibleSplit := strings.SplitN(lines[i], "\u0000", 3) if len(possibleSplit) > 2 && isGitSha(possibleSplit[sha]) { i-- break diff --git a/git/client_test.go b/git/client_test.go index df44fac9d..a8ac173fe 100644 --- a/git/client_test.go +++ b/git/client_test.go @@ -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{{ Sha: "6a6872b918c601a0e730710ad8473938a7516d30", 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{{ Sha: "6a6872b918c601a0e730710ad8473938a7516d30", 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{ { 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{ { 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{ { Sha: "6a6872b918c601a0e730710ad8473938a7516d30", @@ -610,7 +610,7 @@ func TestClientCommits(t *testing.T) { testData: stubbedCommitsCommandData{ 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", }, { @@ -619,7 +619,7 @@ func TestClientCommits(t *testing.T) { ErrMsg: "git error message", 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", }, } @@ -656,9 +656,9 @@ func TestCommitsHelperProcess(t *testing.T) { var sb strings.Builder for _, commit := range td.Commits { sb.WriteString(commit.Sha) - sb.WriteString(",") + sb.WriteString("\u0000") sb.WriteString(commit.Title) - sb.WriteString(",") + sb.WriteString("\u0000") sb.WriteString(commit.Body) sb.WriteString("\n") } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index a8baa4883..c9906b7ec 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -627,7 +627,7 @@ func Test_createRun(t *testing.T) { })) }, 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) { 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) { - 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, "") }, promptStubs: func(pm *prompter.PrompterMock) { @@ -1004,9 +1004,9 @@ func Test_createRun(t *testing.T) { }, cmdStubs: func(cs *run.CommandStubber) { 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, - "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, "") }, @@ -1079,10 +1079,10 @@ func Test_createRun(t *testing.T) { }, cmdStubs: func(cs *run.CommandStubber) { 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, - "56b6f8bb7c9e3a30093cd17e48934ce354148e80,second commit of pr\n"+ - "3a9b48085046d156c5acce8f3b3a0532cd706a4a,first commit of pr,first commit description\n", + "56b6f8bb7c9e3a30093cd17e48934ce354148e80\u0000second commit of pr\u0000\n"+ + "3a9b48085046d156c5acce8f3b3a0532cd706a4a\u0000first commit of pr\u0000first commit description\n", ) }, httpStubs: func(reg *httpmock.Registry, t *testing.T) { @@ -1115,20 +1115,20 @@ func Test_createRun(t *testing.T) { }, cmdStubs: func(cs *run.CommandStubber) { 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, - "56b6f8bb7c9e3a30093cd17e48934ce354148e80,second commit of pr,second 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", + "56b6f8bb7c9e3a30093cd17e48934ce354148e80\u0000second commit of pr\u0000second commit 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) { reg.Register( httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { + { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" - } } } + } } } } `, func(input map[string]interface{}) {