From 357de1b18349b1926a9564626bdf01686af712ed Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 30 Oct 2019 16:26:33 -0700 Subject: [PATCH 01/17] Add Issue query --- api/queries.go | 103 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 103 insertions(+) diff --git a/api/queries.go b/api/queries.go index e1e078138..c0bc53eca 100644 --- a/api/queries.go +++ b/api/queries.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "time" ) type PullRequestsPayload struct { @@ -22,6 +23,108 @@ type Repo interface { RepoOwner() string } +type IssuesPayload struct { + Assigned []Issue + Mentioned []Issue + Recent []Issue +} + +type Issue struct { + Number int + Title string +} + +func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload, error) { + type issues struct { + Issues struct { + Edges []struct { + Node Issue + } + } + } + + type response struct { + Assigned issues + Mentioned issues + Recent issues + } + + query := ` + fragment issue on Issue { + number + title + } + query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { + assigned: repository(owner: $owner, name: $repo) { + issues(filterBy: {assignee: $viewer}, first: $per_page) { + edges { + node { + ...issue + } + } + } + } + mentioned: repository(owner: $owner, name: $repo) { + issues(filterBy: {mentioned: $viewer}, first: $per_page) { + edges { + node { + ...issue + } + } + } + } + recent: repository(owner: $owner, name: $repo) { + issues(filterBy: {since: $since, orderBy: {field: CREATED_AT, direction: DESC}}, first: $per_page) { + edges { + node { + ...issue + } + } + } + } + } + ` + + owner := ghRepo.RepoOwner() + repo := ghRepo.RepoName() + since := time.Now().UTC().Add(time.Hour * -24).Format("2006-01-02T15:04:05-0700") + variables := map[string]interface{}{ + "owner": owner, + "repo": repo, + "viewer": currentUsername, + "since": since, + } + + var resp response + err := client.GraphQL(query, variables, &resp) + if err != nil { + return nil, err + } + + var assigned []Issue + for _, edge := range resp.Assigned.Issues.Edges { + assigned = append(assigned, edge.Node) + } + + var mentioned []Issue + for _, edge := range resp.Mentioned.Issues.Edges { + mentioned = append(mentioned, edge.Node) + } + + var recent []Issue + for _, edge := range resp.Recent.Issues.Edges { + recent = append(recent, edge.Node) + } + + payload := IssuesPayload{ + assigned, + mentioned, + recent, + } + + return &payload, nil +} + func PullRequests(client *Client, ghRepo Repo, currentBranch, currentUsername string) (*PullRequestsPayload, error) { type edges struct { Edges []struct { From d9ef40c873afda892b63b422cb7a359b44f88951 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 31 Oct 2019 11:41:24 +0100 Subject: [PATCH 02/17] Add `completion` script --- command/completion.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 command/completion.go diff --git a/command/completion.go b/command/completion.go new file mode 100644 index 000000000..c3b1e86b1 --- /dev/null +++ b/command/completion.go @@ -0,0 +1,41 @@ +package command + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" +) + +var shellType string + +func init() { + RootCmd.AddCommand(completionCmd) + completionCmd.Flags().StringVarP(&shellType, "shell", "s", "bash", "The type of shell") +} + +var completionCmd = &cobra.Command{ + Use: "completion", + Hidden: true, + Short: "Generates completion scripts", + Long: `To enable completion in your shell, run: + + eval "$(gh completion)" + +You can add that to your '~/.bash_profile' to enable completion whenever you +start a new shell. + +When installing with Homebrew, see https://docs.brew.sh/Shell-Completion +`, + RunE: func(cmd *cobra.Command, args []string) error { + switch shellType { + case "bash": + RootCmd.GenBashCompletion(os.Stdout) + case "zsh": + RootCmd.GenZshCompletion(os.Stdout) + default: + return fmt.Errorf("unsupported shell type: %s", shellType) + } + return nil + }, +} From cf1feb847e65c17dacf99bc5b0340e88a97c59b4 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Thu, 31 Oct 2019 11:02:27 -0700 Subject: [PATCH 03/17] Add `gh issue list` and `gh issue view ISSUE_NUMBER` --- command/issue.go | 117 +++++++++++++++++++++++++++++++++++ command/issue_test.go | 61 ++++++++++++++++++ command/pr.go | 6 +- command/pr_test.go | 35 ----------- command/testing.go | 39 ++++++++++++ test/fixtures/issueList.json | 47 ++++++++++++++ test/fixtures/issueView.json | 36 +++++++++++ test/helpers.go | 1 + 8 files changed, 303 insertions(+), 39 deletions(-) create mode 100644 command/issue.go create mode 100644 command/issue_test.go create mode 100644 command/testing.go create mode 100644 test/fixtures/issueList.json create mode 100644 test/fixtures/issueView.json diff --git a/command/issue.go b/command/issue.go new file mode 100644 index 000000000..e64d8d806 --- /dev/null +++ b/command/issue.go @@ -0,0 +1,117 @@ +package command + +import ( + "fmt" + "strconv" + + "github.com/github/gh-cli/api" + "github.com/github/gh-cli/utils" + "github.com/spf13/cobra" +) + +func init() { + RootCmd.AddCommand(issueCmd) + issueCmd.AddCommand( + &cobra.Command{ + Use: "list", + Short: "List issues", + RunE: issueList, + }, + &cobra.Command{ + Use: "view issue-number", + Args: cobra.MinimumNArgs(1), + Short: "Open a issue in the browser", + RunE: issueView, + }, + ) +} + +var issueCmd = &cobra.Command{ + Use: "issue", + Short: "Work with GitHub issues", + Long: `This command allows you to work with issues.`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("%+v is not a valid issue command", args) + }, +} + +func issueList(cmd *cobra.Command, args []string) error { + cmd.SilenceUsage = true + + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return err + } + + currentUser, err := ctx.AuthLogin() + if err != nil { + return err + } + + issuePayload, err := api.Issues(apiClient, baseRepo, currentUser) + if err != nil { + return err + } + + printHeader("Issues assigned to you") + if issuePayload.Assigned != nil { + printIssues(issuePayload.Assigned...) + } else { + message := fmt.Sprintf(" There are no issues assgined to you") + printMessage(message) + } + fmt.Println() + + printHeader("Issues mentioning you") + if len(issuePayload.Mentioned) > 0 { + printIssues(issuePayload.Mentioned...) + } else { + printMessage(" There are no issues mentioning you") + } + fmt.Println() + + printHeader("Recent issues") + if len(issuePayload.Recent) > 0 { + printIssues(issuePayload.Recent...) + } else { + printMessage(" There are no recent issues") + } + fmt.Println() + + return nil +} + +func issueView(cmd *cobra.Command, args []string) error { + cmd.SilenceUsage = true + + ctx := contextForCommand(cmd) + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return err + } + + var openURL string + if number, err := strconv.Atoi(args[0]); err == nil { + // TODO: move URL generation into GitHubRepository + openURL = fmt.Sprintf("https://github.com/%s/%s/issues/%d", baseRepo.RepoOwner(), baseRepo.RepoName(), number) + } else { + return fmt.Errorf("invalid issue number: '%s'", args[0]) + } + + fmt.Printf("Opening %s in your browser.\n", openURL) + return utils.OpenInBrowser(openURL) +} + +func printIssues(issues ...api.Issue) { + for _, issue := range issues { + fmt.Printf(" #%d %s\n", issue.Number, truncateTitle(issue.Title, 70)) + } +} diff --git a/command/issue_test.go b/command/issue_test.go new file mode 100644 index 000000000..69312cac7 --- /dev/null +++ b/command/issue_test.go @@ -0,0 +1,61 @@ +package command + +import ( + "os" + "regexp" + "testing" + + "github.com/github/gh-cli/test" +) + +func TestIssueList(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/issueList.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + output, err := test.RunCommand(RootCmd, "issue list") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + expectedIssues := []*regexp.Regexp{ + regexp.MustCompile(`#8.*carrots`), + regexp.MustCompile(`#9.*squash`), + regexp.MustCompile(`#10.*broccoli`), + regexp.MustCompile(`#11.*swiss chard`), + } + + for _, r := range expectedIssues { + if !r.MatchString(output) { + t.Errorf("output did not match regexp /%s/", r) + } + } +} + +func TestIssueView(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/issueView.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + teardown, callCount := mockOpenInBrowser() + defer teardown() + + output, err := test.RunCommand(RootCmd, "issue view 8") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + if output == "" { + t.Errorf("command output expected got an empty string") + } + + if *callCount != 1 { + t.Errorf("OpenInBrowser should be called 1 time but was called %d time(s)", *callCount) + } +} diff --git a/command/pr.go b/command/pr.go index f1dfe8fb0..182522f29 100644 --- a/command/pr.go +++ b/command/pr.go @@ -129,7 +129,7 @@ func prView(cmd *cobra.Command, args []string) error { func printPrs(prs ...api.PullRequest) { for _, pr := range prs { - fmt.Printf(" #%d %s %s\n", pr.Number, truncateTitle(pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + fmt.Printf(" #%d %s %s\n", pr.Number, truncateTitle(pr.Title, 50), utils.Cyan("["+pr.HeadRefName+"]")) } } @@ -141,9 +141,7 @@ func printMessage(s string) { fmt.Println(utils.Gray(s)) } -func truncateTitle(title string) string { - const maxLength = 50 - +func truncateTitle(title string, maxLength int) string { if len(title) > maxLength { return title[0:maxLength-3] + "..." } diff --git a/command/pr_test.go b/command/pr_test.go index e4a5122d7..5ac21b9bd 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -5,29 +5,9 @@ import ( "regexp" "testing" - "github.com/github/gh-cli/api" - "github.com/github/gh-cli/context" "github.com/github/gh-cli/test" - "github.com/github/gh-cli/utils" ) -func initBlankContext(repo, branch string) { - initContext = func() context.Context { - ctx := context.NewBlank() - ctx.SetBaseRepo(repo) - ctx.SetBranch(branch) - return ctx - } -} - -func initFakeHTTP() *api.FakeHTTP { - http := &api.FakeHTTP{} - apiClientForContext = func(context.Context) (*api.Client, error) { - return api.NewClient(api.ReplaceTripper(http)), nil - } - return http -} - func TestPRList(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() @@ -114,18 +94,3 @@ func TestPRView_NoActiveBranch(t *testing.T) { t.Errorf("OpenInBrowser should be called once but was called %d time(s)", *callCount) } } - -func mockOpenInBrowser() (func(), *int) { - callCount := 0 - originalOpenInBrowser := utils.OpenInBrowser - teardown := func() { - utils.OpenInBrowser = originalOpenInBrowser - } - - utils.OpenInBrowser = func(_ string) error { - callCount++ - return nil - } - - return teardown, &callCount -} diff --git a/command/testing.go b/command/testing.go new file mode 100644 index 000000000..a6ccf4807 --- /dev/null +++ b/command/testing.go @@ -0,0 +1,39 @@ +package command + +import ( + "github.com/github/gh-cli/api" + "github.com/github/gh-cli/context" + "github.com/github/gh-cli/utils" +) + +func initBlankContext(repo, branch string) { + initContext = func() context.Context { + ctx := context.NewBlank() + ctx.SetBaseRepo(repo) + ctx.SetBranch(branch) + return ctx + } +} + +func initFakeHTTP() *api.FakeHTTP { + http := &api.FakeHTTP{} + apiClientForContext = func(context.Context) (*api.Client, error) { + return api.NewClient(api.ReplaceTripper(http)), nil + } + return http +} + +func mockOpenInBrowser() (func(), *int) { + callCount := 0 + originalOpenInBrowser := utils.OpenInBrowser + teardown := func() { + utils.OpenInBrowser = originalOpenInBrowser + } + + utils.OpenInBrowser = func(_ string) error { + callCount++ + return nil + } + + return teardown, &callCount +} diff --git a/test/fixtures/issueList.json b/test/fixtures/issueList.json new file mode 100644 index 000000000..784c0cc8f --- /dev/null +++ b/test/fixtures/issueList.json @@ -0,0 +1,47 @@ +{ + "data": { + "assigned": { + "issues": { + "edges": [ + { + "node": { + "number": 9, + "title": "corey thinks squash tastes bad" + } + }, + { + "node": { + "number": 10, + "title": "broccoli is a superfood" + } + } + ] + } + }, + "mentioned": { + "issues": { + "edges": [ + { + "node": { + "number": 8, + "title": "rabbits eat carrots" + } + }, + { + "node": { + "number": 11, + "title": "swiss chard is neutral" + } + } + ] + } + }, + "recent": { + "issues": { + "edges": [] + } + }, + + "pageInfo": { "hasNextPage": false } + } +} diff --git a/test/fixtures/issueView.json b/test/fixtures/issueView.json new file mode 100644 index 000000000..c9b0cb1b6 --- /dev/null +++ b/test/fixtures/issueView.json @@ -0,0 +1,36 @@ +{ + "data": { + "repository": { + "issues": { + "edges": [ + { + "node": { + "number": 8, + "title": "rabbits eat carrots", + "url": "https://github.com/github/gh-cli/pull/10" + } + }, + { + "node": { + "number": 9, + "title": "corey thinks squash tastes bad" + } + }, + { + "node": { + "number": 10, + "title": "broccoli is a superfood" + } + }, + { + "node": { + "number": 11, + "title": "swiss chard is neutral" + } + } + ] + } + }, + "pageInfo": { "hasNextPage": false } + } +} diff --git a/test/helpers.go b/test/helpers.go index d9276565c..162befa56 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -71,6 +71,7 @@ func RunCommand(root *cobra.Command, s string) (string, error) { root.SetArgs(strings.Split(s, " ")) _, err = root.ExecuteC() }) + if err != nil { return "", err } From 4f03370aa52fba72693002e88d9d732f655b0a8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 31 Oct 2019 23:43:05 +0100 Subject: [PATCH 04/17] Add tests --- command/completion.go | 14 ++++++----- command/completion_test.go | 50 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) create mode 100644 command/completion_test.go diff --git a/command/completion.go b/command/completion.go index c3b1e86b1..2a528a1ba 100644 --- a/command/completion.go +++ b/command/completion.go @@ -2,16 +2,13 @@ package command import ( "fmt" - "os" "github.com/spf13/cobra" ) -var shellType string - func init() { RootCmd.AddCommand(completionCmd) - completionCmd.Flags().StringVarP(&shellType, "shell", "s", "bash", "The type of shell") + completionCmd.Flags().StringP("shell", "s", "bash", "The type of shell") } var completionCmd = &cobra.Command{ @@ -28,11 +25,16 @@ start a new shell. When installing with Homebrew, see https://docs.brew.sh/Shell-Completion `, RunE: func(cmd *cobra.Command, args []string) error { + shellType, err := cmd.Flags().GetString("shell") + if err != nil { + return err + } + switch shellType { case "bash": - RootCmd.GenBashCompletion(os.Stdout) + RootCmd.GenBashCompletion(cmd.OutOrStdout()) case "zsh": - RootCmd.GenZshCompletion(os.Stdout) + RootCmd.GenZshCompletion(cmd.OutOrStdout()) default: return fmt.Errorf("unsupported shell type: %s", shellType) } diff --git a/command/completion_test.go b/command/completion_test.go new file mode 100644 index 000000000..8cea652b1 --- /dev/null +++ b/command/completion_test.go @@ -0,0 +1,50 @@ +package command + +import ( + "bytes" + "strings" + "testing" +) + +func TestCompletion_bash(t *testing.T) { + out := bytes.Buffer{} + completionCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"completion"}) + _, err := RootCmd.ExecuteC() + if err != nil { + t.Fatal(err) + } + + outStr := out.String() + if !strings.Contains(outStr, "complete -o default -F __start_gh gh") { + t.Errorf("problem in bash completion:\n%s", outStr) + } +} + +func TestCompletion_zsh(t *testing.T) { + out := bytes.Buffer{} + completionCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"completion", "-s", "zsh"}) + _, err := RootCmd.ExecuteC() + if err != nil { + t.Fatal(err) + } + + outStr := out.String() + if !strings.Contains(outStr, "#compdef _gh gh") { + t.Errorf("problem in zsh completion:\n%s", outStr) + } +} + +func TestCompletion_unsupported(t *testing.T) { + out := bytes.Buffer{} + completionCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"completion", "-s", "fish"}) + _, err := RootCmd.ExecuteC() + if err == nil || err.Error() != "unsupported shell type: fish" { + t.Fatal(err) + } +} From 875352a03cd1c7afc4cf717e5c74bf46e367eafc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 14:34:33 +0100 Subject: [PATCH 05/17] Fix issues order --- api/queries.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/api/queries.go b/api/queries.go index c0bc53eca..161aa9e59 100644 --- a/api/queries.go +++ b/api/queries.go @@ -56,7 +56,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } query($owner: String!, $repo: String!, $since: DateTime!, $viewer: String!, $per_page: Int = 10) { assigned: repository(owner: $owner, name: $repo) { - issues(filterBy: {assignee: $viewer}, first: $per_page) { + issues(filterBy: {assignee: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -65,7 +65,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } } mentioned: repository(owner: $owner, name: $repo) { - issues(filterBy: {mentioned: $viewer}, first: $per_page) { + issues(filterBy: {mentioned: $viewer}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue @@ -74,7 +74,7 @@ func Issues(client *Client, ghRepo Repo, currentUsername string) (*IssuesPayload } } recent: repository(owner: $owner, name: $repo) { - issues(filterBy: {since: $since, orderBy: {field: CREATED_AT, direction: DESC}}, first: $per_page) { + issues(filterBy: {since: $since}, first: $per_page, orderBy: {field: CREATED_AT, direction: DESC}) { edges { node { ...issue From d881a2e52ed3d2cb6a9b276ac9cf83f4252688fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 22:16:23 +0100 Subject: [PATCH 06/17] Ensure git operations preserve their stderr in error output This also provides a SetPrepareCmd hook for tests to be able to define stubs for commands that are supposed to be run --- git/git.go | 39 ++++++++++------------------- utils/prepare_cmd.go | 59 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 26 deletions(-) create mode 100644 utils/prepare_cmd.go diff --git a/git/git.go b/git/git.go index b3b63388e..a2a385f7f 100644 --- a/git/git.go +++ b/git/git.go @@ -8,12 +8,13 @@ import ( "os/exec" "path/filepath" "strings" + + "github.com/github/gh-cli/utils" ) func Dir() (string, error) { dirCmd := exec.Command("git", "rev-parse", "-q", "--git-dir") - dirCmd.Stderr = nil - output, err := dirCmd.Output() + output, err := utils.PrepareCmd(dirCmd).Output() if err != nil { return "", fmt.Errorf("Not a git repository (or any of the parent directories): .git") } @@ -33,7 +34,7 @@ func Dir() (string, error) { func WorkdirName() (string, error) { toplevelCmd := exec.Command("git", "rev-parse", "--show-toplevel") toplevelCmd.Stderr = nil - output, err := toplevelCmd.Output() + output, err := utils.PrepareCmd(toplevelCmd).Output() dir := firstLine(output) if dir == "" { return "", fmt.Errorf("unable to determine git working directory") @@ -44,8 +45,7 @@ func WorkdirName() (string, error) { func HasFile(segments ...string) bool { // The blessed way to resolve paths within git dir since Git 2.5.0 pathCmd := exec.Command("git", "rev-parse", "-q", "--git-path", filepath.Join(segments...)) - pathCmd.Stderr = nil - if output, err := pathCmd.Output(); err == nil { + if output, err := utils.PrepareCmd(pathCmd).Output(); err == nil { if lines := outputLines(output); len(lines) == 1 { if _, err := os.Stat(lines[0]); err == nil { return true @@ -97,8 +97,7 @@ func BranchAtRef(paths ...string) (name string, err error) { func Editor() (string, error) { varCmd := exec.Command("git", "var", "GIT_EDITOR") - varCmd.Stderr = nil - output, err := varCmd.Output() + output, err := utils.PrepareCmd(varCmd).Output() if err != nil { return "", fmt.Errorf("Can't load git var: GIT_EDITOR") } @@ -112,8 +111,7 @@ func Head() (string, error) { func SymbolicFullName(name string) (string, error) { parseCmd := exec.Command("git", "rev-parse", "--symbolic-full-name", name) - parseCmd.Stderr = nil - output, err := parseCmd.Output() + output, err := utils.PrepareCmd(parseCmd).Output() if err != nil { return "", fmt.Errorf("Unknown revision or path not in the working tree: %s", name) } @@ -145,9 +143,7 @@ func CommentChar(text string) (string, error) { func Show(sha string) (string, error) { cmd := exec.Command("git", "-c", "log.showSignature=false", "show", "-s", "--format=%s%n%+b", sha) - cmd.Stderr = nil - - output, err := cmd.Output() + output, err := utils.PrepareCmd(cmd).Output() return strings.TrimSpace(string(output)), err } @@ -157,7 +153,7 @@ func Log(sha1, sha2 string) (string, error) { "-c", "log.showSignature=false", "log", "--no-color", "--format=%h (%aN, %ar)%n%w(78,3,3)%s%n%+b", "--cherry", shaRange) - outputs, err := cmd.Output() + outputs, err := utils.PrepareCmd(cmd).Output() if err != nil { return "", fmt.Errorf("Can't load git log %s..%s", sha1, sha2) } @@ -167,14 +163,13 @@ func Log(sha1, sha2 string) (string, error) { func listRemotes() ([]string, error) { remoteCmd := exec.Command("git", "remote", "-v") - remoteCmd.Stderr = nil - output, err := remoteCmd.Output() + output, err := utils.PrepareCmd(remoteCmd).Output() return outputLines(output), err } func Config(name string) (string, error) { configCmd := exec.Command("git", "config", name) - output, err := configCmd.Output() + output, err := utils.PrepareCmd(configCmd).Output() if err != nil { return "", fmt.Errorf("unknown config key: %s", name) } @@ -190,21 +185,16 @@ func ConfigAll(name string) ([]string, error) { } configCmd := exec.Command("git", "config", mode, name) - output, err := configCmd.Output() + output, err := utils.PrepareCmd(configCmd).Output() if err != nil { return nil, fmt.Errorf("Unknown config %s", name) } return outputLines(output), nil } -func Run(args ...string) error { - cmd := exec.Command("git", args...) - return cmd.Run() -} - func LocalBranches() ([]string, error) { branchesCmd := exec.Command("git", "branch", "--list") - output, err := branchesCmd.Output() + output, err := utils.PrepareCmd(branchesCmd).Output() if err != nil { return nil, err } @@ -218,9 +208,6 @@ func LocalBranches() ([]string, error) { func outputLines(output []byte) []string { lines := strings.TrimSuffix(string(output), "\n") - if lines == "" { - return []string{} - } return strings.Split(lines, "\n") } diff --git a/utils/prepare_cmd.go b/utils/prepare_cmd.go new file mode 100644 index 000000000..41cfeecd0 --- /dev/null +++ b/utils/prepare_cmd.go @@ -0,0 +1,59 @@ +package utils + +import ( + "bytes" + "fmt" + "os/exec" + "strings" +) + +// Runnable is typically an exec.Cmd or its stub in tests +type Runnable interface { + Output() ([]byte, error) + Run() error +} + +// PrepareCmd extends exec.Cmd with extra error reporting features and provides a +// hook to stub command execution in tests +var PrepareCmd = func(cmd *exec.Cmd) Runnable { + return &cmdWithStderr{cmd} +} + +// SetPrepareCmd overrides PrepareCmd and returns a func to revert it back +func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() { + origPrepare := PrepareCmd + PrepareCmd = fn + return func() { + PrepareCmd = origPrepare + } +} + +// cmdWithStderr augments Output() by adding stderr to the error message +type cmdWithStderr struct { + *exec.Cmd +} + +func (c cmdWithStderr) Output() ([]byte, error) { + errStream := &bytes.Buffer{} + c.Cmd.Stderr = errStream + out, err := c.Cmd.Output() + if err != nil { + err = &CmdError{errStream, c.Cmd.Args, err} + } + return out, err +} + +// CmdError provides more visibility into why an exec.Cmd had failed +type CmdError struct { + Stderr *bytes.Buffer + Args []string + Err error +} + +func (e CmdError) Error() string { + msg := e.Stderr.String() + if msg != "" && !strings.HasSuffix(msg, "\n") { + msg += "\n" + } + return fmt.Sprintf("%s%s: %s", msg, e.Args[0], e.Err) +} From f6fcdf114e8958c32f2850684dc6eb69a0cbcd46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 22:18:12 +0100 Subject: [PATCH 07/17] Use SetPrepareCmd hook to spy on OpenInBrowser We are now able to assert that the browse command was called with the correct URL --- command/pr_test.go | 64 ++++++++++++++++++++++++--------------- test/fixtures/prView.json | 8 ++--- utils/utils.go | 5 +-- 3 files changed, 46 insertions(+), 31 deletions(-) diff --git a/command/pr_test.go b/command/pr_test.go index e4a5122d7..88eb6f2a8 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -2,6 +2,7 @@ package command import ( "os" + "os/exec" "regexp" "testing" @@ -63,8 +64,12 @@ func TestPRView(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - teardown, callCount := mockOpenInBrowser() - defer teardown() + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() output, err := test.RunCommand(RootCmd, "pr view") if err != nil { @@ -75,9 +80,25 @@ func TestPRView(t *testing.T) { t.Errorf("command output expected got an empty string") } - if *callCount != 1 { - t.Errorf("OpenInBrowser should be called 1 time but was called %d time(s)", *callCount) + if seenCmd == nil { + t.Fatal("expected a command to run") } + url := seenCmd.Args[len(seenCmd.Args)-1] + if url != "https://github.com/OWNER/REPO/pull/10" { + t.Errorf("got: %q", url) + } +} + +type outputStub struct { + contents []byte +} + +func (s outputStub) Output() ([]byte, error) { + return s.contents, nil +} + +func (s outputStub) Run() error { + return nil } func TestPRView_NoActiveBranch(t *testing.T) { @@ -88,16 +109,20 @@ func TestPRView_NoActiveBranch(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - teardown, callCount := mockOpenInBrowser() - defer teardown() + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() output, err := test.RunCommand(RootCmd, "pr view") if err == nil || err.Error() != "the 'master' branch has no open pull requests" { t.Errorf("error running command `pr view`: %v", err) } - if *callCount > 0 { - t.Errorf("OpenInBrowser should NOT be called but was called %d time(s)", *callCount) + if seenCmd != nil { + t.Fatalf("unexpected command: %v", seenCmd.Args) } // Now run again but provide a PR number @@ -110,22 +135,11 @@ func TestPRView_NoActiveBranch(t *testing.T) { t.Errorf("command output expected got an empty string") } - if *callCount != 1 { - t.Errorf("OpenInBrowser should be called once but was called %d time(s)", *callCount) + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + if url != "https://github.com/OWNER/REPO/pull/23" { + t.Errorf("got: %q", url) } } - -func mockOpenInBrowser() (func(), *int) { - callCount := 0 - originalOpenInBrowser := utils.OpenInBrowser - teardown := func() { - utils.OpenInBrowser = originalOpenInBrowser - } - - utils.OpenInBrowser = func(_ string) error { - callCount++ - return nil - } - - return teardown, &callCount -} diff --git a/test/fixtures/prView.json b/test/fixtures/prView.json index 444fa5e01..5ac892498 100644 --- a/test/fixtures/prView.json +++ b/test/fixtures/prView.json @@ -6,7 +6,7 @@ "node": { "number": 10, "title": "Blueberries are a good fruit", - "url": "https://github.com/github/gh-cli/pull/10", + "url": "https://github.com/OWNER/REPO/pull/10", "headRefName": "[blueberries]" } } @@ -19,7 +19,7 @@ "node": { "number": 8, "title": "Strawberries are not actually berries", - "url": "https://github.com/github/gh-cli/pull/8", + "url": "https://github.com/OWNER/REPO/pull/8", "headRefName": "[strawberries]" } } @@ -32,7 +32,7 @@ "node": { "number": 9, "title": "Apples are tasty", - "url": "https://github.com/github/gh-cli/pull/9", + "url": "https://github.com/OWNER/REPO/pull/9", "headRefName": "[apples]" } }, @@ -40,7 +40,7 @@ "node": { "number": 11, "title": "Figs are my favorite", - "url": "https://github.com/github/gh-cli/pull/1", + "url": "https://github.com/OWNER/REPO/pull/1", "headRefName": "[figs]" } } diff --git a/utils/utils.go b/utils/utils.go index ff1eb1993..b034a18d9 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -27,7 +27,7 @@ func ConcatPaths(paths ...string) string { return strings.Join(paths, "/") } -var OpenInBrowser = func(url string) error { +func OpenInBrowser(url string) error { browser := os.Getenv("BROWSER") if browser == "" { browser = searchBrowserLauncher(runtime.GOOS) @@ -45,7 +45,8 @@ var OpenInBrowser = func(url string) error { } endingArgs := append(browserArgs[1:], url) - return exec.Command(browserArgs[0], endingArgs...).Run() + browseCmd := exec.Command(browserArgs[0], endingArgs...) + return PrepareCmd(browseCmd).Run() } func searchBrowserLauncher(goos string) (browser string) { From 8ee97d72cd56ac12fac55d5cb0c8391a6f85e96d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 23:20:15 +0100 Subject: [PATCH 08/17] Extract outputStub into `testing.go` --- command/pr_test.go | 12 ------------ command/testing.go | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 12 deletions(-) create mode 100644 command/testing.go diff --git a/command/pr_test.go b/command/pr_test.go index 88eb6f2a8..6d5bf8869 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -89,18 +89,6 @@ func TestPRView(t *testing.T) { } } -type outputStub struct { - contents []byte -} - -func (s outputStub) Output() ([]byte, error) { - return s.contents, nil -} - -func (s outputStub) Run() error { - return nil -} - func TestPRView_NoActiveBranch(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/testing.go b/command/testing.go new file mode 100644 index 000000000..3d7284d23 --- /dev/null +++ b/command/testing.go @@ -0,0 +1,14 @@ +package command + +// outputStub implements a simple utils.Runnable +type outputStub struct { + output []byte +} + +func (s outputStub) Output() ([]byte, error) { + return s.output, nil +} + +func (s outputStub) Run() error { + return nil +} From 531db42c473e4985e056647eac82a05ec2086d0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 1 Nov 2019 23:20:45 +0100 Subject: [PATCH 09/17] Log executed git commands when DEBUG is active --- utils/prepare_cmd.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/utils/prepare_cmd.go b/utils/prepare_cmd.go index 41cfeecd0..0129d1a5b 100644 --- a/utils/prepare_cmd.go +++ b/utils/prepare_cmd.go @@ -3,6 +3,7 @@ package utils import ( "bytes" "fmt" + "os" "os/exec" "strings" ) @@ -28,12 +29,15 @@ func SetPrepareCmd(fn func(*exec.Cmd) Runnable) func() { } } -// cmdWithStderr augments Output() by adding stderr to the error message +// cmdWithStderr augments exec.Cmd by adding stderr to the error message type cmdWithStderr struct { *exec.Cmd } func (c cmdWithStderr) Output() ([]byte, error) { + if os.Getenv("DEBUG") != "" { + fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) + } errStream := &bytes.Buffer{} c.Cmd.Stderr = errStream out, err := c.Cmd.Output() @@ -43,6 +47,19 @@ func (c cmdWithStderr) Output() ([]byte, error) { return out, err } +func (c cmdWithStderr) Run() error { + if os.Getenv("DEBUG") != "" { + fmt.Fprintf(os.Stderr, "%v\n", c.Cmd.Args) + } + errStream := &bytes.Buffer{} + c.Cmd.Stderr = errStream + err := c.Cmd.Run() + if err != nil { + err = &CmdError{errStream, c.Cmd.Args, err} + } + return err +} + // CmdError provides more visibility into why an exec.Cmd had failed type CmdError struct { Stderr *bytes.Buffer From 667704d574eaec480460423239c851620288a975 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 Nov 2019 17:33:45 +0100 Subject: [PATCH 10/17] Add `pr list` command Old `pr list` is now `pr status` --- api/queries.go | 92 ++++++++++++++++++++++ command/pr.go | 147 ++++++++++++++++++++++++++++++------ command/pr_test.go | 70 ++++++++++++++++- go.mod | 1 + go.sum | 1 + test/fixtures/prList.json | 80 +++++++++----------- test/fixtures/prStatus.json | 50 ++++++++++++ 7 files changed, 368 insertions(+), 73 deletions(-) create mode 100644 test/fixtures/prStatus.json diff --git a/api/queries.go b/api/queries.go index e1e078138..7e9ae0831 100644 --- a/api/queries.go +++ b/api/queries.go @@ -171,3 +171,95 @@ func PullRequestsForBranch(client *Client, ghRepo Repo, branch string) ([]PullRe return prs, nil } + +func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([]PullRequest, error) { + type response struct { + Repository struct { + PullRequests struct { + Edges []struct { + Node PullRequest + } + PageInfo struct { + HasNextPage bool + EndCursor string + } + } + } + } + + query := ` + query( + $owner: String!, + $repo: String!, + $limit: Int!, + $endCursor: String, + $baseBranch: String, + $labels: [String!], + $state: [PullRequestState!] = OPEN + ) { + repository(owner: $owner, name: $repo) { + pullRequests( + states: $state, + baseRefName: $baseBranch, + labels: $labels, + first: $limit, + after: $endCursor, + orderBy: {field: CREATED_AT, direction: DESC} + ) { + edges { + node { + number + title + url + headRefName + } + } + pageInfo { + hasNextPage + endCursor + } + } + } + }` + + prs := []PullRequest{} + pageLimit := min(limit, 100) + variables := map[string]interface{}{} + for name, val := range vars { + variables[name] = val + } + + for { + variables["limit"] = pageLimit + var data response + err := client.GraphQL(query, variables, &data) + if err != nil { + return nil, err + } + prData := data.Repository.PullRequests + + for _, edge := range prData.Edges { + prs = append(prs, edge.Node) + if len(prs) == limit { + goto done + } + } + + if prData.PageInfo.HasNextPage { + variables["endCursor"] = prData.PageInfo.EndCursor + pageLimit = min(pageLimit, limit-len(prs)) + continue + } + done: + break + } + + return prs, nil +} + +func min(a, b int) int { + if a < b { + return a + } + return b +} diff --git a/command/pr.go b/command/pr.go index f1dfe8fb0..4be283ee6 100644 --- a/command/pr.go +++ b/command/pr.go @@ -2,41 +2,49 @@ package command import ( "fmt" + "os" "strconv" "github.com/github/gh-cli/api" "github.com/github/gh-cli/utils" "github.com/spf13/cobra" + "golang.org/x/crypto/ssh/terminal" ) func init() { RootCmd.AddCommand(prCmd) - prCmd.AddCommand( - &cobra.Command{ - Use: "list", - Short: "List pull requests", - RunE: prList, - }, - &cobra.Command{ - Use: "view [pr-number]", - Short: "Open a pull request in the browser", - RunE: prView, - }, - ) + prCmd.AddCommand(prListCmd) + prCmd.AddCommand(prStatusCmd) + prCmd.AddCommand(prViewCmd) + + prListCmd.Flags().IntP("limit", "L", 30, "maximum number of items to fetch") + prListCmd.Flags().StringP("state", "s", "open", "filter by state") + prListCmd.Flags().StringP("base", "b", "", "filter by base branch") + prListCmd.Flags().StringArrayP("label", "l", nil, "filter by label") } var prCmd = &cobra.Command{ Use: "pr", Short: "Work with pull requests", - Long: `This command allows you to -work with pull requests.`, - Args: cobra.MinimumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - return fmt.Errorf("%+v is not a valid PR command", args) - }, + Long: `Helps you work with pull requests.`, +} +var prListCmd = &cobra.Command{ + Use: "list", + Short: "List pull requests", + RunE: prList, +} +var prStatusCmd = &cobra.Command{ + Use: "status", + Short: "Show status of relevant pull requests", + RunE: prStatus, +} +var prViewCmd = &cobra.Command{ + Use: "view [pr-number]", + Short: "Open a pull request in the browser", + RunE: prView, } -func prList(cmd *cobra.Command, args []string) error { +func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) if err != nil { @@ -89,6 +97,101 @@ func prList(cmd *cobra.Command, args []string) error { return nil } +func prList(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := ctx.BaseRepo() + if err != nil { + return err + } + + limit, err := cmd.Flags().GetInt("limit") + if err != nil { + return err + } + state, err := cmd.Flags().GetString("state") + if err != nil { + return err + } + baseBranch, err := cmd.Flags().GetString("base") + if err != nil { + return err + } + labels, err := cmd.Flags().GetStringArray("label") + if err != nil { + return err + } + + var graphqlState string + switch state { + case "open": + graphqlState = "OPEN" + case "closed": + graphqlState = "CLOSED" + case "all": + graphqlState = "ALL" + default: + return fmt.Errorf("invalid state: %s", state) + } + + params := map[string]interface{}{ + "owner": baseRepo.RepoOwner(), + "repo": baseRepo.RepoName(), + "state": graphqlState, + } + if len(labels) > 0 { + params["labels"] = labels + } + if baseBranch != "" { + params["baseBranch"] = baseBranch + } + + prs, err := api.PullRequestList(apiClient, params, limit) + if err != nil { + return err + } + + tty := false + ttyWidth := 80 + out := cmd.OutOrStdout() + if outFile, isFile := out.(*os.File); isFile { + fd := int(outFile.Fd()) + tty = terminal.IsTerminal(fd) + if w, _, err := terminal.GetSize(fd); err == nil { + ttyWidth = w + } + } + + numWidth := 8 + branchWidth := 40 + titleWidth := ttyWidth - branchWidth - 2 - numWidth - 2 + maxTitleWidth := 0 + for _, pr := range prs { + if len(pr.Title) > maxTitleWidth { + maxTitleWidth = len(pr.Title) + } + } + if maxTitleWidth < titleWidth { + branchWidth += titleWidth - maxTitleWidth + titleWidth = maxTitleWidth + } + + for _, pr := range prs { + if tty { + prNum := utils.Yellow(fmt.Sprintf("% *s", numWidth, fmt.Sprintf("#%d", pr.Number))) + prBranch := utils.Cyan(truncate(branchWidth, pr.HeadRefName)) + fmt.Fprintf(out, "%s %-*s %s\n", prNum, titleWidth, truncate(titleWidth, pr.Title), prBranch) + } else { + fmt.Fprintf(out, "%d\t%s\t%s\n", pr.Number, pr.Title, pr.HeadRefName) + } + } + return nil +} + func prView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) baseRepo, err := ctx.BaseRepo() @@ -129,7 +232,7 @@ func prView(cmd *cobra.Command, args []string) error { func printPrs(prs ...api.PullRequest) { for _, pr := range prs { - fmt.Printf(" #%d %s %s\n", pr.Number, truncateTitle(pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) + fmt.Printf(" #%d %s %s\n", pr.Number, truncate(50, pr.Title), utils.Cyan("["+pr.HeadRefName+"]")) } } @@ -141,9 +244,7 @@ func printMessage(s string) { fmt.Println(utils.Gray(s)) } -func truncateTitle(title string) string { - const maxLength = 50 - +func truncate(maxLength int, title string) string { if len(title) > maxLength { return title[0:maxLength-3] + "..." } diff --git a/command/pr_test.go b/command/pr_test.go index e4a5122d7..57569c506 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -1,7 +1,11 @@ package command import ( + "bytes" + "encoding/json" + "io/ioutil" "os" + "reflect" "regexp" "testing" @@ -11,6 +15,13 @@ import ( "github.com/github/gh-cli/utils" ) +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + func initBlankContext(repo, branch string) { initContext = func() context.Context { ctx := context.NewBlank() @@ -28,17 +39,17 @@ func initFakeHTTP() *api.FakeHTTP { return http } -func TestPRList(t *testing.T) { +func TestPRStatus(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - jsonFile, _ := os.Open("../test/fixtures/prList.json") + jsonFile, _ := os.Open("../test/fixtures/prStatus.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := test.RunCommand(RootCmd, "pr list") + output, err := test.RunCommand(RootCmd, "pr status") if err != nil { - t.Errorf("error running command `pr list`: %v", err) + t.Errorf("error running command `pr status`: %v", err) } expectedPrs := []*regexp.Regexp{ @@ -55,6 +66,57 @@ func TestPRList(t *testing.T) { } } +func TestPRList(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + jsonFile, _ := os.Open("../test/fixtures/prList.json") + defer jsonFile.Close() + http.StubResponse(200, jsonFile) + + out := bytes.Buffer{} + prListCmd.SetOut(&out) + + RootCmd.SetArgs([]string{"pr", "list"}) + _, err := RootCmd.ExecuteC() + if err != nil { + t.Fatal(err) + } + + eq(t, out.String(), `32 New feature feature +29 Fixed bad bug bug-fix +28 Improve documentation docs +`) +} + +func TestPRList_filtering(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + + respBody := bytes.NewBufferString(`{ "data": {} }`) + http.StubResponse(200, respBody) + + prListCmd.SetOut(ioutil.Discard) + + RootCmd.SetArgs([]string{"pr", "list", "-s", "all", "-l", "one", "-l", "two"}) + _, err := RootCmd.ExecuteC() + if err != nil { + t.Fatal(err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + reqBody := struct { + Variables struct { + State string + Labels []string + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.State, "ALL") + eq(t, reqBody.Variables.Labels, []string{"one", "two"}) +} + func TestPRView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() diff --git a/go.mod b/go.mod index b80ee1d9a..07a356b32 100644 --- a/go.mod +++ b/go.mod @@ -9,5 +9,6 @@ require ( github.com/mattn/go-isatty v0.0.9 github.com/mitchellh/go-homedir v1.1.0 github.com/spf13/cobra v0.0.5 + golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 gopkg.in/yaml.v3 v3.0.0-20191010095647-fc94e3f71652 ) diff --git a/go.sum b/go.sum index 1a7c223fc..e5b196f09 100644 --- a/go.sum +++ b/go.sum @@ -43,6 +43,7 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q= +golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9 h1:mKdxBk7AujPs8kU4m80U72y/zjbZ3UcXC7dClwKbUI0= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= diff --git a/test/fixtures/prList.json b/test/fixtures/prList.json index 444fa5e01..7ff9502a0 100644 --- a/test/fixtures/prList.json +++ b/test/fixtures/prList.json @@ -1,50 +1,38 @@ -{"data":{ - "repository": { - "pullRequests": { - "edges": [ - { - "node": { - "number": 10, - "title": "Blueberries are a good fruit", - "url": "https://github.com/github/gh-cli/pull/10", - "headRefName": "[blueberries]" +{ + "data": { + "repository": { + "pullRequests": { + "edges": [ + { + "node": { + "number": 32, + "title": "New feature", + "url": "https://github.com/monalisa/hello/pull/32", + "headRefName": "feature" + } + }, + { + "node": { + "number": 29, + "title": "Fixed bad bug", + "url": "https://github.com/monalisa/hello/pull/29", + "headRefName": "bug-fix" + } + }, + { + "node": { + "number": 28, + "title": "Improve documentation", + "url": "https://github.com/monalisa/hello/pull/28", + "headRefName": "docs" + } } + ], + "pageInfo": { + "hasNextPage": false, + "endCursor": "" } - ] + } } - }, - "viewerCreated": { - "edges": [ - { - "node": { - "number": 8, - "title": "Strawberries are not actually berries", - "url": "https://github.com/github/gh-cli/pull/8", - "headRefName": "[strawberries]" - } - } - ], - "pageInfo": { "hasNextPage": false } - }, - "reviewRequested": { - "edges": [ - { - "node": { - "number": 9, - "title": "Apples are tasty", - "url": "https://github.com/github/gh-cli/pull/9", - "headRefName": "[apples]" - } - }, - { - "node": { - "number": 11, - "title": "Figs are my favorite", - "url": "https://github.com/github/gh-cli/pull/1", - "headRefName": "[figs]" - } - } - ], - "pageInfo": { "hasNextPage": false } } -}} \ No newline at end of file +} \ No newline at end of file diff --git a/test/fixtures/prStatus.json b/test/fixtures/prStatus.json new file mode 100644 index 000000000..444fa5e01 --- /dev/null +++ b/test/fixtures/prStatus.json @@ -0,0 +1,50 @@ +{"data":{ + "repository": { + "pullRequests": { + "edges": [ + { + "node": { + "number": 10, + "title": "Blueberries are a good fruit", + "url": "https://github.com/github/gh-cli/pull/10", + "headRefName": "[blueberries]" + } + } + ] + } + }, + "viewerCreated": { + "edges": [ + { + "node": { + "number": 8, + "title": "Strawberries are not actually berries", + "url": "https://github.com/github/gh-cli/pull/8", + "headRefName": "[strawberries]" + } + } + ], + "pageInfo": { "hasNextPage": false } + }, + "reviewRequested": { + "edges": [ + { + "node": { + "number": 9, + "title": "Apples are tasty", + "url": "https://github.com/github/gh-cli/pull/9", + "headRefName": "[apples]" + } + }, + { + "node": { + "number": 11, + "title": "Figs are my favorite", + "url": "https://github.com/github/gh-cli/pull/1", + "headRefName": "[figs]" + } + } + ], + "pageInfo": { "hasNextPage": false } + } +}} \ No newline at end of file From e3a11c8ffbb0ddaf75821d1a740ff5883fbbdb37 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 5 Nov 2019 14:48:32 -0700 Subject: [PATCH 11/17] fix pr filtering --- command/pr.go | 10 ++++++---- command/pr_test.go | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/command/pr.go b/command/pr.go index 4be283ee6..50b6efd9a 100644 --- a/command/pr.go +++ b/command/pr.go @@ -126,14 +126,16 @@ func prList(cmd *cobra.Command, args []string) error { return err } - var graphqlState string + var graphqlState []string switch state { case "open": - graphqlState = "OPEN" + graphqlState = []string{"OPEN"} case "closed": - graphqlState = "CLOSED" + graphqlState = []string{"CLOSED"} + case "merged": + graphqlState = []string{"MERGED"} case "all": - graphqlState = "ALL" + graphqlState = []string{"OPEN", "CLOSED", "MERGED"} default: return fmt.Errorf("invalid state: %s", state) } diff --git a/command/pr_test.go b/command/pr_test.go index 57569c506..47f6df62e 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -107,13 +107,13 @@ func TestPRList_filtering(t *testing.T) { bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) reqBody := struct { Variables struct { - State string + State []string Labels []string } }{} json.Unmarshal(bodyBytes, &reqBody) - eq(t, reqBody.Variables.State, "ALL") + eq(t, reqBody.Variables.State, []string{"OPEN", "CLOSED", "MERGED"}) eq(t, reqBody.Variables.Labels, []string{"one", "two"}) } From 9133ad9f870f7efaffe4e2b950c78349633bffff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 Nov 2019 18:48:41 +0100 Subject: [PATCH 12/17] `pr list`: indicate state by color, useful for `-s all` --- api/queries.go | 2 ++ command/pr.go | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/api/queries.go b/api/queries.go index 7e9ae0831..68a9d14ba 100644 --- a/api/queries.go +++ b/api/queries.go @@ -13,6 +13,7 @@ type PullRequestsPayload struct { type PullRequest struct { Number int Title string + State string URL string HeadRefName string } @@ -210,6 +211,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] node { number title + state url headRefName } diff --git a/command/pr.go b/command/pr.go index 50b6efd9a..90b91ecb6 100644 --- a/command/pr.go +++ b/command/pr.go @@ -184,7 +184,15 @@ func prList(cmd *cobra.Command, args []string) error { for _, pr := range prs { if tty { - prNum := utils.Yellow(fmt.Sprintf("% *s", numWidth, fmt.Sprintf("#%d", pr.Number))) + prNum := fmt.Sprintf("% *s", numWidth, fmt.Sprintf("#%d", pr.Number)) + switch pr.State { + case "OPEN": + prNum = utils.Green(prNum) + case "CLOSED": + prNum = utils.Red(prNum) + case "MERGED": + prNum = utils.Magenta(prNum) + } prBranch := utils.Cyan(truncate(branchWidth, pr.HeadRefName)) fmt.Fprintf(out, "%s %-*s %s\n", prNum, titleWidth, truncate(titleWidth, pr.Title), prBranch) } else { From e6d55fff1cda55202af4dee038750480a4cddda6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 Nov 2019 18:53:58 +0100 Subject: [PATCH 13/17] Calculate instead of hardcode the width of the PR number column --- command/pr.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/command/pr.go b/command/pr.go index 90b91ecb6..efbf8d995 100644 --- a/command/pr.go +++ b/command/pr.go @@ -168,15 +168,21 @@ func prList(cmd *cobra.Command, args []string) error { } } - numWidth := 8 - branchWidth := 40 - titleWidth := ttyWidth - branchWidth - 2 - numWidth - 2 + numWidth := 0 maxTitleWidth := 0 for _, pr := range prs { + numLen := len(strconv.Itoa(pr.Number)) + 1 + if numLen > numWidth { + numWidth = numLen + } if len(pr.Title) > maxTitleWidth { maxTitleWidth = len(pr.Title) } } + + branchWidth := 40 + titleWidth := ttyWidth - branchWidth - 2 - numWidth - 2 + if maxTitleWidth < titleWidth { branchWidth += titleWidth - maxTitleWidth titleWidth = maxTitleWidth From b50898f05b97d36292281172035ef9ea575522db Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 6 Nov 2019 10:01:16 -0800 Subject: [PATCH 14/17] move to init --- command/issue.go | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/command/issue.go b/command/issue.go index e64d8d806..17dc9507e 100644 --- a/command/issue.go +++ b/command/issue.go @@ -10,11 +10,20 @@ import ( ) func init() { - RootCmd.AddCommand(issueCmd) + var issueCmd = &cobra.Command{ + Use: "issue", + Short: "Work with GitHub issues", + Long: `This command allows you to work with issues.`, + Args: cobra.MinimumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + return fmt.Errorf("%+v is not a valid issue command", args) + }, + } + issueCmd.AddCommand( &cobra.Command{ - Use: "list", - Short: "List issues", + Use: "status", + Short: "Display issue status", RunE: issueList, }, &cobra.Command{ @@ -24,16 +33,8 @@ func init() { RunE: issueView, }, ) -} -var issueCmd = &cobra.Command{ - Use: "issue", - Short: "Work with GitHub issues", - Long: `This command allows you to work with issues.`, - Args: cobra.MinimumNArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - return fmt.Errorf("%+v is not a valid issue command", args) - }, + RootCmd.AddCommand(issueCmd) } func issueList(cmd *cobra.Command, args []string) error { From 738562436fdf602059118fea878ecc3f6541fa73 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 6 Nov 2019 10:02:35 -0800 Subject: [PATCH 15/17] Update tests --- command/issue_test.go | 8 ++++---- test/fixtures/{issueList.json => issueStatus.json} | 0 2 files changed, 4 insertions(+), 4 deletions(-) rename test/fixtures/{issueList.json => issueStatus.json} (100%) diff --git a/command/issue_test.go b/command/issue_test.go index 69312cac7..2181fcf37 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -8,17 +8,17 @@ import ( "github.com/github/gh-cli/test" ) -func TestIssueList(t *testing.T) { +func TestIssueStatus(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - jsonFile, _ := os.Open("../test/fixtures/issueList.json") + jsonFile, _ := os.Open("../test/fixtures/issueStatus.json") defer jsonFile.Close() http.StubResponse(200, jsonFile) - output, err := test.RunCommand(RootCmd, "issue list") + output, err := test.RunCommand(RootCmd, "issue status") if err != nil { - t.Errorf("error running command `issue list`: %v", err) + t.Errorf("error running command `issue status`: %v", err) } expectedIssues := []*regexp.Regexp{ diff --git a/test/fixtures/issueList.json b/test/fixtures/issueStatus.json similarity index 100% rename from test/fixtures/issueList.json rename to test/fixtures/issueStatus.json From 3782ed6c8d43c332bdfa19d7ae30714478a6d245 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Wed, 6 Nov 2019 10:07:24 -0800 Subject: [PATCH 16/17] Tweaks --- command/issue.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/command/issue.go b/command/issue.go index 17dc9507e..973163f36 100644 --- a/command/issue.go +++ b/command/issue.go @@ -27,7 +27,7 @@ func init() { RunE: issueList, }, &cobra.Command{ - Use: "view issue-number", + Use: "view [issue-number]", Args: cobra.MinimumNArgs(1), Short: "Open a issue in the browser", RunE: issueView, @@ -38,8 +38,6 @@ func init() { } func issueList(cmd *cobra.Command, args []string) error { - cmd.SilenceUsage = true - ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) if err != nil { @@ -90,8 +88,6 @@ func issueList(cmd *cobra.Command, args []string) error { } func issueView(cmd *cobra.Command, args []string) error { - cmd.SilenceUsage = true - ctx := contextForCommand(cmd) baseRepo, err := ctx.BaseRepo() From 524fe0a69b7121a7a00ac5d45ff839dc47cc8355 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 6 Nov 2019 19:41:18 +0100 Subject: [PATCH 17/17] :fire: last instance of mockOpenInBrowser --- command/issue_test.go | 18 ++++++++++++++---- command/pr_test.go | 1 + command/testing.go | 18 +----------------- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/command/issue_test.go b/command/issue_test.go index 2181fcf37..4571bc43e 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -2,10 +2,12 @@ package command import ( "os" + "os/exec" "regexp" "testing" "github.com/github/gh-cli/test" + "github.com/github/gh-cli/utils" ) func TestIssueStatus(t *testing.T) { @@ -43,8 +45,12 @@ func TestIssueView(t *testing.T) { defer jsonFile.Close() http.StubResponse(200, jsonFile) - teardown, callCount := mockOpenInBrowser() - defer teardown() + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() output, err := test.RunCommand(RootCmd, "issue view 8") if err != nil { @@ -55,7 +61,11 @@ func TestIssueView(t *testing.T) { t.Errorf("command output expected got an empty string") } - if *callCount != 1 { - t.Errorf("OpenInBrowser should be called 1 time but was called %d time(s)", *callCount) + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + if url != "https://github.com/OWNER/REPO/issues/8" { + t.Errorf("got: %q", url) } } diff --git a/command/pr_test.go b/command/pr_test.go index b51a3d26c..e4a4a7048 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/github/gh-cli/test" + "github.com/github/gh-cli/utils" ) func TestPRList(t *testing.T) { diff --git a/command/testing.go b/command/testing.go index 3b4717982..2e1cf9505 100644 --- a/command/testing.go +++ b/command/testing.go @@ -3,7 +3,6 @@ package command import ( "github.com/github/gh-cli/api" "github.com/github/gh-cli/context" - "github.com/github/gh-cli/utils" ) func initBlankContext(repo, branch string) { @@ -23,21 +22,6 @@ func initFakeHTTP() *api.FakeHTTP { return http } -func mockOpenInBrowser() (func(), *int) { - callCount := 0 - originalOpenInBrowser := utils.OpenInBrowser - teardown := func() { - utils.OpenInBrowser = originalOpenInBrowser - } - - utils.OpenInBrowser = func(_ string) error { - callCount++ - return nil - } - - return teardown, &callCount -} - // outputStub implements a simple utils.Runnable type outputStub struct { output []byte @@ -49,4 +33,4 @@ func (s outputStub) Output() ([]byte, error) { func (s outputStub) Run() error { return nil -} \ No newline at end of file +}