diff --git a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md index 86e73d76a..ca33dd34c 100644 --- a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md +++ b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md @@ -1,7 +1,13 @@ +--- +name: "\U0001F41B Bug fix" +about: Fix a bug in GitHub CLI + +--- + ## Summary diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..57e53d6fb --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,3 @@ +linters: + enable: + gofmt 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/client.go b/api/client.go index b8c6857fb..1c8c1eaa2 100644 --- a/api/client.go +++ b/api/client.go @@ -35,7 +35,11 @@ func NewClient(opts ...ClientOption) *Client { func AddHeader(name, value string) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - req.Header.Add(name, value) + // prevent the token from leaking to non-GitHub hosts + // TODO: GHE support + if !strings.EqualFold(name, "Authorization") || strings.HasSuffix(req.URL.Hostname(), ".github.com") { + req.Header.Add(name, value) + } return tr.RoundTrip(req) }} } @@ -45,7 +49,11 @@ func AddHeader(name, value string) ClientOption { func AddHeaderFunc(name string, value func() string) ClientOption { return func(tr http.RoundTripper) http.RoundTripper { return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { - req.Header.Add(name, value()) + // prevent the token from leaking to non-GitHub hosts + // TODO: GHE support + if !strings.EqualFold(name, "Authorization") || strings.HasSuffix(req.URL.Hostname(), ".github.com") { + req.Header.Add(name, value()) + } return tr.RoundTrip(req) }} } diff --git a/command/alias.go b/command/alias.go index da005a4ab..64056f9d5 100644 --- a/command/alias.go +++ b/command/alias.go @@ -5,6 +5,7 @@ import ( "sort" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" @@ -23,24 +24,30 @@ var aliasCmd = &cobra.Command{ } var aliasSetCmd = &cobra.Command{ - Use: "set ", + Use: "set ", + Short: "Create a shortcut for a gh command", + Long: `Declare a word as a command alias that will expand to the specified command. + +The expansion may specify additional arguments and flags. If the expansion +includes positional placeholders such as '$1', '$2', etc., any extra arguments +that follow the invocation of an alias will be inserted appropriately.`, + Example: heredoc.Doc(` + $ gh alias set pv 'pr view' + $ gh pv -w 123 + #=> gh pr view -w 123 + + $ gh alias set bugs 'issue list --label="bugs"' + + $ gh alias set epicsBy 'issue list --author="$1" --label="epic"' + $ gh epicsBy vilmibm + #=> gh issue list --author="vilmibm" --label="epic" + `), + Args: cobra.MinimumNArgs(2), + RunE: aliasSet, + // NB: this allows a user to eschew quotes when specifiying 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, - Short: "Create a shortcut for a gh command", - Long: `This command lets you write your own shortcuts for running gh. They can be simple strings or accept placeholder arguments.`, - Example: ` - gh alias set pv 'pr view' - # gh pv -w 123 -> gh pr view -w 123. - - gh alias set bugs 'issue list --label="bugs"' - # gh bugs -> gh issue list --label="bugs". - - gh alias set epicsBy 'issue list --author="$1" --label="epic"' - # gh epicsBy vilmibm -> gh issue list --author="$1" --label="epic" -`, - Args: cobra.MinimumNArgs(2), - RunE: aliasSet, } func aliasSet(cmd *cobra.Command, args []string) error { @@ -168,11 +175,10 @@ func aliasList(cmd *cobra.Command, args []string) error { } var aliasDeleteCmd = &cobra.Command{ - Use: "delete ", - Short: "Delete an alias.", - Args: cobra.ExactArgs(1), - Example: "gh alias delete co", - RunE: aliasDelete, + Use: "delete ", + Short: "Delete an alias.", + Args: cobra.ExactArgs(1), + RunE: aliasDelete, } func aliasDelete(cmd *cobra.Command, args []string) error { diff --git a/command/config.go b/command/config.go index 7e2a541f7..7f34b8687 100644 --- a/command/config.go +++ b/command/config.go @@ -2,6 +2,8 @@ package command import ( "fmt" + + "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" ) @@ -21,36 +23,32 @@ func init() { var configCmd = &cobra.Command{ Use: "config", - Short: "Set and get gh settings", - Long: `Get and set key/value strings. + Short: "Manage configuration for gh", + Long: `Display or change configuration settings for gh. Current respected settings: -- git_protocol: https or ssh. Default is https. +- git_protocol: "https" or "ssh". Default is "https". - editor: if unset, defaults to environment variables. `, } var configGetCmd = &cobra.Command{ Use: "get ", - Short: "Prints the value of a given configuration key", - Long: `Get the value for a given configuration key. - -Examples: - $ gh config get git_protocol - https -`, + Short: "Print the value of a given configuration key", + Example: heredoc.Doc(` + $ gh config get git_protocol + https + `), Args: cobra.ExactArgs(1), RunE: configGet, } var configSetCmd = &cobra.Command{ Use: "set ", - Short: "Updates configuration with the value of a given key", - Long: `Update the configuration by setting a key to a value. - -Examples: - $ gh config set editor vim -`, + Short: "Update configuration with a value for the given key", + Example: heredoc.Doc(` + $ gh config set editor vim + `), Args: cobra.ExactArgs(2), RunE: configSet, } diff --git a/command/credits.go b/command/credits.go index 9d5560bad..0104edb02 100644 --- a/command/credits.go +++ b/command/credits.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" "golang.org/x/crypto/ssh/terminal" @@ -44,11 +45,16 @@ var creditsCmd = &cobra.Command{ Use: "credits", Short: "View credits for this tool", Long: `View animated credits for gh, the tool you are currently using :)`, - Example: `gh credits # see a credits animation for this project - gh credits owner/repo # see a credits animation for owner/repo - gh credits -s # display a non-animated thank you - gh credits | cat # just print the contributors, one per line -`, + Example: heredoc.Doc(` + # see a credits animation for this project + $ gh credits + + # display a non-animated thank you + $ gh credits -s + + # just print the contributors, one per line + $ gh credits | cat + `), Args: cobra.ExactArgs(0), RunE: ghCredits, Hidden: true, diff --git a/command/gist.go b/command/gist.go index 04e6133fe..227f8bd92 100644 --- a/command/gist.go +++ b/command/gist.go @@ -8,6 +8,7 @@ import ( "os" "path" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/utils" "github.com/spf13/cobra" @@ -27,21 +28,30 @@ var gistCmd = &cobra.Command{ } var gistCreateCmd = &cobra.Command{ - Use: `create {|-}...`, + Use: `create [... | -]`, Short: "Create a new gist", - Long: `gh gist create: create gists + Long: `Create a new GitHub gist with given contents. -Gists can be created from one or many files. This command can also read from STDIN. By default, gists are private; use --public to change this. +Gists can be created from one or multiple files. Alternatively, pass "-" as +file name to read from standard input. -Examples +By default, gists are private; use '--public' to make publicly listed ones.`, + Example: heredoc.Doc(` + # publish file 'hello.py' as a public gist + $ gh gist create --public hello.py + + # create a gist with a description + $ gh gist create hello.py -d "my Hello-World program in Python" - gh gist create hello.py # turn file hello.py into a gist - gh gist create --public hello.py # turn file hello.py into a public gist - gh gist create -d"a file!" hello.py # turn file hello.py into a gist, with description - gh gist create hello.py world.py cool.txt # make a gist out of several files - gh gist create - # read from STDIN to create a gist - cat cool.txt | gh gist create # read the output of another command and make a gist out of it -`, + # create a gist containing several files + $ gh gist create hello.py world.py cool.txt + + # read from standard input to create a gist + $ gh gist create - + + # create a gist from output piped from another command + $ cat cool.txt | gh gist create + `), RunE: gistCreate, } diff --git a/command/help.go b/command/help.go new file mode 100644 index 000000000..b776d23f1 --- /dev/null +++ b/command/help.go @@ -0,0 +1,122 @@ +package command + +import ( + "fmt" + "regexp" + "strings" + + "github.com/cli/cli/utils" + "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 + } + 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) + } + } + + coreCommands := []string{} + additionalCommands := []string{} + for _, c := range command.Commands() { + if c.Short == "" { + continue + } + if c.Hidden { + continue + } + + s := rpad(c.Name()+":", c.NamePadding()) + c.Short + if _, ok := c.Annotations["IsCore"]; ok { + coreCommands = append(coreCommands, s) + } else { + additionalCommands = append(additionalCommands, s) + } + } + + // If there are no core commands, assume everything is a core command + if len(coreCommands) == 0 { + coreCommands = additionalCommands + additionalCommands = []string{} + } + + type helpEntry struct { + Title string + Body string + } + + helpEntries := []helpEntry{} + if command.Long != "" { + helpEntries = append(helpEntries, helpEntry{"", command.Long}) + } else if command.Short != "" { + helpEntries = append(helpEntries, helpEntry{"", command.Short}) + } + helpEntries = append(helpEntries, helpEntry{"USAGE", command.UseLine()}) + if len(coreCommands) > 0 { + helpEntries = append(helpEntries, helpEntry{"CORE COMMANDS", strings.Join(coreCommands, "\n")}) + } + if len(additionalCommands) > 0 { + helpEntries = append(helpEntries, helpEntry{"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}) + } + + flagUsages := command.LocalFlags().FlagUsages() + if flagUsages != "" { + dedent := regexp.MustCompile(`(?m)^ `) + helpEntries = append(helpEntries, helpEntry{"FLAGS", dedent.ReplaceAllString(flagUsages, "")}) + } + if _, ok := command.Annotations["help:arguments"]; ok { + helpEntries = append(helpEntries, helpEntry{"ARGUMENTS", command.Annotations["help:arguments"]}) + } + if command.Example != "" { + helpEntries = append(helpEntries, helpEntry{"EXAMPLES", command.Example}) + } + helpEntries = append(helpEntries, helpEntry{"LEARN MORE", ` +Use "gh --help" for more information about a command. +Read the manual at https://cli.github.com/manual`}) + if _, ok := command.Annotations["help:feedback"]; ok { + helpEntries = append(helpEntries, helpEntry{"FEEDBACK", command.Annotations["help:feedback"]}) + } + + out := colorableOut(command) + for _, e := range helpEntries { + if e.Title != "" { + // If there is a title, add indentation to each line in the body + fmt.Fprintln(out, utils.Bold(e.Title)) + + for _, l := range strings.Split(strings.Trim(e.Body, "\n\r"), "\n") { + fmt.Fprintln(out, " "+l) + } + } else { + // If there is no title print the body as is + fmt.Fprintln(out, e.Body) + } + fmt.Fprintln(out) + } +} + +// rpad adds padding to the right of a string. +func rpad(s string, padding int) string { + template := fmt.Sprintf("%%-%ds ", padding) + return fmt.Sprintf(template, s) +} diff --git a/command/issue.go b/command/issue.go index cf5087bff..a66235ddd 100644 --- a/command/issue.go +++ b/command/issue.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" @@ -49,13 +50,19 @@ func init() { } var issueCmd = &cobra.Command{ - Use: "issue", + Use: "issue ", Short: "Create and view issues", - Long: `Work with GitHub issues. - -An issue can be supplied as argument in any of the following formats: + Long: `Work with GitHub issues`, + Example: heredoc.Doc(` + $ gh issue list + $ gh issue create --label bug + $ gh issue view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": `An issue can be supplied as argument in any of the following formats: - by number, e.g. "123"; or -- by URL, e.g. "https://github.com/OWNER/REPO/issues/123".`, +- by URL, e.g. "https://github.com/OWNER/REPO/issues/123".`}, } var issueCreateCmd = &cobra.Command{ Use: "create", @@ -135,6 +142,9 @@ func issueList(cmd *cobra.Command, args []string) error { if err != nil { return err } + if limit <= 0 { + return fmt.Errorf("invalid limit: %v", limit) + } author, err := cmd.Flags().GetString("author") if err != nil { @@ -248,11 +258,9 @@ func issueView(cmd *cobra.Command, args []string) error { if web { fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) - } else { - out := colorableOut(cmd) - return printIssuePreview(out, issue) } - + out := colorableOut(cmd) + return printIssuePreview(out, issue) } func issueStateTitleWithColor(state string) string { @@ -361,11 +369,11 @@ func issueCreate(cmd *cobra.Command, args []string) error { return err } - var templateFiles []string + var nonLegacyTemplateFiles []string if baseOverride == "" { if rootDir, err := git.ToplevelDir(); err == nil { // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "ISSUE_TEMPLATE") + nonLegacyTemplateFiles = githubtemplate.FindNonLegacy(rootDir, "ISSUE_TEMPLATE") } } @@ -409,7 +417,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { if err != nil { return err } - } else if len(templateFiles) > 1 { + } else if len(nonLegacyTemplateFiles) > 1 { openURL += "/choose" } cmd.Printf("Opening %s in your browser.\n", displayURL(openURL)) @@ -428,6 +436,7 @@ func issueCreate(cmd *cobra.Command, args []string) error { action := SubmitAction tb := issueMetadataState{ + Type: issueMetadata, Assignees: assignees, Labels: labelNames, Projects: projectNames, @@ -437,7 +446,14 @@ func issueCreate(cmd *cobra.Command, args []string) error { interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) if interactive { - err := titleBodySurvey(cmd, &tb, apiClient, baseRepo, title, body, defaults{}, templateFiles, false, repo.ViewerCanTriage()) + var legacyTemplateFile *string + if baseOverride == "" { + if rootDir, err := git.ToplevelDir(); err == nil { + // TODO: figure out how to stub this in tests + legacyTemplateFile = githubtemplate.FindLegacy(rootDir, "ISSUE_TEMPLATE") + } + } + err := titleBodySurvey(cmd, &tb, apiClient, baseRepo, title, body, defaults{}, nonLegacyTemplateFiles, legacyTemplateFile, false, repo.ViewerCanTriage()) if err != nil { return fmt.Errorf("could not collect title and/or body: %w", err) } diff --git a/command/issue_test.go b/command/issue_test.go index a612cfab4..418de25d3 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -181,6 +181,18 @@ No issues match your search in OWNER/REPO eq(t, reqBody.Variables.Author, "foo") } +func TestIssueList_withInvalidLimitFlag(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand("issue list --limit=0") + + if err == nil || err.Error() != "invalid limit: 0" { + t.Errorf("error running command `issue list`: %v", err) + } +} + func TestIssueList_nullAssigneeLabels(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/pr.go b/command/pr.go index e92fa7cf0..c5fc1ae7b 100644 --- a/command/pr.go +++ b/command/pr.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/git" @@ -46,19 +47,30 @@ func init() { } var prCmd = &cobra.Command{ - Use: "pr", + Use: "pr ", Short: "Create, view, and checkout pull requests", - Long: `Work with GitHub pull requests. - -A pull request can be supplied as argument in any of the following formats: + Long: `Work with GitHub pull requests`, + Example: heredoc.Doc(` + $ gh pr checkout 353 + $ gh pr create --fill + $ gh pr view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": `A pull request can be supplied as argument in any of the following formats: - by number, e.g. "123"; - by URL, e.g. "https://github.com/OWNER/REPO/pull/123"; or -- by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`, +- by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1".`}, } var prListCmd = &cobra.Command{ Use: "list", Short: "List and filter pull requests in this repository", - RunE: prList, + Example: heredoc.Doc(` + $ gh pr list --limit 999 + $ gh pr list --state closed + $ gh pr list --label "priority 1" --label "bug" + `), + RunE: prList, } var prStatusCmd = &cobra.Command{ Use: "status", @@ -183,6 +195,10 @@ func prList(cmd *cobra.Command, args []string) error { if err != nil { return err } + if limit <= 0 { + return fmt.Errorf("invalid limit: %v", limit) + } + state, err := cmd.Flags().GetString("state") if err != nil { return err @@ -316,10 +332,9 @@ func prView(cmd *cobra.Command, args []string) error { if web { fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) - } else { - out := colorableOut(cmd) - return printPrPreview(out, pr) } + out := colorableOut(cmd) + return printPrPreview(out, pr) } func prClose(cmd *cobra.Command, args []string) error { diff --git a/command/pr_create.go b/command/pr_create.go index 45ec801d4..5eafdbcbe 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -40,8 +40,8 @@ func computeDefaults(baseRef, headRef string) (defaults, error) { out.Title = utils.Humanize(headRef) body := "" - for _, c := range commits { - body += fmt.Sprintf("- %s\n", c.Title) + for i := len(commits) - 1; i >= 0; i-- { + body += fmt.Sprintf("- %s\n", commits[i].Title) } out.Body = body } @@ -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)) @@ -203,6 +213,7 @@ func prCreate(cmd *cobra.Command, _ []string) error { } tb := issueMetadataState{ + Type: prMetadata, Reviewers: reviewers, Assignees: assignees, Labels: labelNames, @@ -213,13 +224,14 @@ func prCreate(cmd *cobra.Command, _ []string) error { interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) if !isWeb && !autofill && interactive { - var templateFiles []string + var nonLegacyTemplateFiles []string + var legacyTemplateFile *string if rootDir, err := git.ToplevelDir(); err == nil { // TODO: figure out how to stub this in tests - templateFiles = githubtemplate.Find(rootDir, "PULL_REQUEST_TEMPLATE") + nonLegacyTemplateFiles = githubtemplate.FindNonLegacy(rootDir, "PULL_REQUEST_TEMPLATE") + legacyTemplateFile = githubtemplate.FindLegacy(rootDir, "PULL_REQUEST_TEMPLATE") } - - err := titleBodySurvey(cmd, &tb, client, baseRepo, title, body, defs, templateFiles, true, baseRepo.ViewerCanTriage()) + err := titleBodySurvey(cmd, &tb, client, baseRepo, title, body, defs, nonLegacyTemplateFiles, legacyTemplateFile, true, baseRepo.ViewerCanTriage()) if err != nil { return fmt.Errorf("could not collect title and/or body: %w", err) } @@ -243,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") } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index e13ecc15f..2eb0872c7 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -519,7 +519,7 @@ func TestPRCreate_survey_defaults_multicommit(t *testing.T) { }{} _ = json.Unmarshal(bodyBytes, &reqBody) - expectedBody := "- commit 0\n- commit 1\n" + expectedBody := "- commit 1\n- commit 0\n" eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") eq(t, reqBody.Variables.Input.Title, "cool bug fixes") diff --git a/command/pr_review.go b/command/pr_review.go index 642c9ac24..08efc638d 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" "github.com/cli/cli/api" @@ -23,18 +24,24 @@ func init() { var prReviewCmd = &cobra.Command{ Use: "review [ | | ]", - Short: "Add a review to a pull request.", - Args: cobra.MaximumNArgs(1), - Long: `Add a review to either a specified pull request or the pull request associated with the current branch. + Short: "Add a review to a pull request", + Long: `Add a review to a pull request. -Examples: - - gh pr review # add a review for the current branch's pull request - gh pr review 123 # add a review for pull request 123 - gh pr review -a # mark the current branch's pull request as approved - gh pr review -c -b "interesting" # comment on the current branch's pull request - gh pr review 123 -r -b "needs more ascii art" # request changes on pull request 123 - `, +Without an argument, the pull request that belongs to the current branch is reviewed.`, + Example: heredoc.Doc(` + # approve the pull request of the current branch + $ gh pr review --approve + + # leave a review comment for the current branch + $ gh pr review --comment -b "interesting" + + # add a review for a specific pull request + $ gh pr review 123 + + # request changes on a specific pull request + $ gh pr review 123 -r -b "needs more ASCII art" + `), + Args: cobra.MaximumNArgs(1), RunE: prReview, } @@ -180,7 +187,7 @@ func reviewSurvey(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { } bodyQs := []*survey.Question{ - &survey.Question{ + { Name: "body", Prompt: &surveyext.GhEditor{ BlankAllowed: blankAllowed, diff --git a/command/pr_review_test.go b/command/pr_review_test.go index 612f6c574..2c2673db4 100644 --- a/command/pr_review_test.go +++ b/command/pr_review_test.go @@ -213,10 +213,10 @@ func TestPRReview(t *testing.T) { ExpectedBody string } cases := []c{ - c{`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, - c{`pr review --approve`, "APPROVE", ""}, - c{`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"}, - c{`pr review --comment --body "i donno"`, "COMMENT", "i donno"}, + {`pr review --request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, + {`pr review --approve`, "APPROVE", ""}, + {`pr review --approve -b"hot damn"`, "APPROVE", "hot damn"}, + {`pr review --comment --body "i donno"`, "COMMENT", "i donno"}, } for _, kase := range cases { diff --git a/command/pr_test.go b/command/pr_test.go index a11c60820..4c31198b0 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -437,6 +437,17 @@ func TestPRList_filteringAssigneeLabels(t *testing.T) { } } +func TestPRList_withInvalidLimitFlag(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + _, err := RunCommand(`pr list --limit=0`) + if err == nil && err.Error() != "invalid limit: 0" { + t.Errorf("error running command `issue list`: %v", err) + } +} + func TestPRView_Preview(t *testing.T) { tests := map[string]struct { ownerRepo string diff --git a/command/repo.go b/command/repo.go index b1a57d3a7..7c15945aa 100644 --- a/command/repo.go +++ b/command/repo.go @@ -10,6 +10,7 @@ import ( "time" "github.com/AlecAivazis/survey/v2" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" @@ -44,13 +45,20 @@ func init() { } var repoCmd = &cobra.Command{ - Use: "repo", + Use: "repo ", Short: "Create, clone, fork, and view repositories", - Long: `Work with GitHub repositories. - + Long: `Work with GitHub repositories`, + Example: heredoc.Doc(` + $ gh repo create + $ gh repo clone cli/cli + $ gh repo view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": ` A repository can be supplied as an argument in any of the following formats: - "OWNER/REPO" -- by URL, e.g. "https://github.com/OWNER/REPO"`, +- by URL, e.g. "https://github.com/OWNER/REPO"`}, } var repoCloneCmd = &cobra.Command{ @@ -69,9 +77,20 @@ To pass 'git clone' flags, separate them with '--'.`, var repoCreateCmd = &cobra.Command{ Use: "create []", Short: "Create a new repository", - Long: `Create a new GitHub repository. + Long: `Create a new GitHub repository.`, + Example: heredoc.Doc(` + # create a repository under your account using the current directory name + $ gh repo create -Use the "ORG/NAME" syntax to create a repository within your organization.`, + # create a repository with a specific name + $ gh repo create my-project + + # create a repository in an organization + $ gh repo create cli/my-project + `), + Annotations: map[string]string{"help:arguments": `A repository can be supplied as an argument in any of the following formats: +- +- by URL, e.g. "https://github.com/OWNER/REPO"`}, RunE: repoCreate, } @@ -98,11 +117,19 @@ With '--web', open the repository in a web browser instead.`, var repoCreditsCmd = &cobra.Command{ Use: "credits []", Short: "View credits for a repository", - Example: `$ gh repo credits # view credits for the current repository -$ gh repo credits cool/repo # view credits for cool/repo -$ gh repo credits -s # print a non-animated thank you -$ gh repo credits | cat # pipe to just print the contributors, one per line -`, + Example: heredoc.Doc(` + # view credits for the current repository + $ gh repo credits + + # view credits for a specific repository + $ gh repo credits cool/repo + + # print a non-animated thank you + $ gh repo credits -s + + # pipe to just print the contributors, one per line + $ gh repo credits | cat + `), Args: cobra.MaximumNArgs(1), RunE: repoCredits, Hidden: true, diff --git a/command/repo_test.go b/command/repo_test.go index ff882d12e..52928c8f4 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -1,7 +1,6 @@ package command import ( - "bytes" "encoding/json" "io/ioutil" "os/exec" @@ -459,11 +458,13 @@ func TestRepoClone(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "parent": null - } } } - `)) + http.Register( + httpmock.GraphQL(`\brepository\(`), + httpmock.StringResponse(` + { "data": { "repository": { + "parent": null + } } } + `)) cs, restore := test.InitCmdStubber() defer restore() @@ -485,14 +486,16 @@ func TestRepoClone(t *testing.T) { func TestRepoClone_hasParent(t *testing.T) { http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "parent": { - "owner": {"login": "hubot"}, - "name": "ORIG" - } - } } } - `)) + http.Register( + httpmock.GraphQL(`\brepository\(`), + httpmock.StringResponse(` + { "data": { "repository": { + "parent": { + "owner": {"login": "hubot"}, + "name": "ORIG" + } + } } } + `)) cs, restore := test.InitCmdStubber() defer restore() @@ -548,18 +551,19 @@ func TestRepoCreate(t *testing.T) { } http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createRepository": { - "repository": { - "id": "REPOID", - "url": "https://github.com/OWNER/REPO", - "name": "REPO", - "owner": { - "login": "OWNER" + http.Register( + httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.StringResponse(` + { "data": { "createRepository": { + "repository": { + "id": "REPOID", + "url": "https://github.com/OWNER/REPO", + "name": "REPO", + "owner": { + "login": "OWNER" + } } - } - } } } - `)) + } } }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -613,22 +617,24 @@ func TestRepoCreate_org(t *testing.T) { } http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "node_id": "ORGID" - } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createRepository": { - "repository": { - "id": "REPOID", - "url": "https://github.com/ORG/REPO", - "name": "REPO", - "owner": { - "login": "ORG" + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` + { "node_id": "ORGID" + }`)) + http.Register( + httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.StringResponse(` + { "data": { "createRepository": { + "repository": { + "id": "REPOID", + "url": "https://github.com/ORG/REPO", + "name": "REPO", + "owner": { + "login": "ORG" + } } - } - } } } - `)) + } } }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -681,23 +687,25 @@ func TestRepoCreate_orgWithTeam(t *testing.T) { } http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { "node_id": "TEAMID", - "organization": { "node_id": "ORGID" } - } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createRepository": { - "repository": { - "id": "REPOID", - "url": "https://github.com/ORG/REPO", - "name": "REPO", - "owner": { - "login": "ORG" + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` + { "node_id": "TEAMID", + "organization": { "node_id": "ORGID" } + }`)) + http.Register( + httpmock.GraphQL(`\bcreateRepository\(`), + httpmock.StringResponse(` + { "data": { "createRepository": { + "repository": { + "id": "REPOID", + "url": "https://github.com/ORG/REPO", + "name": "REPO", + "owner": { + "login": "ORG" + } } - } - } } } - `)) + } } }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -746,9 +754,10 @@ func TestRepoView_web(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { } - `)) + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` + { }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -779,9 +788,10 @@ func TestRepoView_web_ownerRepo(t *testing.T) { return ctx } http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { } - `)) + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` + { }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { @@ -812,9 +822,10 @@ func TestRepoView_web_fullURL(t *testing.T) { return ctx } http := initFakeHTTP() - http.StubResponse(200, bytes.NewBufferString(` - { } - `)) + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` + { }`)) var seenCmd *exec.Cmd restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { seenCmd = cmd @@ -841,16 +852,18 @@ func TestRepoView(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`\brepository\(`), + httpmock.StringResponse(` { "data": { - "repository": { - "description": "social distancing" - }}} - `)) - http.StubResponse(200, bytes.NewBufferString(` + "repository": { + "description": "social distancing" + } } }`)) + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` { "name": "readme.md", - "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="} - `)) + "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`)) output, err := RunCommand("repo view") if err != nil { @@ -869,16 +882,18 @@ func TestRepoView_nonmarkdown_readme(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`\brepository\(`), + httpmock.StringResponse(` { "data": { "repository": { - "description": "social distancing" - }}} - `)) - http.StubResponse(200, bytes.NewBufferString(` + "description": "social distancing" + } } }`)) + http.Register( + httpmock.MatchAny, + httpmock.StringResponse(` { "name": "readme.org", - "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="} - `)) + "content": "IyB0cnVseSBjb29sIHJlYWRtZSBjaGVjayBpdCBvdXQ="}`)) output, err := RunCommand("repo view") if err != nil { @@ -896,8 +911,8 @@ func TestRepoView_blanks(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString("{}")) - http.StubResponse(200, bytes.NewBufferString("{}")) + http.Register(httpmock.MatchAny, httpmock.StringResponse("{}")) + http.Register(httpmock.MatchAny, httpmock.StringResponse("{}")) output, err := RunCommand("repo view") if err != nil { diff --git a/command/root.go b/command/root.go index 05e864eb7..5b9923fa7 100644 --- a/command/root.go +++ b/command/root.go @@ -10,6 +10,7 @@ import ( "runtime/debug" "strings" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/internal/config" @@ -34,7 +35,6 @@ var Version = "DEV" var BuildDate = "" // YYYY-MM-DD var versionOutput = "" -var cobraDefaultHelpFunc func(*cobra.Command, []string) func init() { if Version == "DEV" { @@ -58,9 +58,11 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") - cobraDefaultHelpFunc = RootCmd.HelpFunc() RootCmd.SetHelpFunc(rootHelpFunc) + // This will silence the usage func on error + RootCmd.SetUsageFunc(func(_ *cobra.Command) error { return nil }) + RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err @@ -74,8 +76,10 @@ func init() { HttpClient: func() (*http.Client, error) { token := os.Getenv("GITHUB_TOKEN") if len(token) == 0 { + // TODO: decouple from `context` ctx := context.New() var err error + // TODO: pass IOStreams to this so that the auth flow knows if it's interactive or not token, err = ctx.AuthToken() if err != nil { return nil, err @@ -83,6 +87,11 @@ func init() { } return httpClient(token), nil }, + BaseRepo: func() (ghrepo.Interface, error) { + // TODO: decouple from `context` + ctx := context.New() + return ctx.BaseRepo() + }, } RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil)) } @@ -95,6 +104,15 @@ var RootCmd = &cobra.Command{ SilenceErrors: true, SilenceUsage: true, + Example: heredoc.Doc(` + $ gh issue create + $ gh repo clone cli/cli + $ gh pr checkout 321 + `), + Annotations: map[string]string{ + "help:feedback": ` +Fill out our feedback form https://forms.gle/umxd3h31c7aMQFKG7 +Open an issue using “gh issue create -R cli/cli”`}, } var versionCmd = &cobra.Command{ @@ -320,100 +338,6 @@ func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Co return baseRepo, nil } -func rootHelpFunc(command *cobra.Command, args []string) { - if command != RootCmd { - // 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.Parent() == RootCmd && len(args) >= 2 { - if command.SuggestionsMinimumDistance <= 0 { - command.SuggestionsMinimumDistance = 2 - } - 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) - } - cobraDefaultHelpFunc(command, args) - return - } - - type helpEntry struct { - Title string - Body string - } - - coreCommandNames := []string{"issue", "pr", "repo"} - var coreCommands []string - var additionalCommands []string - for _, c := range command.Commands() { - if c.Short == "" { - continue - } - s := " " + rpad(c.Name()+":", c.NamePadding()) + c.Short - if includes(coreCommandNames, c.Name()) { - coreCommands = append(coreCommands, s) - } else if !c.Hidden { - additionalCommands = append(additionalCommands, s) - } - } - - helpEntries := []helpEntry{ - { - "", - command.Long}, - {"USAGE", command.Use}, - {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, - {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, - {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, - {"EXAMPLES", ` - $ gh issue create - $ gh repo clone - $ gh pr checkout 321`}, - {"LEARN MORE", ` - Use "gh --help" for more information about a command. - Read the manual at http://cli.github.com/manual`}, - {"FEEDBACK", ` - Fill out our feedback form https://forms.gle/umxd3h31c7aMQFKG7 - Open an issue using “gh issue create -R cli/cli”`}, - } - - out := colorableOut(command) - for _, e := range helpEntries { - if e.Title != "" { - fmt.Fprintln(out, utils.Bold(e.Title)) - } - fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") - } -} - -// rpad adds padding to the right of a string. -func rpad(s string, padding int) string { - template := fmt.Sprintf("%%-%ds ", padding) - return fmt.Sprintf(template, s) -} - -func includes(a []string, s string) bool { - for _, x := range a { - if x == s { - return true - } - } - return false -} - func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string { ctx := contextForCommand(cmd) diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 2b8bdda3a..1dcf3c668 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -13,8 +13,16 @@ import ( ) type Action int +type metadataStateType int + +const ( + issueMetadata metadataStateType = iota + prMetadata +) type issueMetadataState struct { + Type metadataStateType + Body string Title string Action Action @@ -99,35 +107,46 @@ func confirmSubmission(allowPreview bool, allowMetadata bool) (Action, error) { } } -func selectTemplate(templatePaths []string) (string, error) { +func selectTemplate(nonLegacyTemplatePaths []string, legacyTemplatePath *string, metadataType metadataStateType) (string, error) { templateResponse := struct { Index int }{} - if len(templatePaths) > 1 { - templateNames := make([]string, 0, len(templatePaths)) - for _, p := range templatePaths { - templateNames = append(templateNames, githubtemplate.ExtractName(p)) - } - - selectQs := []*survey.Question{ - { - Name: "index", - Prompt: &survey.Select{ - Message: "Choose a template", - Options: templateNames, - }, - }, - } - if err := SurveyAsk(selectQs, &templateResponse); err != nil { - return "", fmt.Errorf("could not prompt: %w", err) - } + templateNames := make([]string, 0, len(nonLegacyTemplatePaths)) + for _, p := range nonLegacyTemplatePaths { + templateNames = append(templateNames, githubtemplate.ExtractName(p)) + } + if metadataType == issueMetadata { + templateNames = append(templateNames, "Open a blank issue") + } else if metadataType == prMetadata { + templateNames = append(templateNames, "Open a blank pull request") } - templateContents := githubtemplate.ExtractContents(templatePaths[templateResponse.Index]) + selectQs := []*survey.Question{ + { + Name: "index", + Prompt: &survey.Select{ + Message: "Choose a template", + Options: templateNames, + }, + }, + } + if err := SurveyAsk(selectQs, &templateResponse); err != nil { + return "", fmt.Errorf("could not prompt: %w", err) + } + + if templateResponse.Index == len(nonLegacyTemplatePaths) { // the user has selected the blank template + if legacyTemplatePath != nil { + templateContents := githubtemplate.ExtractContents(*legacyTemplatePath) + return string(templateContents), nil + } else { + return "", nil + } + } + templateContents := githubtemplate.ExtractContents(nonLegacyTemplatePaths[templateResponse.Index]) return string(templateContents), nil } -func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClient *api.Client, repo ghrepo.Interface, providedTitle, providedBody string, defs defaults, templatePaths []string, allowReviewers, allowMetadata bool) error { +func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClient *api.Client, repo ghrepo.Interface, providedTitle, providedBody string, defs defaults, nonLegacyTemplatePaths []string, legacyTemplatePath *string, allowReviewers, allowMetadata bool) error { editorCommand, err := determineEditor(cmd) if err != nil { return err @@ -137,13 +156,15 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie templateContents := "" if providedBody == "" { - if len(templatePaths) > 0 { + if len(nonLegacyTemplatePaths) > 0 { var err error - templateContents, err = selectTemplate(templatePaths) + templateContents, err = selectTemplate(nonLegacyTemplatePaths, legacyTemplatePath, issueState.Type) if err != nil { return err } issueState.Body = templateContents + } else if legacyTemplatePath != nil { + issueState.Body = string(githubtemplate.ExtractContents(*legacyTemplatePath)) } else { issueState.Body = defs.Body } @@ -350,10 +371,8 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie issueState.Assignees = values.Assignees issueState.Labels = values.Labels issueState.Projects = values.Projects - issueState.Milestones = []string{values.Milestone} - - if len(issueState.Milestones) > 0 && issueState.Milestones[0] == noMilestone { - issueState.Milestones = issueState.Milestones[0:0] + if values.Milestone != "" && values.Milestone != noMilestone { + issueState.Milestones = []string{values.Milestone} } allowPreview = !issueState.HasMetadata() diff --git a/context/remote_test.go b/context/remote_test.go index 9cf7a0adf..1e0f47ba0 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -95,12 +95,12 @@ func Test_resolvedRemotes_triangularSetup(t *testing.T) { }, Network: api.RepoNetworkResult{ Repositories: []*api.Repository{ - &api.Repository{ + { Name: "NEWNAME", Owner: api.RepositoryOwner{Login: "NEWOWNER"}, ViewerPermission: "READ", }, - &api.Repository{ + { Name: "REPO", Owner: api.RepositoryOwner{Login: "MYSELF"}, ViewerPermission: "ADMIN", @@ -163,7 +163,7 @@ func Test_resolvedRemotes_forkLookup(t *testing.T) { }, Network: api.RepoNetworkResult{ Repositories: []*api.Repository{ - &api.Repository{ + { Name: "NEWNAME", Owner: api.RepositoryOwner{Login: "NEWOWNER"}, ViewerPermission: "READ", @@ -196,7 +196,7 @@ func Test_resolvedRemotes_clonedFork(t *testing.T) { }, Network: api.RepoNetworkResult{ Repositories: []*api.Repository{ - &api.Repository{ + { Name: "REPO", Owner: api.RepositoryOwner{Login: "OWNER"}, ViewerPermission: "ADMIN", diff --git a/docs/triage.md b/docs/triage.md new file mode 100644 index 000000000..95e1bf852 --- /dev/null +++ b/docs/triage.md @@ -0,0 +1,62 @@ +# Triage role + +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. + +# Incoming issues + +just imagine a flowchart + +- can this be closed outright? + - e.g. spam/junk + - close without comment +- do we not want to do it? + - e.g. have already discussed not wanting to do or duplicate issue + - comment acknowledging receipt + - 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`) + - 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 +- does it need more info from the issue author? + - ask the user for that + - add `needs-user-input` label +- is it a user asking for help and you have all the info you need to help? + - try and help + +# Incoming PRs + +just imagine a flowchart + +- can it be closed outright? + - ie spam/junk +- do we not want to do it? + - ie have already discussed not wanting to do, duplicate issue + - comment acknowledging receipt + - close +- is it intriguing but needs discussion? + - request an issue + - close +- is it something we want to include? + - add `community` label + - add to `needs review` column + +# Weekly PR audit + +In the interest of not letting our open PR list get out of hand (20+ total PRs _or_ multiple PRs +over a few months old), try to audit open PRs each week with the goal of getting them merged and/or +closed. It's likely too much work to deal with every PR, but even getting a few closer to done is +helpful. + +For each PR, ask: + +- is this too stale? 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 diff --git a/git/git_test.go b/git/git_test.go index c84023e40..8cd97a10d 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -15,9 +15,9 @@ func Test_UncommittedChangeCount(t *testing.T) { Output string } cases := []c{ - c{Label: "no changes", Expected: 0, Output: ""}, - c{Label: "one change", Expected: 1, Output: " M poem.txt"}, - c{Label: "untracked file", Expected: 2, Output: " M poem.txt\n?? new.txt"}, + {Label: "no changes", Expected: 0, Output: ""}, + {Label: "one change", Expected: 1, Output: " M poem.txt"}, + {Label: "untracked file", Expected: 2, Output: " M poem.txt\n?? new.txt"}, } teardown := run.SetPrepareCmd(func(*exec.Cmd) run.Runnable { diff --git a/git/ssh_config_test.go b/git/ssh_config_test.go index 35a0c93e6..28f339aa6 100644 --- a/git/ssh_config_test.go +++ b/git/ssh_config_test.go @@ -36,9 +36,9 @@ func Test_Translator(t *testing.T) { tr := m.Translator() cases := [][]string{ - []string{"ssh://gh/o/r", "ssh://github.com/o/r"}, - []string{"ssh://github.com/o/r", "ssh://github.com/o/r"}, - []string{"https://gh/o/r", "https://gh/o/r"}, + {"ssh://gh/o/r", "ssh://github.com/o/r"}, + {"ssh://github.com/o/r", "ssh://github.com/o/r"}, + {"https://gh/o/r", "https://gh/o/r"}, } for _, c := range cases { u, _ := url.Parse(c[0]) diff --git a/go.mod b/go.mod index 7f1484a03..6857e1947 100644 --- a/go.mod +++ b/go.mod @@ -4,6 +4,7 @@ go 1.13 require ( github.com/AlecAivazis/survey/v2 v2.0.7 + github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.11.1 github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058 github.com/dlclark/regexp2 v1.2.0 // indirect diff --git a/go.sum b/go.sum index d0d7585bf..42f30698a 100644 --- a/go.sum +++ b/go.sum @@ -3,6 +3,8 @@ github.com/AlecAivazis/survey/v2 v2.0.7 h1:+f825XHLse/hWd2tE/V5df04WFGimk34Eyg/z github.com/AlecAivazis/survey/v2 v2.0.7/go.mod h1:mlizQTaPjnR4jcpwRSaSlkbsRfYFEyKgLQvYTzxxiHA= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= +github.com/MakeNowJust/heredoc v1.0.0 h1:cXCdzVdstXyiTqTvfqk9SDHpKNjxuom+DOlyEeQ4pzQ= +github.com/MakeNowJust/heredoc v1.0.0/go.mod h1:mG5amYoWBHf8vpLOuehzbGGw0EHxpZZ6lCpQ4fNJ8LE= github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8 h1:xzYJEypr/85nBpB11F9br+3HUrpgb+fcm5iADzXXYEw= github.com/Netflix/go-expect v0.0.0-20180615182759-c93bf25de8e8/go.mod h1:oX5x61PbNXchhh0oikYAH+4Pcfw5LKv21+Jnpr6r6Pc= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index edc3a6ae9..d19130bb5 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -7,6 +7,7 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" "gopkg.in/yaml.v3" ) @@ -96,16 +97,16 @@ github.com: defer StubBackupConfig()() _, err := ParseConfig("config.yml") - eq(t, err, nil) + assert.Nil(t, err) - expectedMain := "" + expectedMain := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" expectedHosts := `github.com: user: keiyuri oauth_token: "123456" ` - eq(t, mainBuf.String(), expectedMain) - eq(t, hostsBuf.String(), expectedHosts) + assert.Equal(t, expectedMain, mainBuf.String()) + assert.Equal(t, expectedHosts, hostsBuf.String()) } func Test_parseConfigFile(t *testing.T) { diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 8db26f965..a57d21dec 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -124,8 +124,50 @@ func NewConfig(root *yaml.Node) Config { func NewBlankConfig() Config { return NewConfig(&yaml.Node{ - Kind: yaml.DocumentNode, - Content: []*yaml.Node{{Kind: yaml.MappingNode}}, + Kind: yaml.DocumentNode, + Content: []*yaml.Node{ + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + HeadComment: "What protocol to use when performing git operations. Supported values: ssh, https", + Kind: yaml.ScalarNode, + Value: "git_protocol", + }, + { + Kind: yaml.ScalarNode, + Value: "https", + }, + { + HeadComment: "What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.", + Kind: yaml.ScalarNode, + Value: "editor", + }, + { + Kind: yaml.ScalarNode, + Value: "", + }, + { + HeadComment: "Aliases allow you to create nicknames for gh commands", + Kind: yaml.ScalarNode, + Value: "aliases", + }, + { + Kind: yaml.MappingNode, + Content: []*yaml.Node{ + { + Kind: yaml.ScalarNode, + Value: "co", + }, + { + Kind: yaml.ScalarNode, + Value: "pr checkout", + }, + }, + }, + }, + }, + }, }) } diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 1c8105186..df33effa3 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -19,7 +19,8 @@ func Test_fileConfig_Set(t *testing.T) { assert.NoError(t, c.Set("github.com", "user", "hubot")) assert.NoError(t, c.Write()) - assert.Equal(t, "editor: nano\n", mainBuf.String()) + expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor: nano\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" + assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, `github.com: git_protocol: ssh user: hubot @@ -28,14 +29,29 @@ example.com: `, hostsBuf.String()) } -func Test_fileConfig_Write(t *testing.T) { +func Test_defaultConfig(t *testing.T) { mainBuf := bytes.Buffer{} hostsBuf := bytes.Buffer{} defer StubWriteConfig(&mainBuf, &hostsBuf)() - c := NewBlankConfig() - assert.NoError(t, c.Write()) + cfg := NewBlankConfig() + assert.NoError(t, cfg.Write()) - assert.Equal(t, "", mainBuf.String()) + expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" + assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, "", hostsBuf.String()) + + proto, err := cfg.Get("", "git_protocol") + assert.Nil(t, err) + assert.Equal(t, "https", proto) + + editor, err := cfg.Get("", "editor") + assert.Nil(t, err) + assert.Equal(t, "", editor) + + aliases, err := cfg.Aliases() + assert.Nil(t, err) + assert.Equal(t, len(aliases.All()), 1) + expansion, _ := aliases.Get("co") + assert.Equal(t, expansion, "pr checkout") } diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 4158ff8c1..a5540cf2d 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -1,16 +1,23 @@ package api import ( + "bytes" + "encoding/json" "fmt" "io" "io/ioutil" "net/http" "os" + "regexp" + "sort" "strconv" "strings" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/jsoncolor" "github.com/spf13/cobra" ) @@ -20,18 +27,21 @@ type ApiOptions struct { RequestMethod string RequestMethodPassed bool RequestPath string + RequestInputFile string MagicFields []string RawFields []string RequestHeaders []string ShowResponseHeaders bool HttpClient func() (*http.Client, error) + BaseRepo func() (ghrepo.Interface, error) } func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command { opts := ApiOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + BaseRepo: f.BaseRepo, } cmd := &cobra.Command{ @@ -39,13 +49,16 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command Short: "Make an authenticated GitHub API request", Long: `Makes an authenticated HTTP request to the GitHub API and prints the response. -The argument should either be a path of a GitHub API v3 endpoint, or +The endpoint argument should either be a path of a GitHub API v3 endpoint, or "graphql" to access the GitHub API v4. +Placeholder values ":owner" and ":repo" in the endpoint argument will get replaced +with values from the repository of the current directory. + The default HTTP request method is "GET" normally and "POST" if any parameters were added. Override the method with '--method'. -Pass one or more '--raw-field' values in "=" format to add +Pass one or more '--raw-field' values in "key=value" format to add JSON-encoded string parameters to the POST body. The '--field' flag behaves like '--raw-field' with magic type conversion based @@ -53,9 +66,28 @@ on the format of the value: - literal values "true", "false", "null", and integer numbers get converted to appropriate JSON types; +- placeholder values ":owner" and ":repo" get populated with values from the + repository of the current directory; - if the value starts with "@", the rest of the value is interpreted as a filename to read the value from. Pass "-" to read from standard input. + +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. `, + Example: heredoc.Doc(` + $ gh api repos/:owner/:repo/releases + + $ gh api graphql -F owner=':owner' -F name=':repo' -f query=' + query($name: String!, $owner: String!) { + repository(owner: $owner, name: $name) { + releases(last: 3) { + nodes { tagName } + } + } + } + ' + `), Args: cobra.ExactArgs(1), RunE: func(c *cobra.Command, args []string) error { opts.RequestPath = args[0] @@ -73,6 +105,7 @@ on the format of the value: 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().StringVar(&opts.RequestInputFile, "input", "", "The file to use as body for the HTTP request") return cmd } @@ -82,46 +115,133 @@ func apiRun(opts *ApiOptions) error { return err } + requestPath, err := fillPlaceholders(opts.RequestPath, opts) + if err != nil { + return fmt.Errorf("unable to expand placeholder in path: %w", err) + } method := opts.RequestMethod - if len(params) > 0 && !opts.RequestMethodPassed { + requestHeaders := opts.RequestHeaders + var requestBody interface{} = params + + if !opts.RequestMethodPassed && (len(params) > 0 || opts.RequestInputFile != "") { method = "POST" } + if opts.RequestInputFile != "" { + file, size, err := openUserFile(opts.RequestInputFile, opts.IO.In) + if err != nil { + return err + } + defer file.Close() + requestPath = addQuery(requestPath, params) + requestBody = file + if size >= 0 { + requestHeaders = append([]string{fmt.Sprintf("Content-Length: %d", size)}, requestHeaders...) + } + } + httpClient, err := opts.HttpClient() if err != nil { return err } - resp, err := httpRequest(httpClient, method, opts.RequestPath, params, opts.RequestHeaders) + resp, err := httpRequest(httpClient, method, requestPath, requestBody, requestHeaders) if err != nil { return err } if opts.ShowResponseHeaders { - for name, vals := range resp.Header { - fmt.Fprintf(opts.IO.Out, "%s: %s\r\n", name, strings.Join(vals, ", ")) - } + fmt.Fprintln(opts.IO.Out, resp.Proto, resp.Status) + printHeaders(opts.IO.Out, resp.Header, opts.IO.ColorEnabled()) fmt.Fprint(opts.IO.Out, "\r\n") } if resp.StatusCode == 204 { return nil } + var responseBody io.Reader = resp.Body defer resp.Body.Close() - _, err = io.Copy(opts.IO.Out, resp.Body) - if err != nil { - return err + isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) + + var serverError string + if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { + responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode) + if err != nil { + return err + } } - // TODO: detect GraphQL errors - if resp.StatusCode > 299 { + if isJSON && opts.IO.ColorEnabled() { + err = jsoncolor.Write(opts.IO.Out, responseBody, " ") + if err != nil { + return err + } + } else { + _, err = io.Copy(opts.IO.Out, responseBody) + if err != nil { + return err + } + } + + if serverError != "" { + fmt.Fprintf(opts.IO.ErrOut, "gh: %s\n", serverError) + return cmdutil.SilentError + } else if resp.StatusCode > 299 { + fmt.Fprintf(opts.IO.ErrOut, "gh: HTTP %d\n", resp.StatusCode) return cmdutil.SilentError } return nil } +var placeholderRE = regexp.MustCompile(`\:(owner|repo)\b`) + +// fillPlaceholders populates `:owner` and `:repo` placeholders with values from the current repository +func fillPlaceholders(value string, opts *ApiOptions) (string, error) { + if !placeholderRE.MatchString(value) { + return value, nil + } + + baseRepo, err := opts.BaseRepo() + if err != nil { + return value, err + } + + value = placeholderRE.ReplaceAllStringFunc(value, func(m string) string { + switch m { + case ":owner": + return baseRepo.RepoOwner() + case ":repo": + return baseRepo.RepoName() + default: + panic(fmt.Sprintf("invalid placeholder: %q", m)) + } + }) + + return value, nil +} + +func printHeaders(w io.Writer, headers http.Header, colorize bool) { + var names []string + for name := range headers { + if name == "Status" { + continue + } + names = append(names, name) + } + sort.Strings(names) + + var headerColor, headerColorReset string + if colorize { + headerColor = "\x1b[1;34m" // bright blue + headerColorReset = "\x1b[m" + } + for _, name := range names { + fmt.Fprintf(w, "%s%s%s: %s\r\n", headerColor, name, headerColorReset, strings.Join(headers[name], ", ")) + } +} + func parseFields(opts *ApiOptions) (map[string]interface{}, error) { params := make(map[string]interface{}) for _, f := range opts.RawFields { @@ -136,7 +256,7 @@ func parseFields(opts *ApiOptions) (map[string]interface{}, error) { if err != nil { return params, err } - value, err := magicFieldValue(strValue, opts.IO.In) + value, err := magicFieldValue(strValue, opts) if err != nil { return params, fmt.Errorf("error parsing %q value: %w", key, err) } @@ -153,9 +273,9 @@ func parseField(f string) (string, string, error) { return f[0:idx], f[idx+1:], nil } -func magicFieldValue(v string, stdin io.ReadCloser) (interface{}, error) { +func magicFieldValue(v string, opts *ApiOptions) (interface{}, error) { if strings.HasPrefix(v, "@") { - return readUserFile(v[1:], stdin) + return readUserFile(v[1:], opts.IO.In) } if n, err := strconv.Atoi(v); err == nil { @@ -170,7 +290,7 @@ func magicFieldValue(v string, stdin io.ReadCloser) (interface{}, error) { case "null": return nil, nil default: - return v, nil + return fillPlaceholders(v, opts) } } @@ -188,3 +308,52 @@ func readUserFile(fn string, stdin io.ReadCloser) ([]byte, error) { defer r.Close() return ioutil.ReadAll(r) } + +func openUserFile(fn string, stdin io.ReadCloser) (io.ReadCloser, int64, error) { + if fn == "-" { + return stdin, -1, nil + } + + r, err := os.Open(fn) + if err != nil { + return r, -1, err + } + + s, err := os.Stat(fn) + if err != nil { + return r, -1, err + } + + return r, s.Size(), nil +} + +func parseErrorResponse(r io.Reader, statusCode int) (io.Reader, string, error) { + bodyCopy := &bytes.Buffer{} + b, err := ioutil.ReadAll(io.TeeReader(r, bodyCopy)) + if err != nil { + return r, "", err + } + + var parsedBody struct { + Message string + Errors []struct { + Message string + } + } + err = json.Unmarshal(b, &parsedBody) + if err != nil { + return r, "", err + } + + if parsedBody.Message != "" { + return bodyCopy, fmt.Sprintf("%s (HTTP %d)", parsedBody.Message, statusCode), nil + } else if len(parsedBody.Errors) > 0 { + msgs := make([]string, len(parsedBody.Errors)) + for i, e := range parsedBody.Errors { + msgs[i] = e.Message + } + return bodyCopy, strings.Join(msgs, "\n"), nil + } + + return bodyCopy, "", nil +} diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 8149f5906..605e4bf9e 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -3,12 +3,12 @@ package api import ( "bytes" "fmt" - "io" "io/ioutil" "net/http" "os" "testing" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" @@ -31,6 +31,7 @@ func Test_NewCmdApi(t *testing.T) { RequestMethod: "GET", RequestMethodPassed: false, RequestPath: "graphql", + RequestInputFile: "", RawFields: []string(nil), MagicFields: []string(nil), RequestHeaders: []string(nil), @@ -45,6 +46,7 @@ func Test_NewCmdApi(t *testing.T) { RequestMethod: "DELETE", RequestMethodPassed: true, RequestPath: "repos/octocat/Spoon-Knife", + RequestInputFile: "", RawFields: []string(nil), MagicFields: []string(nil), RequestHeaders: []string(nil), @@ -59,6 +61,7 @@ func Test_NewCmdApi(t *testing.T) { RequestMethod: "GET", RequestMethodPassed: false, RequestPath: "graphql", + RequestInputFile: "", RawFields: []string{"query=QUERY"}, MagicFields: []string{"body=@file.txt"}, RequestHeaders: []string(nil), @@ -73,6 +76,7 @@ func Test_NewCmdApi(t *testing.T) { RequestMethod: "GET", RequestMethodPassed: false, RequestPath: "user", + RequestInputFile: "", RawFields: []string(nil), MagicFields: []string(nil), RequestHeaders: []string{"accept: text/plain"}, @@ -80,6 +84,21 @@ func Test_NewCmdApi(t *testing.T) { }, wantsErr: false, }, + { + name: "with request body from file", + cli: "user --input myfile", + wants: ApiOptions{ + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "user", + RequestInputFile: "myfile", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + }, + wantsErr: false, + }, { name: "no arguments", cli: "", @@ -92,6 +111,7 @@ func Test_NewCmdApi(t *testing.T) { assert.Equal(t, tt.wants.RequestMethod, o.RequestMethod) assert.Equal(t, tt.wants.RequestMethodPassed, o.RequestMethodPassed) assert.Equal(t, tt.wants.RequestPath, o.RequestPath) + assert.Equal(t, tt.wants.RequestInputFile, o.RequestInputFile) assert.Equal(t, tt.wants.RawFields, o.RawFields) assert.Equal(t, tt.wants.MagicFields, o.MagicFields) assert.Equal(t, tt.wants.RequestHeaders, o.RequestHeaders) @@ -140,12 +160,14 @@ func Test_apiRun(t *testing.T) { ShowResponseHeaders: true, }, httpResponse: &http.Response{ + Proto: "HTTP/1.1", + Status: "200 Okey-dokey", StatusCode: 200, Body: ioutil.NopCloser(bytes.NewBufferString(`body`)), Header: http.Header{"Content-Type": []string{"text/plain"}}, }, err: nil, - stdout: "Content-Type: text/plain\r\n\r\nbody", + stdout: "HTTP/1.1 200 Okey-dokey\nContent-Type: text/plain\r\n\r\nbody", stderr: ``, }, { @@ -158,6 +180,31 @@ func Test_apiRun(t *testing.T) { stdout: ``, stderr: ``, }, + { + name: "REST error", + httpResponse: &http.Response{ + StatusCode: 400, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"message": "THIS IS FINE"}`)), + Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, + }, + err: cmdutil.SilentError, + stdout: `{"message": "THIS IS FINE"}`, + stderr: "gh: THIS IS FINE (HTTP 400)\n", + }, + { + name: "GraphQL error", + options: ApiOptions{ + RequestPath: "graphql", + }, + httpResponse: &http.Response{ + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(`{"errors": [{"message":"AGAIN"}, {"message":"FINE"}]}`)), + Header: http.Header{"Content-Type": []string{"application/json; charset=utf-8"}}, + }, + err: cmdutil.SilentError, + stdout: `{"errors": [{"message":"AGAIN"}, {"message":"FINE"}]}`, + stderr: "gh: AGAIN\nFINE\n", + }, { name: "failure", httpResponse: &http.Response{ @@ -166,7 +213,7 @@ func Test_apiRun(t *testing.T) { }, err: cmdutil.SilentError, stdout: `gateway timeout`, - stderr: ``, + stderr: "gh: HTTP 502\n", }, } @@ -199,6 +246,81 @@ func Test_apiRun(t *testing.T) { } } +func Test_apiRun_inputFile(t *testing.T) { + tests := []struct { + name string + inputFile string + inputContents []byte + + contentLength int64 + expectedContents []byte + }{ + { + name: "stdin", + inputFile: "-", + inputContents: []byte("I WORK OUT"), + contentLength: 0, + }, + { + name: "from file", + inputFile: "gh-test-file", + inputContents: []byte("I WORK OUT"), + contentLength: 10, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, stdin, _, _ := iostreams.Test() + resp := &http.Response{StatusCode: 204} + + inputFile := tt.inputFile + if tt.inputFile == "-" { + _, _ = stdin.Write(tt.inputContents) + } else { + f, err := ioutil.TempFile("", tt.inputFile) + if err != nil { + t.Fatal(err) + } + _, _ = f.Write(tt.inputContents) + f.Close() + t.Cleanup(func() { os.Remove(f.Name()) }) + inputFile = f.Name() + } + + var bodyBytes []byte + options := ApiOptions{ + RequestPath: "hello", + RequestInputFile: inputFile, + RawFields: []string{"a=b", "c=d"}, + + IO: io, + HttpClient: func() (*http.Client, error) { + var tr roundTripper = func(req *http.Request) (*http.Response, error) { + var err error + if bodyBytes, err = ioutil.ReadAll(req.Body); err != nil { + return nil, err + } + resp.Request = req + return resp, nil + } + return &http.Client{Transport: tr}, nil + }, + } + + err := apiRun(&options) + if err != nil { + t.Errorf("got error %v", err) + } + + assert.Equal(t, "POST", resp.Request.Method) + assert.Equal(t, "/hello?a=b&c=d", resp.Request.URL.RequestURI()) + assert.Equal(t, tt.contentLength, resp.Request.ContentLength) + assert.Equal(t, "", resp.Request.Header.Get("Content-Type")) + assert.Equal(t, tt.inputContents, bodyBytes) + }) + } +} + func Test_parseFields(t *testing.T) { io, stdin, _, _ := iostreams.Test() fmt.Fprint(stdin, "pasted contents") @@ -244,9 +366,11 @@ func Test_magicFieldValue(t *testing.T) { f.Close() t.Cleanup(func() { os.Remove(f.Name()) }) + io, _, _, _ := iostreams.Test() + type args struct { - v string - stdin io.ReadCloser + v string + opts *ApiOptions } tests := []struct { name string @@ -279,21 +403,41 @@ func Test_magicFieldValue(t *testing.T) { wantErr: false, }, { - name: "file", - args: args{v: "@" + f.Name()}, + name: "placeholder", + args: args{ + v: ":owner", + opts: &ApiOptions{ + IO: io, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "hubot", + wantErr: false, + }, + { + name: "file", + args: args{ + v: "@" + f.Name(), + opts: &ApiOptions{IO: io}, + }, want: []byte("file contents"), wantErr: false, }, { - name: "file error", - args: args{v: "@"}, + name: "file error", + args: args{ + v: "@", + opts: &ApiOptions{IO: io}, + }, want: nil, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := magicFieldValue(tt.args.v, tt.args.stdin) + got, err := magicFieldValue(tt.args.v, tt.args.opts) if (err != nil) != tt.wantErr { t.Errorf("magicFieldValue() error = %v, wantErr %v", err, tt.wantErr) return @@ -305,3 +449,88 @@ func Test_magicFieldValue(t *testing.T) { }) } } + +func Test_openUserFile(t *testing.T) { + f, err := ioutil.TempFile("", "gh-test") + if err != nil { + t.Fatal(err) + } + fmt.Fprint(f, "file contents") + f.Close() + t.Cleanup(func() { os.Remove(f.Name()) }) + + file, length, err := openUserFile(f.Name(), nil) + if err != nil { + t.Fatal(err) + } + defer file.Close() + + fb, err := ioutil.ReadAll(file) + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, int64(13), length) + assert.Equal(t, "file contents", string(fb)) +} + +func Test_fillPlaceholders(t *testing.T) { + type args struct { + value string + opts *ApiOptions + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "no changes", + args: args{ + value: "repos/owner/repo/releases", + opts: &ApiOptions{ + BaseRepo: nil, + }, + }, + want: "repos/owner/repo/releases", + wantErr: false, + }, + { + name: "has substitutes", + args: args{ + value: "repos/:owner/:repo/releases", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "repos/hubot/robot-uprising/releases", + wantErr: false, + }, + { + name: "no greedy substitutes", + args: args{ + value: ":ownership/:repository", + opts: &ApiOptions{ + BaseRepo: nil, + }, + }, + want: ":ownership/:repository", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := fillPlaceholders(tt.args.value, tt.args.opts) + if (err != nil) != tt.wantErr { + t.Errorf("fillPlaceholders() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("fillPlaceholders() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/cmd/api/http.go b/pkg/cmd/api/http.go index 812b95af4..e393bb593 100644 --- a/pkg/cmd/api/http.go +++ b/pkg/cmd/api/http.go @@ -7,12 +7,19 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" ) func httpRequest(client *http.Client, method string, p string, params interface{}, headers []string) (*http.Response, error) { + var requestURL string // TODO: GHE support - url := "https://api.github.com/" + p + if strings.Contains(p, "://") { + requestURL = p + } else { + requestURL = "https://api.github.com/" + p + } + var body io.Reader var bodyIsJSON bool isGraphQL := p == "graphql" @@ -20,7 +27,7 @@ func httpRequest(client *http.Client, method string, p string, params interface{ switch pp := params.(type) { case map[string]interface{}: if strings.EqualFold(method, "GET") { - url = addQuery(url, pp) + requestURL = addQuery(requestURL, pp) } else { for key, value := range pp { switch vv := value.(type) { @@ -46,7 +53,7 @@ func httpRequest(client *http.Client, method string, p string, params interface{ return nil, fmt.Errorf("unrecognized parameters type: %v", params) } - req, err := http.NewRequest(method, url, body) + req, err := http.NewRequest(method, requestURL, body) if err != nil { return nil, err } @@ -56,7 +63,16 @@ func httpRequest(client *http.Client, method string, p string, params interface{ if idx == -1 { return nil, fmt.Errorf("header %q requires a value separated by ':'", h) } - req.Header.Add(h[0:idx], strings.TrimSpace(h[idx+1:])) + name, value := h[0:idx], strings.TrimSpace(h[idx+1:]) + if strings.EqualFold(name, "Content-Length") { + length, err := strconv.ParseInt(value, 10, 0) + if err != nil { + return nil, err + } + req.ContentLength = length + } else { + req.Header.Add(name, value) + } } if bodyIsJSON && req.Header.Get("Content-Type") == "" { req.Header.Set("Content-Type", "application/json; charset=utf-8") diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index 578b29561..ad7162415 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -3,10 +3,12 @@ package cmdutil import ( "net/http" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/iostreams" ) type Factory struct { IOStreams *iostreams.IOStreams HttpClient func() (*http.Client, error) + BaseRepo func() (ghrepo.Interface, error) } diff --git a/pkg/githubtemplate/github_template.go b/pkg/githubtemplate/github_template.go index 89cceda4c..e14ad04c8 100644 --- a/pkg/githubtemplate/github_template.go +++ b/pkg/githubtemplate/github_template.go @@ -10,8 +10,8 @@ import ( "gopkg.in/yaml.v3" ) -// Find returns the list of template file paths -func Find(rootDir string, name string) []string { +// FindNonLegacy returns the list of template file paths from the template folder (according to the "upgraded multiple template builder") +func FindNonLegacy(rootDir string, name string) []string { results := []string{} // https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository @@ -46,21 +46,34 @@ mainLoop: break } } + } + sort.Strings(results) + return results +} + +// FindLegacy returns the file path of the default(legacy) template +func FindLegacy(rootDir string, name string) *string { + // https://help.github.com/en/github/building-a-strong-community/creating-a-pull-request-template-for-your-repository + candidateDirs := []string{ + path.Join(rootDir, ".github"), + rootDir, + path.Join(rootDir, "docs"), + } + for _, dir := range candidateDirs { + files, err := ioutil.ReadDir(dir) + if err != nil { + continue + } // detect a single template file for _, file := range files { if strings.EqualFold(file.Name(), name+".md") { - results = append(results, path.Join(dir, file.Name())) - break + result := path.Join(dir, file.Name()) + return &result } } - if len(results) > 0 { - break - } } - - sort.Strings(results) - return results + return nil } // ExtractName returns the name of the template from YAML front-matter diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index 839e215b3..ee5dfb625 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -8,7 +8,7 @@ import ( "testing" ) -func TestFind(t *testing.T) { +func TestFindNonLegacy(t *testing.T) { tmpdir, err := ioutil.TempDir("", "gh-cli") if err != nil { t.Fatal(err) @@ -25,52 +25,69 @@ func TestFind(t *testing.T) { want []string }{ { - name: "Template in root", + name: "Legacy templates ignored", prepare: []string{ "README.md", "ISSUE_TEMPLATE", "issue_template.md", "issue_template.txt", "pull_request_template.md", - }, - args: args{ - rootDir: tmpdir, - name: "ISSUE_TEMPLATE", - }, - want: []string{ - path.Join(tmpdir, "issue_template.md"), - }, - }, - { - name: "Template in .github takes precedence", - prepare: []string{ - "ISSUE_TEMPLATE.md", ".github/issue_template.md", - }, - args: args{ - rootDir: tmpdir, - name: "ISSUE_TEMPLATE", - }, - want: []string{ - path.Join(tmpdir, ".github/issue_template.md"), - }, - }, - { - name: "Template in docs", - prepare: []string{ - "README.md", "docs/issue_template.md", }, args: args{ rootDir: tmpdir, name: "ISSUE_TEMPLATE", }, + want: []string{}, + }, + { + name: "Template folder in .github takes precedence", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + "ISSUE_TEMPLATE/abc.md", + ".github/ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, want: []string{ - path.Join(tmpdir, "docs/issue_template.md"), + path.Join(tmpdir, ".github/ISSUE_TEMPLATE/abc.md"), }, }, { - name: "Multiple templates", + name: "Template folder in root", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + "ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: []string{ + path.Join(tmpdir, "ISSUE_TEMPLATE/abc.md"), + }, + }, + { + name: "Template folder in docs", + prepare: []string{ + "ISSUE_TEMPLATE.md", + "docs/ISSUE_TEMPLATE/abc.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: []string{ + path.Join(tmpdir, "docs/ISSUE_TEMPLATE/abc.md"), + }, + }, + { + name: "Multiple templates in template folder", prepare: []string{ ".github/ISSUE_TEMPLATE/nope.md", ".github/PULL_REQUEST_TEMPLATE.md", @@ -90,18 +107,17 @@ func TestFind(t *testing.T) { }, }, { - name: "Empty multiple templates directory", + name: "Empty template directories", prepare: []string{ - ".github/issue_template.md", - ".github/issue_template/.keep", + ".github/ISSUE_TEMPLATE/.keep", + ".docs/ISSUE_TEMPLATE/.keep", + "ISSUE_TEMPLATE/.keep", }, args: args{ rootDir: tmpdir, name: "ISSUE_TEMPLATE", }, - want: []string{ - path.Join(tmpdir, ".github/issue_template.md"), - }, + want: []string{}, }, } for _, tt := range tests { @@ -116,7 +132,99 @@ func TestFind(t *testing.T) { file.Close() } - if got := Find(tt.args.rootDir, tt.args.name); !reflect.DeepEqual(got, tt.want) { + if got := FindNonLegacy(tt.args.rootDir, tt.args.name); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Find() = %v, want %v", got, tt.want) + } + }) + os.RemoveAll(tmpdir) + } +} + +func TestFindLegacy(t *testing.T) { + tmpdir, err := ioutil.TempDir("", "gh-cli") + if err != nil { + t.Fatal(err) + } + + type args struct { + rootDir string + name string + } + tests := []struct { + name string + prepare []string + args args + want string + }{ + { + name: "Template in root", + prepare: []string{ + "README.md", + "ISSUE_TEMPLATE", + "issue_template.md", + "issue_template.txt", + "pull_request_template.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, "issue_template.md"), + }, + { + name: "Template in .github takes precedence", + prepare: []string{ + "ISSUE_TEMPLATE.md", + ".github/issue_template.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, ".github/issue_template.md"), + }, + { + name: "Template in docs", + prepare: []string{ + "README.md", + "docs/issue_template.md", + }, + args: args{ + rootDir: tmpdir, + name: "ISSUE_TEMPLATE", + }, + want: path.Join(tmpdir, "docs/issue_template.md"), + }, + { + name: "Non legacy templates ignored", + prepare: []string{ + ".github/PULL_REQUEST_TEMPLATE/abc.md", + "PULL_REQUEST_TEMPLATE/abc.md", + "docs/PULL_REQUEST_TEMPLATE/abc.md", + ".github/PULL_REQUEST_TEMPLATE.md", + }, + args: args{ + rootDir: tmpdir, + name: "PuLl_ReQuEsT_TeMpLaTe", + }, + want: path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, p := range tt.prepare { + fp := path.Join(tmpdir, p) + _ = os.MkdirAll(path.Dir(fp), 0700) + file, err := os.Create(fp) + if err != nil { + t.Fatal(err) + } + file.Close() + } + + if got := FindLegacy(tt.args.rootDir, tt.args.name); *got != tt.want { t.Errorf("Find() = %v, want %v", got, tt.want) } }) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 028b11264..8fd213e9f 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -5,19 +5,36 @@ import ( "io" "io/ioutil" "os" + + "github.com/mattn/go-colorable" + "github.com/mattn/go-isatty" ) type IOStreams struct { In io.ReadCloser Out io.Writer ErrOut io.Writer + + colorEnabled bool +} + +func (s *IOStreams) ColorEnabled() bool { + return s.colorEnabled } func System() *IOStreams { + var out io.Writer = os.Stdout + var colorEnabled bool + if os.Getenv("NO_COLOR") == "" && isTerminal(os.Stdout) { + out = colorable.NewColorable(os.Stdout) + colorEnabled = true + } + return &IOStreams{ - In: os.Stdin, - Out: os.Stdout, - ErrOut: os.Stderr, + In: os.Stdin, + Out: out, + ErrOut: os.Stderr, + colorEnabled: colorEnabled, } } @@ -31,3 +48,7 @@ func Test() (*IOStreams, *bytes.Buffer, *bytes.Buffer, *bytes.Buffer) { ErrOut: errOut, }, in, out, errOut } + +func isTerminal(f *os.File) bool { + return isatty.IsTerminal(f.Fd()) || isatty.IsCygwinTerminal(f.Fd()) +} diff --git a/pkg/jsoncolor/jsoncolor.go b/pkg/jsoncolor/jsoncolor.go new file mode 100644 index 000000000..cf3bd064c --- /dev/null +++ b/pkg/jsoncolor/jsoncolor.go @@ -0,0 +1,96 @@ +package jsoncolor + +import ( + "encoding/json" + "fmt" + "io" + "strings" +) + +const ( + colorDelim = "1;38" // bright white + colorKey = "1;34" // bright blue + colorNull = "1;30" // gray + colorString = "32" // green + colorBool = "33" // yellow +) + +// Write colorized JSON output parsed from reader +func Write(w io.Writer, r io.Reader, indent string) error { + dec := json.NewDecoder(r) + dec.UseNumber() + + var idx int + var stack []json.Delim + + for { + t, err := dec.Token() + if err == io.EOF { + break + } + if err != nil { + return err + } + + switch tt := t.(type) { + case json.Delim: + switch tt { + case '{', '[': + stack = append(stack, tt) + idx = 0 + fmt.Fprintf(w, "\x1b[%sm%s\x1b[m", colorDelim, tt) + if dec.More() { + fmt.Fprint(w, "\n", strings.Repeat(indent, len(stack))) + } + continue + case '}', ']': + stack = stack[:len(stack)-1] + idx = 0 + fmt.Fprintf(w, "\x1b[%sm%s\x1b[m", colorDelim, tt) + } + default: + b, err := json.Marshal(tt) + if err != nil { + return err + } + + isKey := len(stack) > 0 && stack[len(stack)-1] == '{' && idx%2 == 0 + idx++ + + var color string + if isKey { + color = colorKey + } else if tt == nil { + color = colorNull + } else { + switch t.(type) { + case string: + color = colorString + case bool: + color = colorBool + } + } + + if color == "" { + _, _ = w.Write(b) + } else { + fmt.Fprintf(w, "\x1b[%sm%s\x1b[m", color, b) + } + + if isKey { + fmt.Fprintf(w, "\x1b[%sm:\x1b[m ", colorDelim) + continue + } + } + + if dec.More() { + fmt.Fprintf(w, "\x1b[%sm,\x1b[m\n%s", colorDelim, strings.Repeat(indent, len(stack))) + } else if len(stack) > 0 { + fmt.Fprint(w, "\n", strings.Repeat(indent, len(stack)-1)) + } else { + fmt.Fprint(w, "\n") + } + } + + return nil +} diff --git a/pkg/jsoncolor/jsoncolor_test.go b/pkg/jsoncolor/jsoncolor_test.go new file mode 100644 index 000000000..9e2eda343 --- /dev/null +++ b/pkg/jsoncolor/jsoncolor_test.go @@ -0,0 +1,83 @@ +package jsoncolor + +import ( + "bytes" + "io" + "testing" + + "github.com/google/go-cmp/cmp" +) + +func TestWrite(t *testing.T) { + type args struct { + r io.Reader + indent string + } + tests := []struct { + name string + args args + wantW string + wantErr bool + }{ + { + name: "blank", + args: args{ + r: bytes.NewBufferString(``), + indent: "", + }, + wantW: "", + wantErr: false, + }, + { + name: "empty object", + args: args{ + r: bytes.NewBufferString(`{}`), + indent: "", + }, + wantW: "\x1b[1;38m{\x1b[m\x1b[1;38m}\x1b[m\n", + wantErr: false, + }, + { + name: "nested object", + args: args{ + r: bytes.NewBufferString(`{"hash":{"a":1,"b":2},"array":[3,4]}`), + indent: "\t", + }, + wantW: "\x1b[1;38m{\x1b[m\n\t\x1b[1;34m\"hash\"\x1b[m\x1b[1;38m:\x1b[m " + + "\x1b[1;38m{\x1b[m\n\t\t\x1b[1;34m\"a\"\x1b[m\x1b[1;38m:\x1b[m 1\x1b[1;38m,\x1b[m\n\t\t\x1b[1;34m\"b\"\x1b[m\x1b[1;38m:\x1b[m 2\n\t\x1b[1;38m}\x1b[m\x1b[1;38m,\x1b[m" + + "\n\t\x1b[1;34m\"array\"\x1b[m\x1b[1;38m:\x1b[m \x1b[1;38m[\x1b[m\n\t\t3\x1b[1;38m,\x1b[m\n\t\t4\n\t\x1b[1;38m]\x1b[m\n\x1b[1;38m}\x1b[m\n", + wantErr: false, + }, + { + name: "string", + args: args{ + r: bytes.NewBufferString(`"foo"`), + indent: "", + }, + wantW: "\x1b[32m\"foo\"\x1b[m\n", + wantErr: false, + }, + { + name: "error", + args: args{ + r: bytes.NewBufferString(`{{`), + indent: "", + }, + wantW: "\x1b[1;38m{\x1b[m\n", + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + w := &bytes.Buffer{} + if err := Write(w, tt.args.r, tt.args.indent); (err != nil) != tt.wantErr { + t.Errorf("Write() error = %v, wantErr %v", err, tt.wantErr) + return + } + diff := cmp.Diff(tt.wantW, w.String()) + if diff != "" { + t.Error(diff) + } + }) + } +} diff --git a/utils/color.go b/utils/color.go index b2b2add21..4940fe26b 100644 --- a/utils/color.go +++ b/utils/color.go @@ -9,10 +9,20 @@ import ( "github.com/mgutz/ansi" ) -var _isColorEnabled = true -var _isStdoutTerminal = false -var checkedTerminal = false -var checkedNoColor = false +var ( + _isColorEnabled bool = true + _isStdoutTerminal, checkedTerminal, checkedNoColor bool + + // Outputs ANSI color if stdout is a tty + Magenta = makeColorFunc("magenta") + Cyan = makeColorFunc("cyan") + Red = makeColorFunc("red") + Yellow = makeColorFunc("yellow") + Blue = makeColorFunc("blue") + Green = makeColorFunc("green") + Gray = makeColorFunc("black+h") + Bold = makeColorFunc("default+b") +) func isStdoutTerminal() bool { if !checkedTerminal { @@ -49,27 +59,3 @@ func isColorEnabled() bool { } return _isColorEnabled } - -// Magenta outputs ANSI color if stdout is a tty -var Magenta = makeColorFunc("magenta") - -// Cyan outputs ANSI color if stdout is a tty -var Cyan = makeColorFunc("cyan") - -// Red outputs ANSI color if stdout is a tty -var Red = makeColorFunc("red") - -// Yellow outputs ANSI color if stdout is a tty -var Yellow = makeColorFunc("yellow") - -// Blue outputs ANSI color if stdout is a tty -var Blue = makeColorFunc("blue") - -// Green outputs ANSI color if stdout is a tty -var Green = makeColorFunc("green") - -// Gray outputs ANSI color if stdout is a tty -var Gray = makeColorFunc("black+h") - -// Bold outputs ANSI color if stdout is a tty -var Bold = makeColorFunc("default+b")