From 5b9e64b4854535f6b2f307658bf28deb0a36f07d Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Tue, 26 May 2020 08:00:51 +0530 Subject: [PATCH 01/39] Fix incorrect docs in Issue&PR create --- command/issue.go | 8 ++++---- command/pr_create.go | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/command/issue.go b/command/issue.go index 3d3f01708..48fcbc453 100644 --- a/command/issue.go +++ b/command/issue.go @@ -29,14 +29,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") diff --git a/command/pr_create.go b/command/pr_create.go index 96dc08791..be8ca6079 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -423,9 +423,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`") } From a77556237985871b2effa6bae23cc7b25b32eb6c Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Tue, 9 Jun 2020 23:42:24 +0530 Subject: [PATCH 02/39] Added Examples for Issue and PR Create --- command/issue.go | 11 +++++++++++ command/pr_create.go | 15 +++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/command/issue.go b/command/issue.go index 48fcbc453..4a5c98635 100644 --- a/command/issue.go +++ b/command/issue.go @@ -61,6 +61,17 @@ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", RunE: issueCreate, + Long: `Create a new issue in a repository + +Examples: + $ gh issue create + $ gh issue create -t "Issue With Title And Body" -b "This is the issue body" + $ gh issue create -t OpenAWebBrowserToCreateThisIssue --web + $ gh issue create -t IssueWithLabels -b IssueBody -l label1,label2 + $ gh issue create -t IssueWithAssignees -b IssueBody -a user1Login,user2Login + $ gh issue create -t IssueWithProjects -b IssueBody -p cli/1,cli/2 + $ gh issue create -t IssueWithMilestone -b IssueBody -m someMilestone +`, } var issueListCmd = &cobra.Command{ Use: "list", diff --git a/command/pr_create.go b/command/pr_create.go index be8ca6079..ee6d2a20e 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -409,6 +409,21 @@ var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", RunE: prCreate, + Long: `Create a pull request in a repository + +Examples: + $ gh pr create + $ gh pr create --fill + $ gh pr create -t "PR With Title And Body" -b "This is the pull request body" + $ gh pr create -t OpenAWebBrowserToCreateThisPR --web + $ gh pr create -t DraftPR --draft + $ gh pr create -t PRWithBaseBranch -b PRBody -B branchName + $ gh pr create -t PRWithReviewers -b PRBody -r user1Login,user2Login + $ gh pr create -t PRWithLabels -b PRBody -l label1,label2 + $ gh pr create -t PRWithProjects -b PRBody -p cli/1,cli/2 + $ gh pr create -t PRWithAssignees -b PRBody -a user1Login,user2Login + $ gh pr create -t PRWithMilestone -b PRBody -m someMilestone +`, } func init() { From 4749e91c53cd5d3321bd9b31be9422d5c700543c Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Wed, 10 Jun 2020 09:55:19 +0530 Subject: [PATCH 03/39] Use cobra Example field for issue & PR create examples --- command/issue.go | 17 +++++++---------- command/pr_create.go | 25 +++++++++++-------------- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/command/issue.go b/command/issue.go index 4a5c98635..3b7b9e9c9 100644 --- a/command/issue.go +++ b/command/issue.go @@ -61,16 +61,13 @@ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", RunE: issueCreate, - Long: `Create a new issue in a repository - -Examples: - $ gh issue create - $ gh issue create -t "Issue With Title And Body" -b "This is the issue body" - $ gh issue create -t OpenAWebBrowserToCreateThisIssue --web - $ gh issue create -t IssueWithLabels -b IssueBody -l label1,label2 - $ gh issue create -t IssueWithAssignees -b IssueBody -a user1Login,user2Login - $ gh issue create -t IssueWithProjects -b IssueBody -p cli/1,cli/2 - $ gh issue create -t IssueWithMilestone -b IssueBody -m someMilestone + Example: `$ gh issue create +$ gh issue create -t "Issue With Title And Body" -b "This is the issue body" +$ gh issue create -t OpenAWebBrowserToCreateThisIssue --web +$ gh issue create -t IssueWithLabels -b IssueBody -l label1,label2 +$ gh issue create -t IssueWithAssignees -b IssueBody -a user1Login,user2Login +$ gh issue create -t IssueWithProjects -b IssueBody -p cli/1,cli/2 +$ gh issue create -t IssueWithMilestone -b IssueBody -m someMilestone `, } var issueListCmd = &cobra.Command{ diff --git a/command/pr_create.go b/command/pr_create.go index ee6d2a20e..e0fc3ca4e 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -409,20 +409,17 @@ var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", RunE: prCreate, - Long: `Create a pull request in a repository - -Examples: - $ gh pr create - $ gh pr create --fill - $ gh pr create -t "PR With Title And Body" -b "This is the pull request body" - $ gh pr create -t OpenAWebBrowserToCreateThisPR --web - $ gh pr create -t DraftPR --draft - $ gh pr create -t PRWithBaseBranch -b PRBody -B branchName - $ gh pr create -t PRWithReviewers -b PRBody -r user1Login,user2Login - $ gh pr create -t PRWithLabels -b PRBody -l label1,label2 - $ gh pr create -t PRWithProjects -b PRBody -p cli/1,cli/2 - $ gh pr create -t PRWithAssignees -b PRBody -a user1Login,user2Login - $ gh pr create -t PRWithMilestone -b PRBody -m someMilestone + Example: `$ gh pr create +$ gh pr create --fill +$ gh pr create -t "PR With Title And Body" -b "This is the pull request body" +$ gh pr create -t OpenAWebBrowserToCreateThisPR --web +$ gh pr create -t DraftPR --draft +$ gh pr create -t PRWithBaseBranch -b PRBody -B branchName +$ gh pr create -t PRWithReviewers -b PRBody -r user1Login,user2Login +$ gh pr create -t PRWithLabels -b PRBody -l label1,label2 +$ gh pr create -t PRWithProjects -b PRBody -p cli/1,cli/2 +$ gh pr create -t PRWithAssignees -b PRBody -a user1Login,user2Login +$ gh pr create -t PRWithMilestone -b PRBody -m someMilestone `, } From 2ebc5ce514780abcb8129b330a5357329f8819b0 Mon Sep 17 00:00:00 2001 From: ShubhankarKG Date: Wed, 10 Jun 2020 18:40:12 +0530 Subject: [PATCH 04/39] Added examples and custom error message for space separated issues --- command/issue.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/command/issue.go b/command/issue.go index 305b6f832..371440066 100644 --- a/command/issue.go +++ b/command/issue.go @@ -65,7 +65,17 @@ var issueCreateCmd = &cobra.Command{ var issueListCmd = &cobra.Command{ Use: "list", Short: "List and filter issues in this repository", - RunE: issueList, + Args: func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + return fmt.Errorf("unknown command %q for %q. Please include whitespace separated issue names within quotes if necessary", args[0], cmd.CommandPath()) + } + return nil + }, + Example: ` + gh issue list + gh issue list -l "help wanted" + `, + RunE: issueList, } var issueStatusCmd = &cobra.Command{ Use: "status", From 3bb6983b3503686250792f85a18651a806b8d478 Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Wed, 10 Jun 2020 11:41:44 -0700 Subject: [PATCH 05/39] fix regression in support for detached HEAD state for gh pr status --- command/pr.go | 2 +- context/context.go | 6 +++++- git/git.go | 5 ++++- git/git_test.go | 5 ++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/command/pr.go b/command/pr.go index e92fa7cf0..f1c1eb101 100644 --- a/command/pr.go +++ b/command/pr.go @@ -116,7 +116,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 == "" && err != git.ErrNotOnAnyBranch { return fmt.Errorf("could not query for pull request for current branch: %w", err) } diff --git a/context/context.go b/context/context.go index 236f9e722..4f1aa10ba 100644 --- a/context/context.go +++ b/context/context.go @@ -207,7 +207,11 @@ func (c *fsContext) Branch() (string, error) { } currentBranch, err := git.CurrentBranch() - if err != nil { + switch err { + case nil: + case git.ErrNotOnAnyBranch: + return "", err + default: return "", fmt.Errorf("could not determine current branch: %w", err) } 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 c84023e40..1a8f32677 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)) From fb7b01b40c3353015ec9b706d68b8626b356ec6d Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Thu, 11 Jun 2020 17:05:25 -0700 Subject: [PATCH 06/39] resolve PR review --- command/pr.go | 2 +- command/pr_test.go | 32 ++++++++++++++++++++++++++++++++ context/blank_context.go | 3 ++- context/context.go | 6 +----- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/command/pr.go b/command/pr.go index f1c1eb101..10a41246d 100644 --- a/command/pr.go +++ b/command/pr.go @@ -116,7 +116,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 != git.ErrNotOnAnyBranch { + 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_test.go b/command/pr_test.go index a11c60820..ee90a66b9 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/context/blank_context.go b/context/blank_context.go index 3ea657abe..7bb5282ec 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -18,6 +18,7 @@ func NewBlank() *blankContext { type blankContext struct { authToken string branch string + branchErr error baseRepo ghrepo.Interface remotes Remotes } @@ -40,7 +41,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/context/context.go b/context/context.go index 4f1aa10ba..236f9e722 100644 --- a/context/context.go +++ b/context/context.go @@ -207,11 +207,7 @@ func (c *fsContext) Branch() (string, error) { } currentBranch, err := git.CurrentBranch() - switch err { - case nil: - case git.ErrNotOnAnyBranch: - return "", err - default: + if err != nil { return "", fmt.Errorf("could not determine current branch: %w", err) } From fffa39368356e870f70ff051518c1814b9df7f6a Mon Sep 17 00:00:00 2001 From: naman <1977419+metalogical@users.noreply.github.com> Date: Thu, 11 Jun 2020 17:08:18 -0700 Subject: [PATCH 07/39] fix lint --- context/blank_context.go | 1 - 1 file changed, 1 deletion(-) diff --git a/context/blank_context.go b/context/blank_context.go index 7bb5282ec..80a2cd298 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -18,7 +18,6 @@ func NewBlank() *blankContext { type blankContext struct { authToken string branch string - branchErr error baseRepo ghrepo.Interface remotes Remotes } From 15ec2b748e2acfddf8a0ef8c5ff3c6da2b74b352 Mon Sep 17 00:00:00 2001 From: ShubhankarKG Date: Fri, 12 Jun 2020 12:00:01 +0530 Subject: [PATCH 08/39] Refactor code --- command/issue.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/command/issue.go b/command/issue.go index 371440066..cf5087bff 100644 --- a/command/issue.go +++ b/command/issue.go @@ -67,13 +67,13 @@ var issueListCmd = &cobra.Command{ Short: "List and filter issues in this repository", Args: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { - return fmt.Errorf("unknown command %q for %q. Please include whitespace separated issue names within quotes if necessary", args[0], cmd.CommandPath()) + return fmt.Errorf("unknown argument %q for %q: Please quote all flag values that contain spaces.", args[0], cmd.CommandPath()) } return nil }, Example: ` - gh issue list - gh issue list -l "help wanted" + $ gh issue list -l "help wanted" + $ gh issue list -A "some author" `, RunE: issueList, } From e6e8f701a2b3832806649d56490115c447ded754 Mon Sep 17 00:00:00 2001 From: gabgodBB Date: Fri, 12 Jun 2020 14:26:38 -0300 Subject: [PATCH 09/39] Issue #930 - Removing color for output files --- utils/color.go | 4 ++++ utils/utils.go | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/utils/color.go b/utils/color.go index 4940fe26b..e88549667 100644 --- a/utils/color.go +++ b/utils/color.go @@ -53,6 +53,10 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { + if !isStdoutTerminal() { + return false + } + if !checkedNoColor { _isColorEnabled = os.Getenv("NO_COLOR") == "" checkedNoColor = true diff --git a/utils/utils.go b/utils/utils.go index c84860d9e..9441bc2bc 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -23,6 +23,10 @@ func OpenInBrowser(url string) error { func RenderMarkdown(text string) (string, error) { style := "notty" + if !isColorEnabled() { + return text, nil + } + if isColorEnabled() { style = "dark" } From 1105902935c3686e566ba78f0b8fe252851e61fc Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 12 Jun 2020 14:40:03 -0700 Subject: [PATCH 10/39] Add more notes to triage doc --- docs/triage.md | 32 +++++++++++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/triage.md b/docs/triage.md index 95e1bf852..cb02d57f4 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%3Afals) 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 (ie 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 passe # Incoming issues @@ -15,10 +25,14 @@ 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. + - label appropriately (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 @@ -57,6 +71,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 CONTRIUBUTING 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) From 4388ff1a9121ed8b56831ae028a1beff19ebf351 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Fri, 12 Jun 2020 14:42:32 -0700 Subject: [PATCH 11/39] spelling mistakes --- docs/triage.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/triage.md b/docs/triage.md index cb02d57f4..320cafd58 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -38,7 +38,7 @@ just imagine a flowchart - 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 @@ -82,7 +82,7 @@ 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 CONTRIUBUTING policy](https://github.com/cli/cli/pull/864) +[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) From 84f3d46338aa32597336d55e0ebcb13e85ea643f Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Sun, 14 Jun 2020 21:53:56 +0530 Subject: [PATCH 12/39] Better example section in issue and pr create --- command/issue.go | 13 ++++++------- command/pr_create.go | 18 +++++++----------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/command/issue.go b/command/issue.go index 3b7b9e9c9..3aa10c89a 100644 --- a/command/issue.go +++ b/command/issue.go @@ -61,13 +61,12 @@ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", RunE: issueCreate, - Example: `$ gh issue create -$ gh issue create -t "Issue With Title And Body" -b "This is the issue body" -$ gh issue create -t OpenAWebBrowserToCreateThisIssue --web -$ gh issue create -t IssueWithLabels -b IssueBody -l label1,label2 -$ gh issue create -t IssueWithAssignees -b IssueBody -a user1Login,user2Login -$ gh issue create -t IssueWithProjects -b IssueBody -p cli/1,cli/2 -$ gh issue create -t IssueWithMilestone -b IssueBody -m someMilestone + Example: ` + $ gh issue create --title "I found a bug" --body "Nothing works" + $ gh issue create --label label1,label2 + $ gh issue create --label label1 --label label2 + $ gh issue create --assignee user1Login,user2Login + $ gh issue create --project "Our Awesome Project" `, } var issueListCmd = &cobra.Command{ diff --git a/command/pr_create.go b/command/pr_create.go index e0fc3ca4e..0d0089b57 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -409,17 +409,13 @@ var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", RunE: prCreate, - Example: `$ gh pr create -$ gh pr create --fill -$ gh pr create -t "PR With Title And Body" -b "This is the pull request body" -$ gh pr create -t OpenAWebBrowserToCreateThisPR --web -$ gh pr create -t DraftPR --draft -$ gh pr create -t PRWithBaseBranch -b PRBody -B branchName -$ gh pr create -t PRWithReviewers -b PRBody -r user1Login,user2Login -$ gh pr create -t PRWithLabels -b PRBody -l label1,label2 -$ gh pr create -t PRWithProjects -b PRBody -p cli/1,cli/2 -$ gh pr create -t PRWithAssignees -b PRBody -a user1Login,user2Login -$ gh pr create -t PRWithMilestone -b PRBody -m someMilestone + Example: ` + $ gh pr create --title "The bug is fixed" --body "Everything works again" + $ gh pr create --label label1,label2 + $ gh pr create --label label1 --label label2 + $ gh pr create --reviewer user1Login,user2Login + $ gh pr create --project "Our Awesome Project" + $ gh pr create --assignee user1Login,user2Login `, } From 5851ee26ddb97ca8839217e869dc97f28e5491d9 Mon Sep 17 00:00:00 2001 From: ObliviousParadigm Date: Mon, 15 Jun 2020 01:03:27 +0530 Subject: [PATCH 13/39] DOC: Changed few sentences Moved code blocks to new lines to maintain consistency --- README.md | 46 +++++++++++++++++++++++++++++++++------------- 1 file changed, 33 insertions(+), 13 deletions(-) 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 From 21a96baf933498bb48a90e8e6cdebd5dea53ea99 Mon Sep 17 00:00:00 2001 From: ShubhankarKG Date: Mon, 15 Jun 2020 17:45:04 +0530 Subject: [PATCH 14/39] Added description to nfpms --- .goreleaser.yml | 1 + 1 file changed, 1 insertion(+) 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 From c70756d596a08feb7a469b6876d4ad1e416cdabf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edd=C3=BA=20Mel=C3=A9ndez=20Gonzales?= Date: Mon, 15 Jun 2020 08:27:30 -0500 Subject: [PATCH 15/39] Improve message when draft pr is created (#1202) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1199 Co-authored-by: Mislav Marohnić --- command/pr_create.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/command/pr_create.go b/command/pr_create.go index 608656f4b..5eafdbcbe 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -192,8 +192,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 +255,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") } From 1178f20f3d54b6c4423c8fac5b124f34da6bf4ba Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 08:27:44 -0700 Subject: [PATCH 16/39] Update docs/triage.md Thanks @aliabbasMerchant Co-authored-by: Aliabbas Merchant --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 320cafd58..1b4308592 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -12,7 +12,7 @@ triage day. 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 (ie 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 passe +8. Check the clock at each step and just bail out when an hour passes # Incoming issues From ee19d10b9531881a4d4ad94b1bc11149ba1c9036 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 08:29:23 -0700 Subject: [PATCH 17/39] Update update line --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 320cafd58..15f2dcb48 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -11,7 +11,7 @@ triage day. 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 (ie 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. +7. Look for updated [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) and see if they need a response. 8. Check the clock at each step and just bail out when an hour passe # Incoming issues From dab44547ee3f7cf0a267b862a9bfee9dfb7d9863 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 08:38:58 -0700 Subject: [PATCH 18/39] Make a list --- docs/triage.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/triage.md b/docs/triage.md index 15f2dcb48..07cd9d3b7 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -80,9 +80,9 @@ For each PR, ask: 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) +- [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) From d60e7586ede60289e9057ec82001460a0678e57d Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 09:30:03 -0700 Subject: [PATCH 19/39] Header two! --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 07cd9d3b7..492fc41f8 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -75,7 +75,7 @@ For each PR, ask: - 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 +## 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: From b27f371c5dc8663ba518393248b6e359fe5be1a7 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 09:30:09 -0700 Subject: [PATCH 20/39] Fixup --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 492fc41f8..ee122cc94 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -10,7 +10,7 @@ triage day. 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 (ie ones that look worth merging with a cursory glance) as community PRs and move to Needs Review +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 updated [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) and see if they need a response. 8. Check the clock at each step and just bail out when an hour passe From 1fbb2ba49190fafe652288a3349292cc794e9484 Mon Sep 17 00:00:00 2001 From: Corey Johnson Date: Mon, 15 Jun 2020 09:30:28 -0700 Subject: [PATCH 21/39] false --- docs/triage.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index ee122cc94..8612cb8fd 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -6,7 +6,7 @@ triage role. The initial expectation is that the person in the role for the week 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%3Afals) in a tab +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 From b838ac4014adeeaf04658f811dca4297a00595e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 15 Jun 2020 21:01:32 +0200 Subject: [PATCH 22/39] Improve no args error handler and extend it to other commands --- command/issue.go | 15 ++++++--------- command/pr.go | 2 ++ command/pr_create.go | 2 ++ pkg/cmdutil/args.go | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 pkg/cmdutil/args.go diff --git a/command/issue.go b/command/issue.go index a66235ddd..bcab6538e 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" @@ -67,21 +68,17 @@ var issueCmd = &cobra.Command{ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", + Args: cmdutil.NoArgsQuoteReminder, RunE: issueCreate, } var issueListCmd = &cobra.Command{ Use: "list", Short: "List and filter issues in this repository", - Args: func(cmd *cobra.Command, args []string) error { - if len(args) > 0 { - return fmt.Errorf("unknown argument %q for %q: Please quote all flag values that contain spaces.", args[0], cmd.CommandPath()) - } - return nil - }, - Example: ` + Example: heredoc.Doc(` $ gh issue list -l "help wanted" - $ gh issue list -A "some author" - `, + $ gh issue list -A monalisa + `), + Args: cmdutil.NoArgsQuoteReminder, RunE: issueList, } var issueStatusCmd = &cobra.Command{ diff --git a/command/pr.go b/command/pr.go index c5fc1ae7b..abd0f2ffc 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 diff --git a/command/pr_create.go b/command/pr_create.go index 5eafdbcbe..b96c45391 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -11,6 +11,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/githubtemplate" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -452,6 +453,7 @@ 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, } 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)} +} From 9c75cff94b6ba499e69648c098251bee59c80c6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 13:58:01 +0200 Subject: [PATCH 23/39] Show command usage output on invalid flags being passed --- command/help.go | 10 ++++++++++ command/root.go | 4 +--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/command/help.go b/command/help.go index b776d23f1..87e302839 100644 --- a/command/help.go +++ b/command/help.go @@ -9,6 +9,16 @@ import ( "github.com/spf13/cobra" ) +func rootUsageFunc(command *cobra.Command) error { + command.Printf("Usage: %s", command.UseLine()) + + flagUsages := command.LocalFlags().FlagUsages() + if flagUsages != "" { + command.Printf("\n\nFlags:\n%s", flagUsages) + } + return nil +} + 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 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 { From ce59deb7b3a77a5b29ac4d768f9f8ffb530e4882 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 13:58:31 +0200 Subject: [PATCH 24/39] Show inherited flags in help output --- command/help.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/command/help.go b/command/help.go index 87e302839..cf9b64b7a 100644 --- a/command/help.go +++ b/command/help.go @@ -95,6 +95,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"]}) } From 311536433cbcf206e86d8f787496a60a66d87c07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 15:41:46 +0200 Subject: [PATCH 25/39] The ultimate `--help` & spelling suggester handler - short command usage output now lists subcommands instead of flags for parent commands - mistyping a subcommand now results in a non-zero exit status - requesting `--help` or `-h` for any command now prints help docs on stdout and exits with 0 --- cmd/gh/main.go | 3 +++ command/help.go | 64 ++++++++++++++++++++++++++++++++----------------- 2 files changed, 45 insertions(+), 22 deletions(-) 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/help.go b/command/help.go index cf9b64b7a..2fb7b6598 100644 --- a/command/help.go +++ b/command/help.go @@ -12,6 +12,18 @@ import ( 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 + } + command.Printf(" %s\n", c.Name()) + } + return nil + } + flagUsages := command.LocalFlags().FlagUsages() if flagUsages != "" { command.Printf("\n\nFlags:\n%s", flagUsages) @@ -19,32 +31,40 @@ func rootUsageFunc(command *cobra.Command) error { return nil } -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 - } - candidates := command.SuggestionsFor(args[1]) +var hasFailed bool - errOut := command.OutOrStderr() - fmt.Fprintf(errOut, "unknown command %q for %q\n", args[1], "gh "+args[0]) +// HasFailed signals that the main process should exit with non-zero status +func HasFailed() bool { + return hasFailed +} - 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") - } +// 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 `%s`\n", arg, command.UseLine()) - oldOut := command.OutOrStdout() - command.SetOut(errOut) - defer command.SetOut(oldOut) + 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{} From f64e5f16ebe860fd7d7fa539dd5378ea97eb7a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 16:06:51 +0200 Subject: [PATCH 26/39] Add special case for unsupported `gh help` --- command/help.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/command/help.go b/command/help.go index 2fb7b6598..232647d62 100644 --- a/command/help.go +++ b/command/help.go @@ -42,21 +42,26 @@ func HasFailed() bool { // 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 `%s`\n", arg, command.UseLine()) + command.Printf("unknown command %q for %q\n", arg, command.CommandPath()) - if command.SuggestionsMinimumDistance <= 0 { - command.SuggestionsMinimumDistance = 2 + var candidates []string + if arg == "help" { + candidates = []string{"--help"} + } else { + if command.SuggestionsMinimumDistance <= 0 { + command.SuggestionsMinimumDistance = 2 + } + candidates = command.SuggestionsFor(arg) } - 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") } + command.Print("\n") _ = rootUsageFunc(command) } From 3fea249d70cb4f581a34034f9bc71c150b88b895 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 16:38:19 +0200 Subject: [PATCH 27/39] :nail_care: tweak `issue/pr create` examples --- command/issue.go | 12 ++++++------ command/pr_create.go | 15 ++++++++------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/command/issue.go b/command/issue.go index 3aa10c89a..ab12a29ed 100644 --- a/command/issue.go +++ b/command/issue.go @@ -61,13 +61,13 @@ var issueCreateCmd = &cobra.Command{ Use: "create", Short: "Create a new issue", RunE: issueCreate, - Example: ` + Example: heredoc.Doc(` $ gh issue create --title "I found a bug" --body "Nothing works" - $ gh issue create --label label1,label2 - $ gh issue create --label label1 --label label2 - $ gh issue create --assignee user1Login,user2Login - $ gh issue create --project "Our Awesome Project" -`, + $ 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", diff --git a/command/pr_create.go b/command/pr_create.go index 0d0089b57..0c6a5ff26 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" @@ -409,14 +410,14 @@ var prCreateCmd = &cobra.Command{ Use: "create", Short: "Create a pull request", RunE: prCreate, - Example: ` + Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" - $ gh pr create --label label1,label2 - $ gh pr create --label label1 --label label2 - $ gh pr create --reviewer user1Login,user2Login - $ gh pr create --project "Our Awesome Project" - $ gh pr create --assignee user1Login,user2Login -`, + $ 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() { From 27cce8e3ca8df74890757a17394935856c4e2049 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 16:50:34 +0200 Subject: [PATCH 28/39] Apply the no-args handler to `issue/pr status` commands --- command/issue.go | 1 + command/pr.go | 1 + 2 files changed, 2 insertions(+) diff --git a/command/issue.go b/command/issue.go index bcab6538e..372ad8bd1 100644 --- a/command/issue.go +++ b/command/issue.go @@ -84,6 +84,7 @@ var issueListCmd = &cobra.Command{ 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 abd0f2ffc..18150d4eb 100644 --- a/command/pr.go +++ b/command/pr.go @@ -77,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{ From 7907def88081f519bf3266ca975e80e5d519e434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 18:16:49 +0200 Subject: [PATCH 29/39] api command: add support for REST pagination --- pkg/cmd/api/api.go | 52 ++++++++++++++++++++-- pkg/cmd/api/api_test.go | 98 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index a5540cf2d..dff60245c 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) @@ -93,6 +95,13 @@ Pass "-" to read from standard input. In this mode, parameters specified via 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 +114,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 } @@ -145,11 +155,46 @@ 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 + } + + err = processResponse(resp, opts) + if err != nil { + return err + } + + if !opts.Paginate { + break + } + requestPath, hasNextPage = findNextPage(resp) + + if hasNextPage && opts.ShowResponseHeaders { + fmt.Fprint(opts.IO.Out, "\n") + } } + return nil +} + +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 { + continue + } + if m[2] == "next" { + return m[1], true + } + } + return "", false +} + +func processResponse(resp *http.Response, opts *ApiOptions) error { if opts.ShowResponseHeaders { fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) @@ -164,6 +209,7 @@ func apiRun(opts *ApiOptions) error { isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) + var err error var serverError string if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 605e4bf9e..c83cf3b5a 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -36,6 +36,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -51,6 +52,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -66,6 +68,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string{"body=@file.txt"}, RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -81,9 +84,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 +142,7 @@ func Test_NewCmdApi(t *testing.T) { MagicFields: []string(nil), RequestHeaders: []string(nil), ShowResponseHeaders: false, + Paginate: false, }, wantsErr: false, }, @@ -246,6 +293,57 @@ func Test_apiRun(t *testing.T) { } } +func Test_apiRun_pagination(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 + }, + + 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/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_inputFile(t *testing.T) { tests := []struct { name string From 3f940c98f1872d439109b4363139fe59b6097ee7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 16 Jun 2020 18:19:39 +0200 Subject: [PATCH 30/39] Add assertion for 1st api request before pagination --- pkg/cmd/api/api_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index c83cf3b5a..9debf7896 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -331,7 +331,8 @@ func Test_apiRun_pagination(t *testing.T) { return &http.Client{Transport: tr}, nil }, - Paginate: true, + RequestPath: "issues", + Paginate: true, } err := apiRun(&options) @@ -340,6 +341,7 @@ func Test_apiRun_pagination(t *testing.T) { 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", 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()) } From f4ecd365a69d3491e82d52eb16b10aee66d75166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 17 Jun 2020 18:02:20 +0200 Subject: [PATCH 31/39] api command: add GraphQL support for `--paginate` --- pkg/cmd/api/api.go | 76 +++++++++++++-------- pkg/cmd/api/api_test.go | 81 +++++++++++++++++++++- pkg/cmd/api/pagination.go | 87 ++++++++++++++++++++++++ pkg/cmd/api/pagination_test.go | 118 +++++++++++++++++++++++++++++++++ 4 files changed, 335 insertions(+), 27 deletions(-) create mode 100644 pkg/cmd/api/pagination.go create mode 100644 pkg/cmd/api/pagination_test.go diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index dff60245c..067ef6bc8 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -76,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 @@ -89,6 +93,20 @@ 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 { @@ -162,7 +180,7 @@ func apiRun(opts *ApiOptions) error { return err } - err = processResponse(resp, opts) + endCursor, err := processResponse(resp, opts) if err != nil { return err } @@ -170,7 +188,15 @@ func apiRun(opts *ApiOptions) error { if !opts.Paginate { break } - requestPath, hasNextPage = findNextPage(resp) + + if opts.RequestPath == "graphql" { + hasNextPage = endCursor != "" + if hasNextPage { + params["endCursor"] = endCursor + } + } else { + requestPath, hasNextPage = findNextPage(resp) + } if hasNextPage && opts.ShowResponseHeaders { fmt.Fprint(opts.IO.Out, "\n") @@ -180,21 +206,7 @@ func apiRun(opts *ApiOptions) error { return nil } -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 { - continue - } - if m[2] == "next" { - return m[1], true - } - } - return "", false -} - -func processResponse(resp *http.Response, opts *ApiOptions) error { +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()) @@ -202,43 +214,55 @@ func processResponse(resp *http.Response, opts *ApiOptions) error { } if resp.StatusCode == 204 { - return nil + return } var responseBody io.Reader = resp.Body defer resp.Body.Close() isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) - var err error var serverError string 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 9debf7896..dc7a44e41 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) { @@ -293,7 +295,7 @@ func Test_apiRun(t *testing.T) { } } -func Test_apiRun_pagination(t *testing.T) { +func Test_apiRun_paginationREST(t *testing.T) { io, _, stdout, stderr := iostreams.Test() requestCount := 0 @@ -346,6 +348,83 @@ func Test_apiRun_pagination(t *testing.T) { 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..3bd00b822 --- /dev/null +++ b/pkg/cmd/api/pagination.go @@ -0,0 +1,87 @@ +package api + +import ( + "encoding/json" + "io" + "net/http" + "regexp" +) + +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 "" +} diff --git a/pkg/cmd/api/pagination_test.go b/pkg/cmd/api/pagination_test.go new file mode 100644 index 000000000..0ed4566da --- /dev/null +++ b/pkg/cmd/api/pagination_test.go @@ -0,0 +1,118 @@ +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) + } + }) + } +} From ab5cc6ae67486ba8e10da10b87ca9651ef05142c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 17 Jun 2020 12:25:50 -0500 Subject: [PATCH 32/39] mention future label --- docs/triage.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/triage.md b/docs/triage.md index 319554539..6f37b6a0c 100644 --- a/docs/triage.md +++ b/docs/triage.md @@ -27,7 +27,8 @@ just imagine a flowchart - 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. - - label appropriately (examples include `enhancement` or `bug`) + - 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 From ae695e98ceea9774ca8b99fdf80e0f0288919b63 Mon Sep 17 00:00:00 2001 From: AliabbasMerchant Date: Thu, 18 Jun 2020 09:00:56 +0530 Subject: [PATCH 33/39] Fix pr create not using .github/pull_request_template.md --- command/title_body_survey.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 } From 2fc4c000b5354021ba00cdce25445ac6f3afcfcb Mon Sep 17 00:00:00 2001 From: HowJMay Date: Mon, 22 Jun 2020 15:19:50 +0800 Subject: [PATCH 34/39] fix typo --- command/alias.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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, } From 55be0d2a9ca7282c2c25e13d3bd3c67bc27347b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 22 Jun 2020 19:13:28 +0200 Subject: [PATCH 35/39] Tweak `isColorEnabled` --- utils/color.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/utils/color.go b/utils/color.go index e88549667..447765273 100644 --- a/utils/color.go +++ b/utils/color.go @@ -10,8 +10,8 @@ import ( ) var ( - _isColorEnabled bool = true - _isStdoutTerminal, checkedTerminal, checkedNoColor bool + _isColorEnabled bool = true + _isStdoutTerminal, checkedTerminal bool // Outputs ANSI color if stdout is a tty Magenta = makeColorFunc("magenta") @@ -45,7 +45,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,13 +53,9 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if !isStdoutTerminal() { + if os.Getenv("NO_COLOR") != "" { return false } - if !checkedNoColor { - _isColorEnabled = os.Getenv("NO_COLOR") == "" - checkedNoColor = true - } - return _isColorEnabled + return isStdoutTerminal() } From 04b18ea8cb2933d096f5e6b38c559538c4e26fde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 22 Jun 2020 19:32:06 +0200 Subject: [PATCH 36/39] :fire: unused var --- utils/color.go | 1 - 1 file changed, 1 deletion(-) diff --git a/utils/color.go b/utils/color.go index 447765273..dd9a7d11c 100644 --- a/utils/color.go +++ b/utils/color.go @@ -10,7 +10,6 @@ import ( ) var ( - _isColorEnabled bool = true _isStdoutTerminal, checkedTerminal bool // Outputs ANSI color if stdout is a tty From e084a4563fb8aa0d294c0db9c360aacbdf52bef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 22 Jun 2020 19:36:55 +0200 Subject: [PATCH 37/39] Fix `pr checkout OWNER:BRANCH` when maintainer can modify We did not use to request the necessary GraphQL fields to determine whether the PR in question can be modified by maintainers (i.e. pushed back to). --- api/queries_pr.go | 4 ++++ 1 file changed, 4 insertions(+) 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 { From ac7d5ecc44a7821265b5cc8a00a9eace6428da56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 22 Jun 2020 19:44:16 +0200 Subject: [PATCH 38/39] Ensure markdown still passed through Glamour in no-color mode --- utils/utils.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/utils/utils.go b/utils/utils.go index 9441bc2bc..c84860d9e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -23,10 +23,6 @@ func OpenInBrowser(url string) error { func RenderMarkdown(text string) (string, error) { style := "notty" - if !isColorEnabled() { - return text, nil - } - if isColorEnabled() { style = "dark" } From c945fb4336ff6d6229bed30eb68d946deb110320 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 23 Jun 2020 18:42:57 +0200 Subject: [PATCH 39/39] Automatically add `per_page=100` to paginated REST requests Most endpoints respect this parameter by default. Those that don't will just ignore it. The `per_page=100` parameter is not added if there is already a `per_page` parameter specified in the request. --- pkg/cmd/api/api.go | 7 ++++- pkg/cmd/api/api_test.go | 2 +- pkg/cmd/api/pagination.go | 21 ++++++++++++++ pkg/cmd/api/pagination_test.go | 51 ++++++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 067ef6bc8..e1edabbc0 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -143,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) @@ -155,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 { @@ -189,7 +194,7 @@ func apiRun(opts *ApiOptions) error { break } - if opts.RequestPath == "graphql" { + if isGraphQL { hasNextPage = endCursor != "" if hasNextPage { params["endCursor"] = endCursor diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index dc7a44e41..7b89d715c 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -343,7 +343,7 @@ func Test_apiRun_paginationREST(t *testing.T) { 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", responses[0].Request.URL.String()) + 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()) } diff --git a/pkg/cmd/api/pagination.go b/pkg/cmd/api/pagination.go index 3bd00b822..fce88fb92 100644 --- a/pkg/cmd/api/pagination.go +++ b/pkg/cmd/api/pagination.go @@ -2,9 +2,12 @@ package api import ( "encoding/json" + "fmt" "io" "net/http" + "net/url" "regexp" + "strings" ) var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) @@ -85,3 +88,21 @@ loop: } 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 index 0ed4566da..3bb1a8ec5 100644 --- a/pkg/cmd/api/pagination_test.go +++ b/pkg/cmd/api/pagination_test.go @@ -116,3 +116,54 @@ func Test_findEndCursor(t *testing.T) { }) } } + +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) + } + }) + } +}