Skip to content

Commit 15d6d4b

Browse files
author
Nate Smith
authored
Merge pull request cli#405 from arkentos/master
Fix: Nil pointer dereference while previewing PR and Issues with empty body
2 parents 65a7d05 + 2c2c8a9 commit 15d6d4b

File tree

5 files changed

+135
-6
lines changed

5 files changed

+135
-6
lines changed

‎command/issue.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,11 @@ func printIssuePreview(out io.Writer, issue *api.Issue) {
255255
utils.Pluralize(issue.Comments.TotalCount, "comment"),
256256
coloredLabels,
257257
)))
258-
fmt.Fprintln(out)
259-
fmt.Fprintln(out, utils.RenderMarkdown(issue.Body))
260-
fmt.Fprintln(out)
258+
if issue.Body != "" {
259+
fmt.Fprintln(out)
260+
fmt.Fprintln(out, utils.RenderMarkdown(issue.Body))
261+
fmt.Fprintln(out)
262+
}
261263
fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL)
262264
}
263265

‎command/issue_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,51 @@ func TestIssueView_preview(t *testing.T) {
288288
}
289289
}
290290

291+
func TestIssueView_previewWithEmptyBody(t *testing.T) {
292+
initBlankContext("OWNER/REPO", "master")
293+
http := initFakeHTTP()
294+
http.StubRepoResponse("OWNER", "REPO")
295+
296+
http.StubResponse(200, bytes.NewBufferString(`
297+
{ "data": { "repository": { "hasIssuesEnabled": true, "issue": {
298+
"number": 123,
299+
"body": "",
300+
"title": "ix of coins",
301+
"author": {
302+
"login": "marseilles"
303+
},
304+
"labels": {
305+
"nodes": [
306+
{"name": "tarot"}
307+
]
308+
},
309+
"comments": {
310+
"totalCount": 9
311+
},
312+
"url": "https://github.com/OWNER/REPO/issues/123"
313+
} } } }
314+
`))
315+
316+
output, err := RunCommand(issueViewCmd, "issue view -p 123")
317+
if err != nil {
318+
t.Errorf("error running command `issue view`: %v", err)
319+
}
320+
321+
eq(t, output.Stderr(), "")
322+
323+
expectedLines := []*regexp.Regexp{
324+
regexp.MustCompile(`ix of coins`),
325+
regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`),
326+
regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`),
327+
}
328+
for _, r := range expectedLines {
329+
if !r.MatchString(output.String()) {
330+
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
331+
return
332+
}
333+
}
334+
}
335+
291336
func TestIssueView_notFound(t *testing.T) {
292337
initBlankContext("OWNER/REPO", "master")
293338
http := initFakeHTTP()

‎command/pr.go‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -308,9 +308,11 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) {
308308
pr.BaseRefName,
309309
pr.HeadRefName,
310310
)))
311-
fmt.Fprintln(out)
312-
fmt.Fprintln(out, utils.RenderMarkdown(pr.Body))
313-
fmt.Fprintln(out)
311+
if pr.Body != "" {
312+
fmt.Fprintln(out)
313+
fmt.Fprintln(out, utils.RenderMarkdown(pr.Body))
314+
fmt.Fprintln(out)
315+
}
314316
fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL)
315317
}
316318

‎command/pr_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,40 @@ func TestPRView_previewCurrentBranch(t *testing.T) {
313313
}
314314
}
315315

316+
func TestPRView_previewCurrentBranchWithEmptyBody(t *testing.T) {
317+
initBlankContext("OWNER/REPO", "blueberries")
318+
http := initFakeHTTP()
319+
http.StubRepoResponse("OWNER", "REPO")
320+
321+
jsonFile, _ := os.Open("../test/fixtures/prView_EmptyBody.json")
322+
defer jsonFile.Close()
323+
http.StubResponse(200, jsonFile)
324+
325+
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
326+
return &outputStub{}
327+
})
328+
defer restoreCmd()
329+
330+
output, err := RunCommand(prViewCmd, "pr view -p")
331+
if err != nil {
332+
t.Errorf("error running command `pr view`: %v", err)
333+
}
334+
335+
eq(t, output.Stderr(), "")
336+
337+
expectedLines := []*regexp.Regexp{
338+
regexp.MustCompile(`Blueberries are a good fruit`),
339+
regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`),
340+
regexp.MustCompile(`View this pull request on GitHub: https://github.com/OWNER/REPO/pull/10`),
341+
}
342+
for _, r := range expectedLines {
343+
if !r.MatchString(output.String()) {
344+
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
345+
return
346+
}
347+
}
348+
}
349+
316350
func TestPRView_currentBranch(t *testing.T) {
317351
initBlankContext("OWNER/REPO", "blueberries")
318352
http := initFakeHTTP()

‎test/fixtures/prView_EmptyBody.json‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
{
2+
"data": {
3+
"repository": {
4+
"pullRequests": {
5+
"nodes": [
6+
{
7+
"number": 12,
8+
"title": "Blueberries are from a fork",
9+
"body": "yeah",
10+
"url": "https://github.com/OWNER/REPO/pull/12",
11+
"headRefName": "blueberries",
12+
"baseRefName": "master",
13+
"headRepositoryOwner": {
14+
"login": "hubot"
15+
},
16+
"commits": {
17+
"totalCount": 12
18+
},
19+
"author": {
20+
"login": "nobody"
21+
},
22+
"isCrossRepository": true
23+
},
24+
{
25+
"number": 10,
26+
"title": "Blueberries are a good fruit",
27+
"body": "",
28+
"url": "https://github.com/OWNER/REPO/pull/10",
29+
"baseRefName": "master",
30+
"headRefName": "blueberries",
31+
"author": {
32+
"login": "nobody"
33+
},
34+
"headRepositoryOwner": {
35+
"login": "OWNER"
36+
},
37+
"commits": {
38+
"totalCount": 8
39+
},
40+
"isCrossRepository": false
41+
}
42+
]
43+
}
44+
}
45+
}
46+
}

0 commit comments

Comments
 (0)