Skip to content

Commit 5bc6d22

Browse files
committed
review feedback
1 parent bc1ab20 commit 5bc6d22

File tree

6 files changed

+104
-46
lines changed

6 files changed

+104
-46
lines changed

command/issue.go

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ func init() {
3535
issueListCmd.Flags().StringP("state", "s", "", "Filter by state: {open|closed|all}")
3636
issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch")
3737

38-
issueViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in termianl")
38+
issueViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in terminal")
3939
}
4040

4141
var issueCmd = &cobra.Command{
@@ -225,29 +225,8 @@ func issueView(cmd *cobra.Command, args []string) error {
225225
}
226226

227227
if preview {
228-
coloredLabels := labelList(*issue)
229-
if coloredLabels != "" {
230-
coloredLabels = utils.Gray(fmt.Sprintf(" (%s)", coloredLabels))
231-
}
232-
meta := "opened by %s. %d comment"
233-
if issue.Comments.TotalCount != 1 {
234-
meta += "s."
235-
} else {
236-
meta += "."
237-
}
238-
meta += coloredLabels
239-
240228
out := colorableOut(cmd)
241-
242-
fmt.Fprintln(out, utils.Bold(issue.Title))
243-
fmt.Fprintln(out, utils.Gray(fmt.Sprintf(meta,
244-
issue.Author.Login,
245-
issue.Comments.TotalCount,
246-
)))
247-
fmt.Fprintln(out)
248-
fmt.Fprintln(out, utils.RenderMarkdown(issue.Body))
249-
fmt.Fprintln(out)
250-
fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), openURL)
229+
printIssuePreview(out, issue)
251230
return nil
252231
} else {
253232
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
@@ -256,6 +235,25 @@ func issueView(cmd *cobra.Command, args []string) error {
256235

257236
}
258237

238+
func printIssuePreview(out io.Writer, issue *api.Issue) {
239+
coloredLabels := labelList(*issue)
240+
if coloredLabels != "" {
241+
coloredLabels = utils.Gray(fmt.Sprintf("(%s)", coloredLabels))
242+
}
243+
244+
fmt.Fprintln(out, utils.Bold(issue.Title))
245+
fmt.Fprintln(out, utils.Gray(fmt.Sprintf(
246+
"opened by %s. %s. %s",
247+
issue.Author.Login,
248+
utils.Pluralize(issue.Comments.TotalCount, "comment"),
249+
coloredLabels,
250+
)))
251+
fmt.Fprintln(out)
252+
fmt.Fprintln(out, utils.RenderMarkdown(issue.Body))
253+
fmt.Fprintln(out)
254+
fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL)
255+
}
256+
259257
var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`)
260258

261259
func issueFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.Issue, error) {

command/issue_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ func TestIssueView_preview(t *testing.T) {
264264

265265
expectedLines := []*regexp.Regexp{
266266
regexp.MustCompile(`ix of coins`),
267-
regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`),
267+
regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`),
268268
regexp.MustCompile(`bold story`),
269269
regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`),
270270
}

command/pr.go

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func init() {
3232
prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label")
3333
prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee")
3434

35-
prViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in termianl")
35+
prViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in terminal")
3636
}
3737

3838
var prCmd = &cobra.Command{
@@ -276,6 +276,12 @@ func prView(cmd *cobra.Command, args []string) error {
276276

277277
if prNumber > 0 {
278278
openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), prNumber)
279+
if preview {
280+
pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber)
281+
if err != nil {
282+
return err
283+
}
284+
}
279285
} else {
280286
pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner)
281287
if err != nil {
@@ -291,35 +297,30 @@ func prView(cmd *cobra.Command, args []string) error {
291297
}
292298

293299
if preview {
294-
meta := "%s wants to merge %d commit"
295-
if pr.Commits.TotalCount == 1 {
296-
meta += " "
297-
} else {
298-
meta += "s "
299-
}
300-
meta += "into %s from %s"
301-
302300
out := colorableOut(cmd)
303-
304-
fmt.Fprintln(out, utils.Bold(pr.Title))
305-
fmt.Fprintln(out, utils.Gray(fmt.Sprintf(meta,
306-
pr.Author.Login,
307-
pr.Commits.TotalCount,
308-
pr.BaseRefName,
309-
pr.HeadRefName,
310-
)))
311-
fmt.Fprintln(out)
312-
fmt.Fprintln(out, utils.RenderMarkdown(pr.Body))
313-
fmt.Fprintln(out)
314-
fmt.Fprintf(out, utils.Gray("View this PR on GitHub: %s\n"), openURL)
315-
301+
printPrPreview(out, pr)
316302
return nil
317303
} else {
318304
fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL)
319305
return utils.OpenInBrowser(openURL)
320306
}
321307
}
322308

309+
func printPrPreview(out io.Writer, pr *api.PullRequest) {
310+
fmt.Fprintln(out, utils.Bold(pr.Title))
311+
fmt.Fprintln(out, utils.Gray(fmt.Sprintf(
312+
"%s wants to merge %s into %s from %s",
313+
pr.Author.Login,
314+
utils.Pluralize(pr.Commits.TotalCount, "commit"),
315+
pr.BaseRefName,
316+
pr.HeadRefName,
317+
)))
318+
fmt.Fprintln(out)
319+
fmt.Fprintln(out, utils.RenderMarkdown(pr.Body))
320+
fmt.Fprintln(out)
321+
fmt.Fprintf(out, utils.Gray("View this PR on GitHub: %s\n"), pr.URL)
322+
}
323+
323324
var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`)
324325

325326
func prFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.PullRequest, error) {

command/pr_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,40 @@ func TestPRView_preview(t *testing.T) {
267267
}
268268
}
269269

270+
func TestPRView_previewCurrentBranch(t *testing.T) {
271+
initBlankContext("OWNER/REPO", "blueberries")
272+
http := initFakeHTTP()
273+
274+
jsonFile, _ := os.Open("../test/fixtures/prView.json")
275+
defer jsonFile.Close()
276+
http.StubResponse(200, jsonFile)
277+
278+
restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable {
279+
return &outputStub{}
280+
})
281+
defer restoreCmd()
282+
283+
output, err := RunCommand(prViewCmd, "pr view -p")
284+
if err != nil {
285+
t.Errorf("error running command `pr view`: %v", err)
286+
}
287+
288+
eq(t, output.Stderr(), "")
289+
290+
expectedLines := []*regexp.Regexp{
291+
regexp.MustCompile(`Blueberries are a good fruit`),
292+
regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`),
293+
regexp.MustCompile(`blueberries taste good`),
294+
regexp.MustCompile(`View this PR on GitHub: https://github.com/OWNER/REPO/pull/10`),
295+
}
296+
for _, r := range expectedLines {
297+
if !r.MatchString(output.String()) {
298+
t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output)
299+
return
300+
}
301+
}
302+
}
303+
270304
func TestPRView_currentBranch(t *testing.T) {
271305
initBlankContext("OWNER/REPO", "blueberries")
272306
http := initFakeHTTP()

test/fixtures/prView.json

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,37 @@
66
{
77
"number": 12,
88
"title": "Blueberries are from a fork",
9+
"body": "yeah",
910
"url": "https://github.com/OWNER/REPO/pull/12",
1011
"headRefName": "blueberries",
12+
"baseRefName": "master",
1113
"headRepositoryOwner": {
1214
"login": "hubot"
1315
},
16+
"commits": {
17+
"totalCount": 12
18+
},
19+
"author": {
20+
"login": "nobody"
21+
},
1422
"isCrossRepository": true
1523
},
1624
{
1725
"number": 10,
1826
"title": "Blueberries are a good fruit",
27+
"body": "**blueberries taste good**",
1928
"url": "https://github.com/OWNER/REPO/pull/10",
29+
"baseRefName": "master",
2030
"headRefName": "blueberries",
31+
"author": {
32+
"login": "nobody"
33+
},
2134
"headRepositoryOwner": {
2235
"login": "OWNER"
2336
},
37+
"commits": {
38+
"totalCount": 8
39+
},
2440
"isCrossRepository": false
2541
}
2642
]

utils/utils.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package utils
33
import (
44
"bytes"
55
"errors"
6+
"fmt"
67
"os"
78
"os/exec"
89
"runtime"
@@ -84,3 +85,11 @@ func RenderMarkdown(text string) string {
8485

8586
return mdCompiler.Compile(string(textB))
8687
}
88+
89+
func Pluralize(num int, thing string) string {
90+
if num == 1 {
91+
return fmt.Sprintf("%d %s", num, thing)
92+
} else {
93+
return fmt.Sprintf("%d %ss", num, thing)
94+
}
95+
}

0 commit comments

Comments
 (0)