diff --git a/.goreleaser.yml b/.goreleaser.yml index ceb22c12e..0909fa8f7 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -82,6 +82,7 @@ nfpms: bindir: /usr/local dependencies: - git + description: GitHub’s official command line tool. formats: - deb - rpm diff --git a/README.md b/README.md index aae399b64..227090279 100644 --- a/README.md +++ b/README.md @@ -7,11 +7,11 @@ the terminal next to where you are already working with `git` and your code. ## Availability -While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It currently does not support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning on adding support for GitHub Enterprise Server after GitHub CLI is out of beta (likely towards the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. ## We need your feedback -GitHub CLI is currently early in its development, and we're hoping to get feedback from people using it. +GitHub CLI is currently in its early development stages, and we're hoping to get feedback from people using it. If you've installed and used `gh`, we'd love for you to take a short survey here (no more than five minutes): https://forms.gle/umxd3h31c7aMQFKG7 @@ -31,9 +31,9 @@ Read the [official docs](https://cli.github.com/manual/) for more information. ## Comparison with hub -For many years, [hub][] was the unofficial GitHub CLI tool. `gh` is a new project for us to explore +For many years, [hub][] was the unofficial GitHub CLI tool. `gh` is a new project that helps us explore what an official GitHub CLI tool can look like with a fundamentally different design. While both -tools bring GitHub to the terminal, `hub` behaves as a proxy to `git` and `gh` is a standalone +tools bring GitHub to the terminal, `hub` behaves as a proxy to `git`, and `gh` is a standalone tool. Check out our [more detailed explanation](/docs/gh-vs-hub.md) to learn more. @@ -46,15 +46,31 @@ tool. Check out our [more detailed explanation](/docs/gh-vs-hub.md) to learn mor #### Homebrew -Install: `brew install github/gh/gh` +Install: -Upgrade: `brew upgrade gh` +```bash +brew install github/gh/gh +``` + +Upgrade: + +```bash +brew upgrade gh +``` #### MacPorts -Install: `sudo port install gh` +Install: -Upgrade: `sudo port selfupdate && sudo port upgrade gh` +```bash +sudo port install gh +``` + +Upgrade: + +```bash +sudo port selfupdate && sudo port upgrade gh +``` ### Windows @@ -64,24 +80,28 @@ Upgrade: `sudo port selfupdate && sudo port upgrade gh` Install: -``` +```powershell scoop bucket add github-gh https://github.com/cli/scoop-gh.git scoop install gh ``` -Upgrade: `scoop update gh` +Upgrade: + +```powershell +scoop update gh +``` #### Chocolatey Install: -``` +```powershell choco install gh ``` Upgrade: -``` +```powershell choco upgrade gh ``` @@ -122,7 +142,7 @@ Install and upgrade: Arch Linux users can install from the AUR: https://aur.archlinux.org/packages/github-cli/ ```bash -$ yay -S github-cli +yay -S github-cli ``` ### Other platforms diff --git a/api/queries_pr.go b/api/queries_pr.go index 188e708a8..00d01661a 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -540,8 +540,12 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea headRepositoryOwner { login } + headRepository { + name + } isCrossRepository isDraft + maintainerCanModify reviewRequests(first: 100) { nodes { requestedReviewer { diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 6391842d4..bac165995 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -56,6 +56,9 @@ func main() { printError(os.Stderr, err, cmd, hasDebug) os.Exit(1) } + if command.HasFailed() { + os.Exit(1) + } newRelease := <-updateMessageChan if newRelease != nil { diff --git a/command/alias.go b/command/alias.go index 64056f9d5..1b21fcc61 100644 --- a/command/alias.go +++ b/command/alias.go @@ -45,7 +45,7 @@ that follow the invocation of an alias will be inserted appropriately.`, Args: cobra.MinimumNArgs(2), RunE: aliasSet, - // NB: this allows a user to eschew quotes when specifiying an alias expansion. We'll have to + // NB: this allows a user to eschew quotes when specifying an alias expansion. We'll have to // revisit it if we ever want to add flags to alias set but we have no current plans for that. DisableFlagParsing: true, } diff --git a/command/help.go b/command/help.go index b776d23f1..232647d62 100644 --- a/command/help.go +++ b/command/help.go @@ -9,32 +9,67 @@ import ( "github.com/spf13/cobra" ) -func rootHelpFunc(command *cobra.Command, args []string) { - // Display helpful error message in case subcommand name was mistyped. - // This matches Cobra's behavior for root command, which Cobra - // confusingly doesn't apply to nested commands. - if command != RootCmd { - if command.Parent() == RootCmd && len(args) >= 2 { - if command.SuggestionsMinimumDistance <= 0 { - command.SuggestionsMinimumDistance = 2 +func rootUsageFunc(command *cobra.Command) error { + command.Printf("Usage: %s", command.UseLine()) + + subcommands := command.Commands() + if len(subcommands) > 0 { + command.Print("\n\nAvailable commands:\n") + for _, c := range subcommands { + if c.Hidden { + continue } - candidates := command.SuggestionsFor(args[1]) - - errOut := command.OutOrStderr() - fmt.Fprintf(errOut, "unknown command %q for %q\n", args[1], "gh "+args[0]) - - if len(candidates) > 0 { - fmt.Fprint(errOut, "\nDid you mean this?\n") - for _, c := range candidates { - fmt.Fprintf(errOut, "\t%s\n", c) - } - fmt.Fprint(errOut, "\n") - } - - oldOut := command.OutOrStdout() - command.SetOut(errOut) - defer command.SetOut(oldOut) + command.Printf(" %s\n", c.Name()) } + return nil + } + + flagUsages := command.LocalFlags().FlagUsages() + if flagUsages != "" { + command.Printf("\n\nFlags:\n%s", flagUsages) + } + return nil +} + +var hasFailed bool + +// HasFailed signals that the main process should exit with non-zero status +func HasFailed() bool { + return hasFailed +} + +// Display helpful error message in case subcommand name was mistyped. +// This matches Cobra's behavior for root command, which Cobra +// confusingly doesn't apply to nested commands. +func nestedSuggestFunc(command *cobra.Command, arg string) { + command.Printf("unknown command %q for %q\n", arg, command.CommandPath()) + + var candidates []string + if arg == "help" { + candidates = []string{"--help"} + } else { + if command.SuggestionsMinimumDistance <= 0 { + command.SuggestionsMinimumDistance = 2 + } + candidates = command.SuggestionsFor(arg) + } + + if len(candidates) > 0 { + command.Print("\nDid you mean this?\n") + for _, c := range candidates { + command.Printf("\t%s\n", c) + } + } + + command.Print("\n") + _ = rootUsageFunc(command) +} + +func rootHelpFunc(command *cobra.Command, args []string) { + if command.Parent() == RootCmd && len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { + nestedSuggestFunc(command, args[1]) + hasFailed = true + return } coreCommands := []string{} @@ -85,6 +120,11 @@ func rootHelpFunc(command *cobra.Command, args []string) { dedent := regexp.MustCompile(`(?m)^ `) helpEntries = append(helpEntries, helpEntry{"FLAGS", dedent.ReplaceAllString(flagUsages, "")}) } + inheritedFlagUsages := command.InheritedFlags().FlagUsages() + if inheritedFlagUsages != "" { + dedent := regexp.MustCompile(`(?m)^ `) + helpEntries = append(helpEntries, helpEntry{"INHERITED FLAGS", dedent.ReplaceAllString(inheritedFlagUsages, "")}) + } if _, ok := command.Annotations["help:arguments"]; ok { helpEntries = append(helpEntries, helpEntry{"ARGUMENTS", command.Annotations["help:arguments"]}) } diff --git a/command/issue.go b/command/issue.go index c00d624af..daf974e2c 100644 --- a/command/issue.go +++ b/command/issue.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/githubtemplate" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -30,14 +31,14 @@ func init() { issueCreateCmd.Flags().StringP("body", "b", "", "Supply a body. Will prompt for one otherwise.") issueCreateCmd.Flags().BoolP("web", "w", false, "Open the browser to create an issue") - issueCreateCmd.Flags().StringSliceP("assignee", "a", nil, "Assign a person by their `login`") - issueCreateCmd.Flags().StringSliceP("label", "l", nil, "Add a label by `name`") - issueCreateCmd.Flags().StringSliceP("project", "p", nil, "Add the issue to a project by `name`") + issueCreateCmd.Flags().StringSliceP("assignee", "a", nil, "Assign people by their `login`") + issueCreateCmd.Flags().StringSliceP("label", "l", nil, "Add labels by `name`") + issueCreateCmd.Flags().StringSliceP("project", "p", nil, "Add the issue to projects by `name`") issueCreateCmd.Flags().StringP("milestone", "m", "", "Add the issue to a milestone by `name`") issueCmd.AddCommand(issueListCmd) issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by label") + issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels") issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") issueListCmd.Flags().StringP("author", "A", "", "Filter by author") @@ -69,16 +70,30 @@ var issueCmd = &cobra.Command{ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", + Args: cmdutil.NoArgsQuoteReminder, RunE: issueCreate, + Example: heredoc.Doc(` + $ gh issue create --title "I found a bug" --body "Nothing works" + $ gh issue create --label "bug,help wanted" + $ gh issue create --label bug --label "help wanted" + $ gh issue create --assignee monalisa,hubot + $ gh issue create --project "Roadmap" + `), } var issueListCmd = &cobra.Command{ Use: "list", Short: "List and filter issues in this repository", - RunE: issueList, + Example: heredoc.Doc(` + $ gh issue list -l "help wanted" + $ gh issue list -A monalisa + `), + Args: cmdutil.NoArgsQuoteReminder, + RunE: issueList, } var issueStatusCmd = &cobra.Command{ Use: "status", Short: "Show status of relevant issues", + Args: cmdutil.NoArgsQuoteReminder, RunE: issueStatus, } var issueViewCmd = &cobra.Command{ diff --git a/command/pr.go b/command/pr.go index c5fc1ae7b..2068a10e8 100644 --- a/command/pr.go +++ b/command/pr.go @@ -15,6 +15,7 @@ import ( "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -65,6 +66,7 @@ var prCmd = &cobra.Command{ var prListCmd = &cobra.Command{ Use: "list", Short: "List and filter pull requests in this repository", + Args: cmdutil.NoArgsQuoteReminder, Example: heredoc.Doc(` $ gh pr list --limit 999 $ gh pr list --state closed @@ -75,6 +77,7 @@ var prListCmd = &cobra.Command{ var prStatusCmd = &cobra.Command{ Use: "status", Short: "Show status of relevant pull requests", + Args: cmdutil.NoArgsQuoteReminder, RunE: prStatus, } var prViewCmd = &cobra.Command{ @@ -128,7 +131,7 @@ func prStatus(cmd *cobra.Command, args []string) error { repoOverride, _ := cmd.Flags().GetString("repo") currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) - if err != nil && repoOverride == "" && err.Error() != "git: not on any branch" { + if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) { return fmt.Errorf("could not query for pull request for current branch: %w", err) } diff --git a/command/pr_create.go b/command/pr_create.go index 608656f4b..b2dd48697 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -7,10 +7,12 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/githubtemplate" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -192,8 +194,18 @@ func prCreate(cmd *cobra.Command, _ []string) error { } } + isDraft, err := cmd.Flags().GetBool("draft") + if err != nil { + return fmt.Errorf("could not parse draft: %w", err) + } + if !isWeb && !autofill { - fmt.Fprintf(colorableErr(cmd), "\nCreating pull request for %s into %s in %s\n\n", + message := "\nCreating pull request for %s into %s in %s\n\n" + if isDraft { + message = "\nCreating draft pull request for %s into %s in %s\n\n" + } + + fmt.Fprintf(colorableErr(cmd), message, utils.Cyan(headBranch), utils.Cyan(baseBranch), ghrepo.FullName(baseRepo)) @@ -245,10 +257,6 @@ func prCreate(cmd *cobra.Command, _ []string) error { return errors.New("pull request title must not be blank") } - isDraft, err := cmd.Flags().GetBool("draft") - if err != nil { - return fmt.Errorf("could not parse draft: %w", err) - } if isDraft && isWeb { return errors.New("the --draft flag is not supported with --web") } @@ -446,7 +454,16 @@ func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assi var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", + Args: cmdutil.NoArgsQuoteReminder, RunE: prCreate, + Example: heredoc.Doc(` + $ gh pr create --title "The bug is fixed" --body "Everything works again" + $ gh issue create --label "bug,help wanted" + $ gh issue create --label bug --label "help wanted" + $ gh pr create --reviewer monalisa,hubot + $ gh pr create --project "Roadmap" + $ gh pr create --base develop + `), } func init() { @@ -461,9 +478,9 @@ func init() { prCreateCmd.Flags().BoolP("web", "w", false, "Open the web browser to create a pull request") prCreateCmd.Flags().BoolP("fill", "f", false, "Do not prompt for title/body and just use commit info") - prCreateCmd.Flags().StringSliceP("reviewer", "r", nil, "Request a review from someone by their `login`") - prCreateCmd.Flags().StringSliceP("assignee", "a", nil, "Assign a person by their `login`") - prCreateCmd.Flags().StringSliceP("label", "l", nil, "Add a label by `name`") - prCreateCmd.Flags().StringSliceP("project", "p", nil, "Add the pull request to a project by `name`") + prCreateCmd.Flags().StringSliceP("reviewer", "r", nil, "Request reviews from people by their `login`") + prCreateCmd.Flags().StringSliceP("assignee", "a", nil, "Assign people by their `login`") + prCreateCmd.Flags().StringSliceP("label", "l", nil, "Add labels by `name`") + prCreateCmd.Flags().StringSliceP("project", "p", nil, "Add the pull request to projects by `name`") prCreateCmd.Flags().StringP("milestone", "m", "", "Add the pull request to a milestone by `name`") } diff --git a/command/pr_test.go b/command/pr_test.go index 4c31198b0..e7e1e2dc3 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -299,6 +299,38 @@ Requesting a code review from you } } +func TestPRStatus_detachedHead(t *testing.T) { + initBlankContext("", "OWNER/REPO", "") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": {} } + `)) + + output, err := RunCommand("pr status") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no current branch + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} + func TestPRList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 5b9923fa7..693603e05 100644 --- a/command/root.go +++ b/command/root.go @@ -59,9 +59,7 @@ func init() { // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") RootCmd.SetHelpFunc(rootHelpFunc) - - // This will silence the usage func on error - RootCmd.SetUsageFunc(func(_ *cobra.Command) error { return nil }) + RootCmd.SetUsageFunc(rootUsageFunc) RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 1dcf3c668..53de6cf2b 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -164,7 +164,8 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie } issueState.Body = templateContents } else if legacyTemplatePath != nil { - issueState.Body = string(githubtemplate.ExtractContents(*legacyTemplatePath)) + templateContents = string(githubtemplate.ExtractContents(*legacyTemplatePath)) + issueState.Body = templateContents } else { issueState.Body = defs.Body } diff --git a/context/blank_context.go b/context/blank_context.go index 3ea657abe..80a2cd298 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -40,7 +40,7 @@ func (c *blankContext) SetAuthToken(t string) { func (c *blankContext) Branch() (string, error) { if c.branch == "" { - return "", fmt.Errorf("branch was not initialized") + return "", fmt.Errorf("branch was not initialized: %w", git.ErrNotOnAnyBranch) } return c.branch, nil } diff --git a/docs/triage.md b/docs/triage.md index 95e1bf852..6f37b6a0c 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -2,7 +2,17 @@ As we get more issues and pull requests opened on the GitHub CLI, we've decided on a weekly rotation triage role. The initial expectation is that the person in the role for the week spends no more than -1-2 hours a day on this work; we can refine that as needed. +1-2 hours a day on this work; we can refine that as needed. Below is a basic timeline for a typical +triage day. + +1. Note the time +2. Open every new [issue](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue)/[pr](https://github.com/cli/cli/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse) in a tab +3. Go through each one and look for things that should be closed outright (See the PR and Issue section below for more details.) +4. Go through again and look for issues that are worth keeping around, update each one with labels/pings +5. Go through again and look for PRs that solve a useful problem but lack obvious things like tests or passing builds; request changes on those +6. Mark any remaining PRs (i.e. ones that look worth merging with a cursory glance) as `community` PRs and move to Needs Review +7. Look for [issues](https://github.com/cli/cli/issues?q=is%3Aopen+is%3Aissue) and [PRs](https://github.com/cli/cli/pulls?q=is%3Apr+is%3Aopen+draft%3Afalse+sort%3Aupdated-desc) updated in the last day and see if they need a response. +8. Check the clock at each step and just bail out when an hour passes # Incoming issues @@ -15,16 +25,21 @@ just imagine a flowchart - e.g. have already discussed not wanting to do or duplicate issue - comment acknowledging receipt - close +- do we want to do it, but not in the next year? + - comment acknowledging it, but that we don't plan on working on it this year. + - add `future` label + - add additional labels as needed(examples include `enhancement` or `bug`) + - close - do we want to do it? - e.g. bugs or things we have discussed before - comment acknowledging it - - label appropriately (examples include `enhancement` or `bug`) + - label appropriately - add to project TODO column if appropriate, otherwise just leave it labeled - is it intriguing but needs discussion? - label `needs-design` if design input is needed, ping - label `needs-investigation` if engineering research is required before action can be taken - ping engineers if eng needed - - ping product if it's about future directions/roadamp/big changes + - ping product if it's about future directions/roadmap/big changes - does it need more info from the issue author? - ask the user for that - add `needs-user-input` label @@ -57,6 +72,18 @@ helpful. For each PR, ask: -- is this too stale? close with comment +- is this too stale (more than two months old or too many conflicts)? close with comment - is this really close but author is absent? push commits to finish, request review - is this waiting on triage? go through the PR triage flow + +## Examples + +We want the cli/cli repo to be a safe and encouraging open-source environment. Below are some examples +of how to empathetically respond to or close an issue/PR: + +- [Closing a quality PR its scope is too large](https://github.com/cli/cli/pull/1161) +- [Closing a stale PR](https://github.com/cli/cli/pull/557#issuecomment-639077269) +- [Closing a PR that doesn't follow our CONTRIBUTING policy](https://github.com/cli/cli/pull/864) +- [Responding to a bug report](https://github.com/desktop/desktop/issues/9195#issuecomment-592243129) +- [Closing an issue that out of scope](https://github.com/cli/cli/issues/777#issuecomment-612926229) +- [Closing an issue with a feature request](https://github.com/desktop/desktop/issues/9722#issuecomment-625461766) diff --git a/git/git.go b/git/git.go index 355f905f0..ff40adde3 100644 --- a/git/git.go +++ b/git/git.go @@ -13,6 +13,9 @@ import ( "github.com/cli/cli/internal/run" ) +// ErrNotOnAnyBranch indicates that the users is in detached HEAD state +var ErrNotOnAnyBranch = errors.New("git: not on any branch") + // Ref represents a git commit reference type Ref struct { Hash string @@ -64,7 +67,7 @@ func CurrentBranch() (string, error) { if errors.As(err, &cmdErr) { if cmdErr.Stderr.Len() == 0 { // Detached head - return "", errors.New("git: not on any branch") + return "", ErrNotOnAnyBranch } } diff --git a/git/git_test.go b/git/git_test.go index 8cd97a10d..c03e7153d 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -67,9 +67,8 @@ func Test_CurrentBranch_detached_head(t *testing.T) { if err == nil { t.Errorf("expected an error") } - expectedError := "git: not on any branch" - if err.Error() != expectedError { - t.Errorf("got unexpected error: %s instead of %s", err.Error(), expectedError) + if err != ErrNotOnAnyBranch { + t.Errorf("got unexpected error: %s instead of %s", err, ErrNotOnAnyBranch) } if len(cs.Calls) != 1 { t.Errorf("expected 1 git call, saw %d", len(cs.Calls)) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index a5540cf2d..e1edabbc0 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -3,6 +3,7 @@ package api import ( "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -32,6 +33,7 @@ type ApiOptions struct { RawFields []string RequestHeaders []string ShowResponseHeaders bool + Paginate bool HttpClient func() (*http.Client, error) BaseRepo func() (ghrepo.Interface, error) @@ -74,7 +76,11 @@ on the format of the value: Raw request body may be passed from the outside via a file specified by '--input'. Pass "-" to read from standard input. In this mode, parameters specified via '--field' flags are serialized into URL query parameters. -`, + +In '--paginate' mode, all pages of results will sequentially be requested until +there are no more pages of results. For GraphQL requests, this requires that the +original query accepts an '$endCursor: String' variable and that it fetches the +'pageInfo{ hasNextPage, endCursor }' set of fields from a collection.`, Example: heredoc.Doc(` $ gh api repos/:owner/:repo/releases @@ -87,12 +93,33 @@ Pass "-" to read from standard input. In this mode, parameters specified via } } ' + + $ gh api graphql --paginate -f query=' + query($endCursor: String) { + viewer { + repositories(first: 100, after: $endCursor) { + nodes { nameWithOwner } + pageInfo { + hasNextPage + endCursor + } + } + } + } + ' `), Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.RequestPath = args[0] opts.RequestMethodPassed = c.Flags().Changed("method") + if opts.Paginate && !strings.EqualFold(opts.RequestMethod, "GET") && opts.RequestPath != "graphql" { + return &cmdutil.FlagError{Err: errors.New(`the '--paginate' option is not supported for non-GET requests`)} + } + if opts.Paginate && opts.RequestInputFile != "" { + return &cmdutil.FlagError{Err: errors.New(`the '--paginate' option is not supported with '--input'`)} + } + if runF != nil { return runF(&opts) } @@ -105,6 +132,7 @@ Pass "-" to read from standard input. In this mode, parameters specified via cmd.Flags().StringArrayVarP(&opts.RawFields, "raw-field", "f", nil, "Add a string parameter") cmd.Flags().StringArrayVarP(&opts.RequestHeaders, "header", "H", nil, "Add an additional HTTP request header") cmd.Flags().BoolVarP(&opts.ShowResponseHeaders, "include", "i", false, "Include HTTP response headers in the output") + cmd.Flags().BoolVar(&opts.Paginate, "paginate", false, "Make additional HTTP requests to fetch all pages of results") cmd.Flags().StringVar(&opts.RequestInputFile, "input", "", "The file to use as body for the HTTP request") return cmd } @@ -115,6 +143,7 @@ func apiRun(opts *ApiOptions) error { return err } + isGraphQL := opts.RequestPath == "graphql" requestPath, err := fillPlaceholders(opts.RequestPath, opts) if err != nil { return fmt.Errorf("unable to expand placeholder in path: %w", err) @@ -127,6 +156,10 @@ func apiRun(opts *ApiOptions) error { method = "POST" } + if opts.Paginate && !isGraphQL { + requestPath = addPerPage(requestPath, 100, params) + } + if opts.RequestInputFile != "" { file, size, err := openUserFile(opts.RequestInputFile, opts.IO.In) if err != nil { @@ -145,11 +178,40 @@ func apiRun(opts *ApiOptions) error { return err } - resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) - if err != nil { - return err + hasNextPage := true + for hasNextPage { + resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) + if err != nil { + return err + } + + endCursor, err := processResponse(resp, opts) + if err != nil { + return err + } + + if !opts.Paginate { + break + } + + if isGraphQL { + hasNextPage = endCursor != "" + if hasNextPage { + params["endCursor"] = endCursor + } + } else { + requestPath, hasNextPage = findNextPage(resp) + } + + if hasNextPage && opts.ShowResponseHeaders { + fmt.Fprint(opts.IO.Out, "\n") + } } + return nil +} + +func processResponse(resp *http.Response, opts *ApiOptions) (endCursor string, err error) { if opts.ShowResponseHeaders { fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) @@ -157,7 +219,7 @@ func apiRun(opts *ApiOptions) error { } if resp.StatusCode == 204 { - return nil + return } var responseBody io.Reader = resp.Body defer resp.Body.Close() @@ -168,31 +230,44 @@ func apiRun(opts *ApiOptions) error { if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode) if err != nil { - return err + return } } + var bodyCopy *bytes.Buffer + isGraphQLPaginate := isJSON && resp.StatusCode == 200 && opts.Paginate && opts.RequestPath == "graphql" + if isGraphQLPaginate { + bodyCopy = &bytes.Buffer{} + responseBody = io.TeeReader(responseBody, bodyCopy) + } + if isJSON && opts.IO.ColorEnabled() { err = jsoncolor.Write(opts.IO.Out, responseBody, " ") if err != nil { - return err + return } } else { _, err = io.Copy(opts.IO.Out, responseBody) if err != nil { - return err + return } } if serverError != "" { fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError) - return cmdutil.SilentError + err = cmdutil.SilentError + return } else if resp.StatusCode > 299 { fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode) - return cmdutil.SilentError + err = cmdutil.SilentError + return } - return nil + if isGraphQLPaginate { + endCursor = findEndCursor(bodyCopy) + } + + return } var placeholderRE = regexp.MustCompile(`\:(owner|repo)\b`) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 605e4bf9e..7b89d715c 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -2,6 +2,7 @@ package api import ( "bytes" + "encoding/json" "fmt" "io/ioutil" "net/http" @@ -13,6 +14,7 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func Test_NewCmdApi(t *testing.T) { @@ -36,6 +38,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -51,6 +54,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -66,6 +70,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string{"body=@file.txt"}, RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -81,9 +86,52 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string{"accept: text/plain"}, ShowResponseHeaders: true, + Paginate: false, }, wantsErr: false, }, + { + name: "with pagination", + cli: "repos/OWNER/REPO/issues --paginate", + wants: ApiOptions{ + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "repos/OWNER/REPO/issues", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: true, + }, + wantsErr: false, + }, + { + name: "POST pagination", + cli: "-XPOST repos/OWNER/REPO/issues --paginate", + wantsErr: true, + }, + { + name: "GraphQL pagination", + cli: "-XPOST graphql --paginate", + wants: ApiOptions{ + RequestMethod: "POST", + RequestMethodPassed: true, + RequestPath: "graphql", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: true, + }, + wantsErr: false, + }, + { + name: "input pagination", + cli: "--input repos/OWNER/REPO/issues --paginate", + wantsErr: true, + }, { name: "with request body from file", cli: "user --input myfile", @@ -96,6 +144,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -246,6 +295,136 @@ func Test_apiRun(t *testing.T) { } } +func Test_apiRun_paginationREST(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + requestCount := 0 + responses := []*http.Response{ + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":1}`)), + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":2}`)), + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + { + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"page":3}`)), + Header: http.Header{}, + }, + } + + options := ApiOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + resp := responses[requestCount] + resp.Request = req + requestCount++ + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + + RequestPath: "issues", + Paginate: true, + } + + err := apiRun(&options) + assert.NoError(t, err) + + assert.Equal(t, `{"page":1}{"page":2}{"page":3}`, stdout.String(), "stdout") + assert.Equal(t, "", stderr.String(), "stderr") + + assert.Equal(t, "https://api.github.com/issues?per_page=100", responses[0].Request.URL.String()) + assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=2", responses[1].Request.URL.String()) + assert.Equal(t, "https://api.github.com/repositories/1227/issues?page=3", responses[2].Request.URL.String()) +} + +func Test_apiRun_paginationGraphQL(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + requestCount := 0 + responses := []*http.Response{ + { + StatusCode: 200, + Header: http.Header{"Content-Type": []string{`application/json`}}, + Body: ioutil.NopCloser(bytes.NewBufferString(`{ + "data": { + "nodes": ["page one"], + "pageInfo": { + "endCursor": "PAGE1_END", + "hasNextPage": true + } + } + }`)), + }, + { + StatusCode: 200, + Header: http.Header{"Content-Type": []string{`application/json`}}, + Body: ioutil.NopCloser(bytes.NewBufferString(`{ + "data": { + "nodes": ["page two"], + "pageInfo": { + "endCursor": "PAGE2_END", + "hasNextPage": false + } + } + }`)), + }, + } + + options := ApiOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + resp := responses[requestCount] + resp.Request = req + requestCount++ + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + + RequestMethod: "POST", + RequestPath: "graphql", + Paginate: true, + } + + err := apiRun(&options) + require.NoError(t, err) + + assert.Contains(t, stdout.String(), `"page one"`) + assert.Contains(t, stdout.String(), `"page two"`) + assert.Equal(t, "", stderr.String(), "stderr") + + var requestData struct { + Variables map[string]interface{} + } + + bb, err := ioutil.ReadAll(responses[0].Request.Body) + require.NoError(t, err) + err = json.Unmarshal(bb, &requestData) + require.NoError(t, err) + _, hasCursor := requestData.Variables["endCursor"].(string) + assert.Equal(t, false, hasCursor) + + bb, err = ioutil.ReadAll(responses[1].Request.Body) + require.NoError(t, err) + err = json.Unmarshal(bb, &requestData) + require.NoError(t, err) + endCursor, hasCursor := requestData.Variables["endCursor"].(string) + assert.Equal(t, true, hasCursor) + assert.Equal(t, "PAGE1_END", endCursor) +} + func Test_apiRun_inputFile(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/api/pagination.go b/pkg/cmd/api/pagination.go new file mode 100644 index 000000000..fce88fb92 --- /dev/null +++ b/pkg/cmd/api/pagination.go @@ -0,0 +1,108 @@ +package api + +import ( + "encoding/json" + "fmt" + "io" + "net/http" + "net/url" + "regexp" + "strings" +) + +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + +func findNextPage(resp *http.Response) (string, bool) { + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) >= 2 && m[2] == "next" { + return m[1], true + } + } + return "", false +} + +func findEndCursor(r io.Reader) string { + dec := json.NewDecoder(r) + + var idx int + var stack []json.Delim + var lastKey string + var contextKey string + + var endCursor string + var hasNextPage bool + var foundEndCursor bool + var foundNextPage bool + +loop: + for { + t, err := dec.Token() + if err == io.EOF { + break + } + if err != nil { + return "" + } + + switch tt := t.(type) { + case json.Delim: + switch tt { + case '{', '[': + stack = append(stack, tt) + contextKey = lastKey + idx = 0 + case '}', ']': + stack = stack[:len(stack)-1] + contextKey = "" + idx = 0 + } + default: + isKey := len(stack) > 0 && stack[len(stack)-1] == '{' && idx%2 == 0 + idx++ + + switch tt := t.(type) { + case string: + if isKey { + lastKey = tt + } else if contextKey == "pageInfo" && lastKey == "endCursor" { + endCursor = tt + foundEndCursor = true + if foundNextPage { + break loop + } + } + case bool: + if contextKey == "pageInfo" && lastKey == "hasNextPage" { + hasNextPage = tt + foundNextPage = true + if foundEndCursor { + break loop + } + } + } + } + } + + if hasNextPage { + return endCursor + } + return "" +} + +func addPerPage(p string, perPage int, params map[string]interface{}) string { + if _, hasPerPage := params["per_page"]; hasPerPage { + return p + } + + idx := strings.IndexRune(p, '?') + sep := "?" + + if idx >= 0 { + if qp, err := url.ParseQuery(p[idx+1:]); err == nil && qp.Get("per_page") != "" { + return p + } + sep = "&" + } + + return fmt.Sprintf("%s%sper_page=%d", p, sep, perPage) +} diff --git a/pkg/cmd/api/pagination_test.go b/pkg/cmd/api/pagination_test.go new file mode 100644 index 000000000..3bb1a8ec5 --- /dev/null +++ b/pkg/cmd/api/pagination_test.go @@ -0,0 +1,169 @@ +package api + +import ( + "bytes" + "io" + "net/http" + "testing" +) + +func Test_findNextPage(t *testing.T) { + tests := []struct { + name string + resp *http.Response + want string + want1 bool + }{ + { + name: "no Link header", + resp: &http.Response{}, + want: "", + want1: false, + }, + { + name: "no next page in Link", + resp: &http.Response{ + Header: http.Header{ + "Link": []string{`; rel="last"`}, + }, + }, + want: "", + want1: false, + }, + { + name: "has next page", + resp: &http.Response{ + Header: http.Header{ + "Link": []string{`; rel="next", ; rel="last"`}, + }, + }, + want: "https://api.github.com/issues?page=2", + want1: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, got1 := findNextPage(tt.resp) + if got != tt.want { + t.Errorf("findNextPage() got = %v, want %v", got, tt.want) + } + if got1 != tt.want1 { + t.Errorf("findNextPage() got1 = %v, want %v", got1, tt.want1) + } + }) + } +} + +func Test_findEndCursor(t *testing.T) { + tests := []struct { + name string + json io.Reader + want string + }{ + { + name: "blank", + json: bytes.NewBufferString(`{}`), + want: "", + }, + { + name: "unrelated fields", + json: bytes.NewBufferString(`{ + "hasNextPage": true, + "endCursor": "THE_END" + }`), + want: "", + }, + { + name: "has next page", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": true, + "endCursor": "THE_END" + } + }`), + want: "THE_END", + }, + { + name: "more pageInfo blocks", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": true, + "endCursor": "THE_END" + }, + "pageInfo": { + "hasNextPage": true, + "endCursor": "NOT_THIS" + } + }`), + want: "THE_END", + }, + { + name: "no next page", + json: bytes.NewBufferString(`{ + "pageInfo": { + "hasNextPage": false, + "endCursor": "THE_END" + } + }`), + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := findEndCursor(tt.json); got != tt.want { + t.Errorf("findEndCursor() = %v, want %v", got, tt.want) + } + }) + } +} + +func Test_addPerPage(t *testing.T) { + type args struct { + p string + perPage int + params map[string]interface{} + } + tests := []struct { + name string + args args + want string + }{ + { + name: "adds per_page", + args: args{ + p: "items", + perPage: 13, + params: nil, + }, + want: "items?per_page=13", + }, + { + name: "avoids adding per_page if already in params", + args: args{ + p: "items", + perPage: 13, + params: map[string]interface{}{ + "state": "open", + "per_page": 99, + }, + }, + want: "items", + }, + { + name: "avoids adding per_page if already in query", + args: args{ + p: "items?per_page=6&state=open", + perPage: 13, + params: nil, + }, + want: "items?per_page=6&state=open", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := addPerPage(tt.args.p, tt.args.perPage, tt.args.params); got != tt.want { + t.Errorf("addPerPage() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/cmdutil/args.go b/pkg/cmdutil/args.go new file mode 100644 index 000000000..c203fb691 --- /dev/null +++ b/pkg/cmdutil/args.go @@ -0,0 +1,33 @@ +package cmdutil + +import ( + "errors" + "fmt" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func NoArgsQuoteReminder(cmd *cobra.Command, args []string) error { + if len(args) < 1 { + return nil + } + + errMsg := fmt.Sprintf("unknown argument %q", args[0]) + if len(args) > 1 { + errMsg = fmt.Sprintf("unknown arguments %q", args) + } + + hasValueFlag := false + cmd.Flags().Visit(func(f *pflag.Flag) { + if f.Value.Type() != "bool" { + hasValueFlag = true + } + }) + + if hasValueFlag { + errMsg += "; please quote all values that have spaces" + } + + return &FlagError{Err: errors.New(errMsg)} +} diff --git a/utils/color.go b/utils/color.go index 4940fe26b..dd9a7d11c 100644 --- a/utils/color.go +++ b/utils/color.go @@ -10,8 +10,7 @@ import ( ) var ( - _isColorEnabled bool = true - _isStdoutTerminal, checkedTerminal, checkedNoColor bool + _isStdoutTerminal, checkedTerminal bool // Outputs ANSI color if stdout is a tty Magenta = makeColorFunc("magenta") @@ -45,7 +44,7 @@ func NewColorable(f *os.File) io.Writer { func makeColorFunc(color string) func(string) string { cf := ansi.ColorFunc(color) return func(arg string) string { - if isColorEnabled() && isStdoutTerminal() { + if isColorEnabled() { return cf(arg) } return arg @@ -53,9 +52,9 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if !checkedNoColor { - _isColorEnabled = os.Getenv("NO_COLOR") == "" - checkedNoColor = true + if os.Getenv("NO_COLOR") != "" { + return false } - return _isColorEnabled + + return isStdoutTerminal() }