From 5bc6d220c4ce678b48dacc847901fbb0bc5df790 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 13 Jan 2020 15:28:13 -0600 Subject: [PATCH] review feedback --- command/issue.go | 44 ++++++++++++++++++-------------------- command/issue_test.go | 2 +- command/pr.go | 45 ++++++++++++++++++++------------------- command/pr_test.go | 34 +++++++++++++++++++++++++++++ test/fixtures/prView.json | 16 ++++++++++++++ utils/utils.go | 9 ++++++++ 6 files changed, 104 insertions(+), 46 deletions(-) diff --git a/command/issue.go b/command/issue.go index b5377f522..d6b06ad29 100644 --- a/command/issue.go +++ b/command/issue.go @@ -35,7 +35,7 @@ func init() { issueListCmd.Flags().StringP("state", "s", "", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") - issueViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in termianl") + issueViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in terminal") } var issueCmd = &cobra.Command{ @@ -225,29 +225,8 @@ func issueView(cmd *cobra.Command, args []string) error { } if preview { - coloredLabels := labelList(*issue) - if coloredLabels != "" { - coloredLabels = utils.Gray(fmt.Sprintf(" (%s)", coloredLabels)) - } - meta := "opened by %s. %d comment" - if issue.Comments.TotalCount != 1 { - meta += "s." - } else { - meta += "." - } - meta += coloredLabels - out := colorableOut(cmd) - - fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf(meta, - issue.Author.Login, - issue.Comments.TotalCount, - ))) - fmt.Fprintln(out) - fmt.Fprintln(out, utils.RenderMarkdown(issue.Body)) - fmt.Fprintln(out) - fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), openURL) + printIssuePreview(out, issue) return nil } else { fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) @@ -256,6 +235,25 @@ func issueView(cmd *cobra.Command, args []string) error { } +func printIssuePreview(out io.Writer, issue *api.Issue) { + coloredLabels := labelList(*issue) + if coloredLabels != "" { + coloredLabels = utils.Gray(fmt.Sprintf("(%s)", coloredLabels)) + } + + fmt.Fprintln(out, utils.Bold(issue.Title)) + fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + "opened by %s. %s. %s", + issue.Author.Login, + utils.Pluralize(issue.Comments.TotalCount, "comment"), + coloredLabels, + ))) + fmt.Fprintln(out) + fmt.Fprintln(out, utils.RenderMarkdown(issue.Body)) + fmt.Fprintln(out) + fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) +} + var issueURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/issues/(\d+)`) func issueFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.Issue, error) { diff --git a/command/issue_test.go b/command/issue_test.go index 93229dac2..e9f616f68 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -264,7 +264,7 @@ func TestIssueView_preview(t *testing.T) { expectedLines := []*regexp.Regexp{ regexp.MustCompile(`ix of coins`), - regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), + regexp.MustCompile(`opened by marseilles. 9 comments. \(tarot\)`), regexp.MustCompile(`bold story`), regexp.MustCompile(`View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`), } diff --git a/command/pr.go b/command/pr.go index 85ac58fa1..328ebb134 100644 --- a/command/pr.go +++ b/command/pr.go @@ -32,7 +32,7 @@ func init() { prListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") prListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - prViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in termianl") + prViewCmd.Flags().BoolP("preview", "p", false, "Preview PR in terminal") } var prCmd = &cobra.Command{ @@ -276,6 +276,12 @@ func prView(cmd *cobra.Command, args []string) error { if prNumber > 0 { openURL = fmt.Sprintf("https://github.com/%s/%s/pull/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), prNumber) + if preview { + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) + if err != nil { + return err + } + } } else { pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) if err != nil { @@ -291,28 +297,8 @@ func prView(cmd *cobra.Command, args []string) error { } if preview { - meta := "%s wants to merge %d commit" - if pr.Commits.TotalCount == 1 { - meta += " " - } else { - meta += "s " - } - meta += "into %s from %s" - out := colorableOut(cmd) - - fmt.Fprintln(out, utils.Bold(pr.Title)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf(meta, - pr.Author.Login, - pr.Commits.TotalCount, - pr.BaseRefName, - pr.HeadRefName, - ))) - fmt.Fprintln(out) - fmt.Fprintln(out, utils.RenderMarkdown(pr.Body)) - fmt.Fprintln(out) - fmt.Fprintf(out, utils.Gray("View this PR on GitHub: %s\n"), openURL) - + printPrPreview(out, pr) return nil } else { fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) @@ -320,6 +306,21 @@ func prView(cmd *cobra.Command, args []string) error { } } +func printPrPreview(out io.Writer, pr *api.PullRequest) { + fmt.Fprintln(out, utils.Bold(pr.Title)) + fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + "%s wants to merge %s into %s from %s", + pr.Author.Login, + utils.Pluralize(pr.Commits.TotalCount, "commit"), + pr.BaseRefName, + pr.HeadRefName, + ))) + fmt.Fprintln(out) + fmt.Fprintln(out, utils.RenderMarkdown(pr.Body)) + fmt.Fprintln(out) + fmt.Fprintf(out, utils.Gray("View this PR on GitHub: %s\n"), pr.URL) +} + var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`) func prFromArg(apiClient *api.Client, baseRepo context.GitHubRepository, arg string) (*api.PullRequest, error) { diff --git a/command/pr_test.go b/command/pr_test.go index e901b38bf..f6bc733b1 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -267,6 +267,40 @@ func TestPRView_preview(t *testing.T) { } } +func TestPRView_previewCurrentBranch(t *testing.T) { + initBlankContext("OWNER/REPO", "blueberries") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/prView.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(prViewCmd, "pr view -p") + if err != nil { + t.Errorf("error running command `pr view`: %v", err) + } + + eq(t, output.Stderr(), "") + + expectedLines := []*regexp.Regexp{ + regexp.MustCompile(`Blueberries are a good fruit`), + regexp.MustCompile(`nobody wants to merge 8 commits into master from blueberries`), + regexp.MustCompile(`blueberries taste good`), + regexp.MustCompile(`View this PR on GitHub: https://github.com/OWNER/REPO/pull/10`), + } + for _, r := range expectedLines { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + func TestPRView_currentBranch(t *testing.T) { initBlankContext("OWNER/REPO", "blueberries") http := initFakeHTTP() diff --git a/test/fixtures/prView.json b/test/fixtures/prView.json index c8ae23ce6..02277d7a5 100644 --- a/test/fixtures/prView.json +++ b/test/fixtures/prView.json @@ -6,21 +6,37 @@ { "number": 12, "title": "Blueberries are from a fork", + "body": "yeah", "url": "https://github.com/OWNER/REPO/pull/12", "headRefName": "blueberries", + "baseRefName": "master", "headRepositoryOwner": { "login": "hubot" }, + "commits": { + "totalCount": 12 + }, + "author": { + "login": "nobody" + }, "isCrossRepository": true }, { "number": 10, "title": "Blueberries are a good fruit", + "body": "**blueberries taste good**", "url": "https://github.com/OWNER/REPO/pull/10", + "baseRefName": "master", "headRefName": "blueberries", + "author": { + "login": "nobody" + }, "headRepositoryOwner": { "login": "OWNER" }, + "commits": { + "totalCount": 8 + }, "isCrossRepository": false } ] diff --git a/utils/utils.go b/utils/utils.go index 0183731c6..49d9cae2c 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -3,6 +3,7 @@ package utils import ( "bytes" "errors" + "fmt" "os" "os/exec" "runtime" @@ -84,3 +85,11 @@ func RenderMarkdown(text string) string { return mdCompiler.Compile(string(textB)) } + +func Pluralize(num int, thing string) string { + if num == 1 { + return fmt.Sprintf("%d %s", num, thing) + } else { + return fmt.Sprintf("%d %ss", num, thing) + } +}