From 7c76fb645a19cfe4aed87def456fd6cef020d970 Mon Sep 17 00:00:00 2001 From: Scott Penrose Date: Sun, 9 Feb 2020 00:00:05 -0500 Subject: [PATCH 01/15] Add repo view command ``` $ gh repo Work with GitHub repositories. A repository can be supplied as an argument in any of the following formats: - by owner/repo, e.g. "cli/cli" - by URL, e.g. "https://github.com/cli/cli" Usage: gh repo [command] Available Commands: view View a repository in the browser ``` --- command/repo.go | 57 ++++++++++++++++++++++++++++ command/repo_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+) create mode 100644 command/repo.go create mode 100644 command/repo_test.go diff --git a/command/repo.go b/command/repo.go new file mode 100644 index 000000000..e453d8b61 --- /dev/null +++ b/command/repo.go @@ -0,0 +1,57 @@ +package command + +import ( + "fmt" + "strings" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +func init() { + RootCmd.AddCommand(repoCmd) + repoCmd.AddCommand(repoViewCmd) +} + +var repoCmd = &cobra.Command{ + Use: "repo", + Short: "View repositories", + Long: `Work with GitHub repositories. + +A repository can be supplied as an argument in any of the following formats: +- by owner/repo, e.g. "cli/cli" +- by URL, e.g. "https://github.com/cli/cli"`, +} + +var repoViewCmd = &cobra.Command{ + Use: "view [{ | }]", + Short: "View a repository in the browser", + Long: `View a repository specified by the argument in the browser. + +Without an argument, the repository that belongs to the current +branch is opened.`, + RunE: repoView, +} + +func repoView(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + var openURL string + if len(args) == 0 { + openURL = fmt.Sprintf("https://github.com/%s", ghrepo.FullName(*baseRepo)) + } else { + if strings.HasPrefix(args[0], "http") { + openURL = args[0] + } else { + openURL = fmt.Sprintf("https://github.com/%s", args[0]) + } + } + + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) + return utils.OpenInBrowser(openURL) +} diff --git a/command/repo_test.go b/command/repo_test.go new file mode 100644 index 000000000..30da540b0 --- /dev/null +++ b/command/repo_test.go @@ -0,0 +1,89 @@ +package command + +import ( + "os/exec" + "testing" + + "github.com/cli/cli/utils" +) + +func TestRepoView(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(repoViewCmd, "repo view") + if err != nil { + t.Errorf("error running command `repo view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO") +} + +func TestRepoView_ownerRepo(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(repoViewCmd, "repo view cli/cli") + if err != nil { + t.Errorf("error running command `repo view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/cli/cli in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/cli/cli") +} + +func TestRepoView_fullURL(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(repoViewCmd, "repo view https://github.com/cli/cli") + if err != nil { + t.Errorf("error running command `repo view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/cli/cli in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/cli/cli") +} From ac94ae587202f72578291b3e19a027a4e4efb9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 13:36:34 +0100 Subject: [PATCH 02/15] Return interface from `determineBaseRepo()`, not pointer to interface It's sufficient to return a value of type `ghrepo.Interface` instead of a pointer to an interface. This avoids having to use `*` whenever we are passing the result of `determineBaseRepo()` into another function that accepts a `ghrepo.Interface`. --- command/issue.go | 20 ++++++++++---------- command/pr.go | 18 +++++++++--------- command/root.go | 7 +++---- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/command/issue.go b/command/issue.go index 3ba00ad4c..0fb1dc3b1 100644 --- a/command/issue.go +++ b/command/issue.go @@ -109,9 +109,9 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(*baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nIssues for %s\n\n", ghrepo.FullName(baseRepo)) - issues, err := api.IssueList(apiClient, *baseRepo, state, labels, assignee, limit) + issues, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit) if err != nil { return err } @@ -169,7 +169,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - issuePayload, err := api.IssueStatus(apiClient, *baseRepo, currentUser) + issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) if err != nil { return err } @@ -177,7 +177,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(*baseRepo)) + fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Issues assigned to you") @@ -221,7 +221,7 @@ func issueView(cmd *cobra.Command, args []string) error { return err } - issue, err := issueFromArg(apiClient, *baseRepo, args[0]) + issue, err := issueFromArg(apiClient, baseRepo, args[0]) if err != nil { return err } @@ -294,7 +294,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(*baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) baseOverride, err := cmd.Flags().GetString("repo") if err != nil { @@ -311,7 +311,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository - openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(*baseRepo)) + openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(baseRepo)) if len(templateFiles) > 1 { openURL += "/choose" } @@ -324,12 +324,12 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - repo, err := api.GitHubRepo(apiClient, *baseRepo) + repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err } if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(*baseRepo)) + return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) } action := SubmitAction @@ -370,7 +370,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if action == PreviewAction { openURL := fmt.Sprintf( "https://github.com/%s/issues/new/?title=%s&body=%s", - ghrepo.FullName(*baseRepo), + ghrepo.FullName(baseRepo), url.QueryEscape(title), url.QueryEscape(body), ) diff --git a/command/pr.go b/command/pr.go index 9d3e2487e..cd767f13d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -85,7 +85,7 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - prPayload, err := api.PullRequests(apiClient, *baseRepo, currentPRNumber, currentPRHeadRef, currentUser) + prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) if err != nil { return err } @@ -93,7 +93,7 @@ func prStatus(cmd *cobra.Command, args []string) error { out := colorableOut(cmd) fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(*baseRepo)) + fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") printHeader(out, "Current branch") @@ -136,7 +136,7 @@ func prList(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(*baseRepo)) + fmt.Fprintf(colorableErr(cmd), "\nPull requests for %s\n\n", ghrepo.FullName(baseRepo)) limit, err := cmd.Flags().GetInt("limit") if err != nil { @@ -174,8 +174,8 @@ func prList(cmd *cobra.Command, args []string) error { } params := map[string]interface{}{ - "owner": (*baseRepo).RepoOwner(), - "repo": (*baseRepo).RepoName(), + "owner": baseRepo.RepoOwner(), + "repo": baseRepo.RepoName(), "state": graphqlState, } if len(labels) > 0 { @@ -261,7 +261,7 @@ func prView(cmd *cobra.Command, args []string) error { var openURL string var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, *baseRepo, args[0]) + pr, err = prFromArg(apiClient, baseRepo, args[0]) if err != nil { return err } @@ -273,15 +273,15 @@ func prView(cmd *cobra.Command, args []string) error { } if prNumber > 0 { - openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(*baseRepo), prNumber) + openURL = fmt.Sprintf("https://github.com/%s/pull/%d", ghrepo.FullName(baseRepo), prNumber) if preview { - pr, err = api.PullRequestByNumber(apiClient, *baseRepo, prNumber) + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) if err != nil { return err } } } else { - pr, err = api.PullRequestForBranch(apiClient, *baseRepo, branchWithOwner) + pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) if err != nil { return err } diff --git a/command/root.go b/command/root.go index 0257b6a48..5130f74c9 100644 --- a/command/root.go +++ b/command/root.go @@ -171,7 +171,7 @@ func changelogURL(version string) string { return url } -func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interface, error) { +func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) { apiClient, err := apiClientForContext(ctx) if err != nil { return nil, err @@ -192,11 +192,10 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (*ghrepo.Interfa return nil, err } - var baseRepo ghrepo.Interface - baseRepo, err = repoContext.BaseRepo() + baseRepo, err := repoContext.BaseRepo() if err != nil { return nil, err } - return &baseRepo, nil + return baseRepo, nil } From 38b58a491420e240c621ba8da68874c0e8f46e0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 13:35:36 +0100 Subject: [PATCH 03/15] Respect explicit title & body with `issue create --web` --- command/issue.go | 32 +++++++++++++++++++------------- command/issue_test.go | 28 +++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/command/issue.go b/command/issue.go index 0fb1dc3b1..686baaa99 100644 --- a/command/issue.go +++ b/command/issue.go @@ -294,8 +294,6 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) - baseOverride, err := cmd.Flags().GetString("repo") if err != nil { return err @@ -309,16 +307,33 @@ func issueCreate(cmd *cobra.Command, args []string) error { } } + title, err := cmd.Flags().GetString("title") + if err != nil { + return fmt.Errorf("could not parse title: %w", err) + } + body, err := cmd.Flags().GetString("body") + if err != nil { + return fmt.Errorf("could not parse body: %w", err) + } + if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { // TODO: move URL generation into GitHubRepository openURL := fmt.Sprintf("https://github.com/%s/issues/new", ghrepo.FullName(baseRepo)) - if len(templateFiles) > 1 { + if title != "" || body != "" { + openURL += fmt.Sprintf( + "?title=%s&body=%s", + url.QueryEscape(title), + url.QueryEscape(body), + ) + } else if len(templateFiles) > 1 { openURL += "/choose" } - cmd.Printf("Opening %s in your browser.\n", openURL) + cmd.Printf("Opening %s in your browser.\n", displayURL(openURL)) return utils.OpenInBrowser(openURL) } + fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) + apiClient, err := apiClientForContext(ctx) if err != nil { return err @@ -334,15 +349,6 @@ func issueCreate(cmd *cobra.Command, args []string) error { action := SubmitAction - title, err := cmd.Flags().GetString("title") - if err != nil { - return fmt.Errorf("could not parse title: %w", err) - } - body, err := cmd.Flags().GetString("body") - if err != nil { - return fmt.Errorf("could not parse body: %w", err) - } - interactive := title == "" || body == "" if interactive { diff --git a/command/issue_test.go b/command/issue_test.go index c09de2c62..41aedb178 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -492,5 +492,31 @@ func TestIssueCreate_web(t *testing.T) { } url := seenCmd.Args[len(seenCmd.Args)-1] eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output.String(), "Opening https://github.com/OWNER/REPO/issues/new in your browser.\n") + eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") + eq(t, output.Stderr(), "") +} + +func TestIssueCreate_webTitleBody(t *testing.T) { + initBlankContext("OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(issueCreateCmd, `issue create -w -t mytitle -b mybody`) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/issues/new?title=mytitle&body=mybody") + eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") } From f6eb710462e9a920b5e4e0c543626686da6e4646 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 13:53:37 +0100 Subject: [PATCH 04/15] Fix test expectation on Windows On Windows, `&` characters in URLs need to be escaped with `^`, but that messes up the test expectation for other platforms, so this normalizes it. --- command/issue_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/command/issue_test.go b/command/issue_test.go index 41aedb178..f8a5d6ad9 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "regexp" + "strings" "testing" "github.com/cli/cli/utils" @@ -516,7 +517,7 @@ func TestIssueCreate_webTitleBody(t *testing.T) { if seenCmd == nil { t.Fatal("expected a command to run") } - url := seenCmd.Args[len(seenCmd.Args)-1] + url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") eq(t, url, "https://github.com/OWNER/REPO/issues/new?title=mytitle&body=mybody") eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") } From 20fc0dfcb74ca7497458226ef834d3b1d09a0db0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 17:14:46 +0100 Subject: [PATCH 05/15] Avoid API requests in `repo view` when repo argument is given --- command/repo.go | 8 ++++---- command/repo_test.go | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/command/repo.go b/command/repo.go index e453d8b61..d77e2efff 100644 --- a/command/repo.go +++ b/command/repo.go @@ -36,13 +36,13 @@ branch is opened.`, func repoView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := determineBaseRepo(cmd, ctx) - if err != nil { - return err - } var openURL string if len(args) == 0 { + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } openURL = fmt.Sprintf("https://github.com/%s", ghrepo.FullName(*baseRepo)) } else { if strings.HasPrefix(args[0], "http") { diff --git a/command/repo_test.go b/command/repo_test.go index 30da540b0..f156f1283 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -4,6 +4,7 @@ import ( "os/exec" "testing" + "github.com/cli/cli/context" "github.com/cli/cli/utils" ) @@ -35,9 +36,12 @@ func TestRepoView(t *testing.T) { } func TestRepoView_ownerRepo(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") + ctx := context.NewBlank() + ctx.SetBranch("master") + initContext = func() context.Context { + return ctx + } + initFakeHTTP() var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { @@ -62,9 +66,12 @@ func TestRepoView_ownerRepo(t *testing.T) { } func TestRepoView_fullURL(t *testing.T) { - initBlankContext("OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") + ctx := context.NewBlank() + ctx.SetBranch("master") + initContext = func() context.Context { + return ctx + } + initFakeHTTP() var seenCmd *exec.Cmd restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { From 398a5defb86f6c6167bbf7d93c479c29b0d5750c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 17:17:00 +0100 Subject: [PATCH 06/15] Use `displayURL` helper for consistency --- command/repo.go | 2 +- command/repo_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/command/repo.go b/command/repo.go index d77e2efff..472b2e83f 100644 --- a/command/repo.go +++ b/command/repo.go @@ -52,6 +52,6 @@ func repoView(cmd *cobra.Command, args []string) error { } } - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) + fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", displayURL(openURL)) return utils.OpenInBrowser(openURL) } diff --git a/command/repo_test.go b/command/repo_test.go index f156f1283..6c4d6f1ef 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -26,7 +26,7 @@ func TestRepoView(t *testing.T) { } eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO in your browser.\n") + eq(t, output.Stderr(), "Opening github.com/OWNER/REPO in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -56,7 +56,7 @@ func TestRepoView_ownerRepo(t *testing.T) { } eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/cli/cli in your browser.\n") + eq(t, output.Stderr(), "Opening github.com/cli/cli in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") @@ -86,7 +86,7 @@ func TestRepoView_fullURL(t *testing.T) { } eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/cli/cli in your browser.\n") + eq(t, output.Stderr(), "Opening github.com/cli/cli in your browser.\n") if seenCmd == nil { t.Fatal("expected a command to run") From e87467d21fade4e6b3105f82892c80944c8f3154 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 17:17:14 +0100 Subject: [PATCH 07/15] Allow `repo view OWNER/REPO` format for owners starting with "http" --- command/repo.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/command/repo.go b/command/repo.go index 472b2e83f..2c0bdabfb 100644 --- a/command/repo.go +++ b/command/repo.go @@ -20,17 +20,16 @@ var repoCmd = &cobra.Command{ Long: `Work with GitHub repositories. A repository can be supplied as an argument in any of the following formats: -- by owner/repo, e.g. "cli/cli" -- by URL, e.g. "https://github.com/cli/cli"`, +- "OWNER/REPO" +- by URL, e.g. "https://github.com/OWNER/REPO"`, } var repoViewCmd = &cobra.Command{ - Use: "view [{ | }]", + Use: "view []", Short: "View a repository in the browser", - Long: `View a repository specified by the argument in the browser. + Long: `View a GitHub repository in the browser. -Without an argument, the repository that belongs to the current -branch is opened.`, +With no argument, the repository for the current directory is opened.`, RunE: repoView, } @@ -45,10 +44,11 @@ func repoView(cmd *cobra.Command, args []string) error { } openURL = fmt.Sprintf("https://github.com/%s", ghrepo.FullName(*baseRepo)) } else { - if strings.HasPrefix(args[0], "http") { - openURL = args[0] + repoArg := args[0] + if strings.HasPrefix(repoArg, "http:/") || strings.HasPrefix(repoArg, "https:/") { + openURL = repoArg } else { - openURL = fmt.Sprintf("https://github.com/%s", args[0]) + openURL = fmt.Sprintf("https://github.com/%s", repoArg) } } From 53532d6f28c2e1b8f7e8f04b8dab8d948a15f43b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 19:33:18 +0100 Subject: [PATCH 08/15] Make sure `git push` output shows up during `pr create` --- git/git.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/git/git.go b/git/git.go index 92d2dfd0f..dfd45bade 100644 --- a/git/git.go +++ b/git/git.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net/url" + "os" "os/exec" "regexp" "strings" @@ -70,8 +71,11 @@ func UncommittedChangeCount() (int, error) { return count, nil } +// Push publishes a git ref to a remote and sets up upstream configuration func Push(remote string, ref string) error { pushCmd := GitCommand("push", "--set-upstream", remote, ref) + pushCmd.Stdout = os.Stdout + pushCmd.Stderr = os.Stderr return utils.PrepareCmd(pushCmd).Run() } From c10f2ff9455abbf1de74511b83bf274ab4ce7601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 19:36:54 +0100 Subject: [PATCH 09/15] Communicate to the user that we're waiting before retrying `git push` When the user has just created a fork, it might not yet be ready for writing. This ensures that the wait period between retries is communicated to the user on stderr. --- command/pr_create.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/command/pr_create.go b/command/pr_create.go index 64f43c895..c40f5d8c9 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -99,7 +99,9 @@ func prCreate(cmd *cobra.Command, _ []string) error { if didForkRepo && pushTries < maxPushTries { pushTries++ // first wait 2 seconds after forking, then 4s, then 6s - time.Sleep(time.Duration(2*pushTries) * time.Second) + waitSeconds := 2 * pushTries + fmt.Fprintf(cmd.ErrOrStderr(), "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second")) + time.Sleep(time.Duration(waitSeconds) * time.Second) continue } return err From 1bf8beb96d3bc885ec12bdf8587c264ea9bc3c10 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 24 Feb 2020 13:22:43 -0600 Subject: [PATCH 10/15] fix baseRepo type --- command/repo.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/repo.go b/command/repo.go index 2c0bdabfb..b00c3d87b 100644 --- a/command/repo.go +++ b/command/repo.go @@ -42,7 +42,7 @@ func repoView(cmd *cobra.Command, args []string) error { if err != nil { return err } - openURL = fmt.Sprintf("https://github.com/%s", ghrepo.FullName(*baseRepo)) + openURL = fmt.Sprintf("https://github.com/%s", ghrepo.FullName(baseRepo)) } else { repoArg := args[0] if strings.HasPrefix(repoArg, "http:/") || strings.HasPrefix(repoArg, "https:/") { From 327dae95a30b31eed077b733c0664f8bba6f9dfd Mon Sep 17 00:00:00 2001 From: Dasio Date: Mon, 24 Feb 2020 21:18:34 +0100 Subject: [PATCH 11/15] Use break label instead of goto --- api/queries_pr.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index 734245acb..b6fdcb271 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -497,7 +497,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] } }` - prs := []PullRequest{} + var prs []PullRequest pageLimit := min(limit, 100) variables := map[string]interface{}{} @@ -555,7 +555,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] variables[name] = val } } - +loop: for { variables["limit"] = pageLimit var data response @@ -571,17 +571,16 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] for _, edge := range prData.Edges { prs = append(prs, edge.Node) if len(prs) == limit { - goto done + break loop } } if prData.PageInfo.HasNextPage { variables["endCursor"] = prData.PageInfo.EndCursor pageLimit = min(pageLimit, limit-len(prs)) - continue + } else { + break } - done: - break } return prs, nil From ad0dedd1acd77064df05a3b778a641d10e43355e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 22:11:09 +0100 Subject: [PATCH 12/15] Enable `issue list` pagination Makes it possible to set a `--limit` greater than 100. --- api/queries_issue.go | 53 ++++++++++++++++++++++-------- api/queries_issue_test.go | 68 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 13 deletions(-) create mode 100644 api/queries_issue_test.go diff --git a/api/queries_issue.go b/api/queries_issue.go index 934467e05..c0fca27e4 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -185,20 +185,23 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str } query := fragments + ` - query($owner: String!, $repo: String!, $limit: Int, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { + query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { repository(owner: $owner, name: $repo) { hasIssuesEnabled - issues(first: $limit, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { + issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { nodes { ...issue } + pageInfo { + hasNextPage + endCursor + } } } } ` variables := map[string]interface{}{ - "limit": limit, "owner": repo.RepoOwner(), "repo": repo.RepoName(), "states": states, @@ -210,25 +213,49 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str variables["assignee"] = assigneeString } - var resp struct { + var response struct { Repository struct { Issues struct { - Nodes []Issue + Nodes []Issue + PageInfo struct { + HasNextPage bool + EndCursor string + } } HasIssuesEnabled bool } } - err := client.GraphQL(query, variables, &resp) - if err != nil { - return nil, err + var issues []Issue + pageLimit := min(limit, 100) + +loop: + for { + variables["limit"] = pageLimit + err := client.GraphQL(query, variables, &response) + if err != nil { + return nil, err + } + if !response.Repository.HasIssuesEnabled { + return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) + } + + for _, issue := range response.Repository.Issues.Nodes { + issues = append(issues, issue) + if len(issues) == limit { + break loop + } + } + + if response.Repository.Issues.PageInfo.HasNextPage { + variables["endCursor"] = response.Repository.Issues.PageInfo.EndCursor + pageLimit = min(pageLimit, limit-len(issues)) + } else { + break + } } - if !resp.Repository.HasIssuesEnabled { - return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) - } - - return resp.Repository.Issues.Nodes, nil + return issues, nil } func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, error) { diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go new file mode 100644 index 000000000..bde6b8696 --- /dev/null +++ b/api/queries_issue_test.go @@ -0,0 +1,68 @@ +package api + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "testing" + + "github.com/cli/cli/internal/ghrepo" +) + +func TestIssueList(t *testing.T) { + http := &FakeHTTP{} + client := NewClient(ReplaceTripper(http)) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { + "nodes": [], + "pageInfo": { + "hasNextPage": true, + "endCursor": "ENDCURSOR" + } + } + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { + "nodes": [], + "pageInfo": { + "hasNextPage": false, + "endCursor": "ENDCURSOR" + } + } + } } } + `)) + + _, err := IssueList(client, ghrepo.FromFullName("OWNER/REPO"), "open", []string{}, "", 251) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if len(http.Requests) != 2 { + t.Fatalf("expected 2 HTTP requests, seen %d", len(http.Requests)) + } + var reqBody struct { + Query string + Variables map[string]interface{} + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + json.Unmarshal(bodyBytes, &reqBody) + if reqLimit := reqBody.Variables["limit"].(float64); reqLimit != 100 { + t.Errorf("expected 100, got %v", reqLimit) + } + if _, cursorPresent := reqBody.Variables["endCursor"]; cursorPresent { + t.Error("did not expect first request to pass 'endCursor'") + } + + bodyBytes, _ = ioutil.ReadAll(http.Requests[1].Body) + json.Unmarshal(bodyBytes, &reqBody) + if endCursor := reqBody.Variables["endCursor"].(string); endCursor != "ENDCURSOR" { + t.Errorf("expected %q, got %q", "ENDCURSOR", endCursor) + } +} From a5445441413da9d8b6f3095955a1f29b4c221608 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 24 Feb 2020 22:21:45 +0100 Subject: [PATCH 13/15] Re-indent graphql query --- api/queries_issue.go | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/api/queries_issue.go b/api/queries_issue.go index c0fca27e4..2c755b1d3 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -185,21 +185,21 @@ func IssueList(client *Client, repo ghrepo.Interface, state string, labels []str } query := fragments + ` - query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { - repository(owner: $owner, name: $repo) { - hasIssuesEnabled - issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { - nodes { - ...issue - } - pageInfo { - hasNextPage - endCursor - } - } - } - } - ` + query($owner: String!, $repo: String!, $limit: Int, $endCursor: String, $states: [IssueState!] = OPEN, $labels: [String!], $assignee: String) { + repository(owner: $owner, name: $repo) { + hasIssuesEnabled + issues(first: $limit, after: $endCursor, orderBy: {field: CREATED_AT, direction: DESC}, states: $states, labels: $labels, filterBy: {assignee: $assignee}) { + nodes { + ...issue + } + pageInfo { + hasNextPage + endCursor + } + } + } + } + ` variables := map[string]interface{}{ "owner": repo.RepoOwner(), From d25ec726adec3bc5b3112233103eb44bebd7051b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 25 Feb 2020 16:47:42 +0100 Subject: [PATCH 14/15] Add `repo clone ` command --- command/repo.go | 30 +++++++++++++++++++++++++ command/repo_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/command/repo.go b/command/repo.go index b00c3d87b..484ef609c 100644 --- a/command/repo.go +++ b/command/repo.go @@ -2,8 +2,10 @@ package command import ( "fmt" + "os" "strings" + "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -11,6 +13,7 @@ import ( func init() { RootCmd.AddCommand(repoCmd) + repoCmd.AddCommand(repoCloneCmd) repoCmd.AddCommand(repoViewCmd) } @@ -24,6 +27,16 @@ A repository can be supplied as an argument in any of the following formats: - by URL, e.g. "https://github.com/OWNER/REPO"`, } +var repoCloneCmd = &cobra.Command{ + Use: "clone ", + Args: cobra.MinimumNArgs(1), + Short: "Clone a repository locally", + Long: `Clone a GitHub repository locally. + +To pass 'git clone' options, separate them with '--'.`, + RunE: repoClone, +} + var repoViewCmd = &cobra.Command{ Use: "view []", Short: "View a repository in the browser", @@ -33,6 +46,23 @@ With no argument, the repository for the current directory is opened.`, RunE: repoView, } +func repoClone(cmd *cobra.Command, args []string) error { + cloneURL := args[0] + if !strings.Contains(cloneURL, ":") { + cloneURL = fmt.Sprintf("https://github.com/%s.git", cloneURL) + } + + cloneArgs := []string{"clone"} + cloneArgs = append(cloneArgs, args[1:]...) + cloneArgs = append(cloneArgs, cloneURL) + + cloneCmd := git.GitCommand(cloneArgs...) + cloneCmd.Stdin = os.Stdin + cloneCmd.Stdout = os.Stdout + cloneCmd.Stderr = os.Stderr + return utils.PrepareCmd(cloneCmd).Run() +} + func repoView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) diff --git a/command/repo_test.go b/command/repo_test.go index 6c4d6f1ef..588ae3520 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -2,12 +2,65 @@ package command import ( "os/exec" + "strings" "testing" "github.com/cli/cli/context" "github.com/cli/cli/utils" ) +func TestRepoClone(t *testing.T) { + tests := []struct { + name string + args string + want string + }{ + { + name: "shorthand", + args: "repo clone OWNER/REPO", + want: "git clone https://github.com/OWNER/REPO.git", + }, + { + name: "clone arguments", + args: "repo clone OWNER/REPO -- -o upstream --depth 1", + want: "git clone -o upstream --depth 1 https://github.com/OWNER/REPO.git", + }, + { + name: "HTTPS URL", + args: "repo clone https://github.com/OWNER/REPO", + want: "git clone https://github.com/OWNER/REPO", + }, + { + name: "SSH URL", + args: "repo clone git@github.com:OWNER/REPO.git", + want: "git clone git@github.com:OWNER/REPO.git", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var seenCmd *exec.Cmd + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + seenCmd = cmd + return &outputStub{} + }) + defer restoreCmd() + + output, err := RunCommand(repoViewCmd, tt.args) + if err != nil { + t.Fatalf("error running command `repo clone`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + eq(t, strings.Join(seenCmd.Args, " "), tt.want) + }) + } +} + func TestRepoView(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() From 2a42f61d8d676c84acd5ab008efed9b9eeada8d1 Mon Sep 17 00:00:00 2001 From: rista404 Date: Tue, 25 Feb 2020 18:47:44 +0100 Subject: [PATCH 15/15] Use gray color for PR number if PR is a draft --- api/queries_pr.go | 4 ++++ command/pr.go | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index b6fdcb271..f18b0bad0 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -40,6 +40,7 @@ type PullRequest struct { } } IsCrossRepository bool + IsDraft bool MaintainerCanModify bool ReviewDecision string @@ -156,6 +157,7 @@ func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, cu login } isCrossRepository + isDraft commits(last: 1) { nodes { commit { @@ -366,6 +368,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) login } isCrossRepository + isDraft } } } @@ -460,6 +463,7 @@ func PullRequestList(client *Client, vars map[string]interface{}, limit int) ([] login } isCrossRepository + isDraft } ` diff --git a/command/pr.go b/command/pr.go index cd767f13d..221ef0bc4 100644 --- a/command/pr.go +++ b/command/pr.go @@ -214,7 +214,7 @@ func prList(cmd *cobra.Command, args []string) error { if table.IsTTY() { prNum = "#" + prNum } - table.AddField(prNum, nil, colorFuncForState(pr.State)) + table.AddField(prNum, nil, colorFuncForPR(pr)) table.AddField(replaceExcessiveWhitespace(pr.Title), nil, nil) table.AddField(pr.HeadLabel(), nil, utils.Cyan) table.EndRow() @@ -227,6 +227,14 @@ func prList(cmd *cobra.Command, args []string) error { return nil } +func colorFuncForPR(pr api.PullRequest) func(string) string { + if pr.State == "OPEN" && pr.IsDraft { + return utils.Gray + } else { + return colorFuncForState(pr.State) + } +} + func colorFuncForState(state string) func(string) string { switch state { case "OPEN": @@ -385,7 +393,13 @@ func prSelectorForCurrentBranch(ctx context.Context) (prNumber int, prHeadRef st func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { for _, pr := range prs { prNumber := fmt.Sprintf("#%d", pr.Number) - fmt.Fprintf(w, " %s %s %s", utils.Green(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) + + prNumberColorFunc := utils.Green + if pr.IsDraft { + prNumberColorFunc = utils.Gray + } + + fmt.Fprintf(w, " %s %s %s", prNumberColorFunc(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) checks := pr.ChecksStatus() reviews := pr.ReviewStatus()