diff --git a/Makefile b/Makefile index ad16069ee..859461984 100644 --- a/Makefile +++ b/Makefile @@ -44,7 +44,6 @@ site-docs: site git -C site pull git -C site rm 'manual/gh*.md' 2>/dev/null || true go run ./cmd/gen-docs --website --doc-path site/manual - for f in site/manual/gh*.md; do sed -i.bak -e '/^### SEE ALSO/,$$d' "$$f"; done rm -f site/manual/*.bak git -C site add 'manual/gh*.md' git -C site commit -m 'update help docs' || true diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index 631185393..5698794e0 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -5,15 +5,14 @@ import ( "os" "strings" + "github.com/cli/cli/internal/docs" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/spf13/cobra/doc" "github.com/spf13/pflag" ) func main() { - var flagError pflag.ErrorHandling docCmd := pflag.NewFlagSet("", flagError) manPage := docCmd.BoolP("man-page", "", false, "Generate manual pages") @@ -39,6 +38,7 @@ func main() { io, _, _, _ := iostreams.Test() rootCmd := root.NewCmdRoot(&cmdutil.Factory{IOStreams: io}, "", "") + rootCmd.InitDefaultHelpCmd() err := os.MkdirAll(*dir, 0755) if err != nil { @@ -46,20 +46,20 @@ func main() { } if *website { - err = doc.GenMarkdownTreeCustom(rootCmd, *dir, filePrepender, linkHandler) + err = docs.GenMarkdownTreeCustom(rootCmd, *dir, filePrepender, linkHandler) if err != nil { fatal(err) } } if *manPage { - header := &doc.GenManHeader{ + header := &docs.GenManHeader{ Title: "gh", Section: "1", - Source: "", //source and manual are just put at the top of the manpage, before name - Manual: "", //if source is an empty string, it's set to "Auto generated by spf13/cobra" + Source: "", + Manual: "", } - err = doc.GenManTree(rootCmd, header, *dir) + err = docs.GenManTree(rootCmd, header, *dir) if err != nil { fatal(err) } diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 728024079..d4abd2f74 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -22,6 +22,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/update" "github.com/cli/cli/utils" + "github.com/mattn/go-colorable" "github.com/mgutz/ansi" "github.com/spf13/cobra" ) @@ -63,6 +64,12 @@ func main() { } } + // Enable running gh from Windows File Explorer's address bar. Without this, the user is told to stop and run from a + // terminal. With this, a user can clone a repo (or take other actions) directly from explorer. + if len(os.Args) > 1 && os.Args[1] != "" { + cobra.MousetrapHelpText = "" + } + rootCmd := root.NewCmdRoot(cmdFactory, buildVersion, buildDate) cfg, err := cmdFactory.Config() @@ -120,12 +127,14 @@ func main() { } } + cs := cmdFactory.IOStreams.ColorScheme() + authCheckEnabled := os.Getenv("GITHUB_TOKEN") == "" && os.Getenv("GITHUB_ENTERPRISE_TOKEN") == "" && cmd != nil && cmdutil.IsAuthCheckEnabled(cmd) if authCheckEnabled { if !cmdutil.CheckAuth(cfg) { - fmt.Fprintln(stderr, utils.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) fmt.Fprintln(stderr) fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") fmt.Fprintln(stderr, "You can also set the GITHUB_TOKEN environment variable, if preferred.") @@ -247,5 +256,5 @@ func basicClient(currentVersion string) (*api.Client, error) { func apiVerboseLog() api.ClientOption { logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") colorize := utils.IsTerminal(os.Stderr) - return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) + return api.VerboseLog(colorable.NewColorable(os.Stderr), logTraffic, colorize) } diff --git a/docs/install_linux.md b/docs/install_linux.md index c7a49d967..d5392ba47 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -81,9 +81,9 @@ Install and upgrade: 1. Download the `.rpm` file from the [releases page][]; 2. Install the downloaded file: `sudo zypper in gh_*_linux_amd64.rpm` -## Community-supported methods +## Unofficial, Community-supported methods -Our team does not directly maintain the following packages or repositories. They are unofficial and we are unable to provide support or guarantees for them. +The core GitHub CLI team does not maintain the following packages or repositories. They are unofficial and we are unable to provide support or guarantees for them. They are linked here as a convenience and their presence does not imply continued oversight from the CLI core team. Users who choose to use them do so at their own risk. ### Arch Linux @@ -101,6 +101,21 @@ Android users can install via Termux: pkg install gh ``` +### Gentoo + +Gentoo Linux users can install from the [main portage tree](https://packages.gentoo.org/packages/dev-util/github-cli): + +``` bash +emerge -av github-cli +``` + +Upgrading can be done by updating the portage tree and then requesting an upgrade: + +``` bash +emerge --sync +emerge -u github-cli +``` + ### Kiss Linux Kiss Linux users can install from the [community repos](https://github.com/kisslinux/community): @@ -117,6 +132,17 @@ Nix/NixOS users can install from [nixpkgs](https://search.nixos.org/packages?sho nix-env -iA nixos.gitAndTools.gh ``` +### Snaps + +Many Linux distro users can install using Snapd from the [Snap Store](https://snapcraft.io/gh) or the associated [repo](https://github.com/casperdcl/cli/tree/snap) + +```bash +sudo snap install --edge gh && snap connect gh:ssh-keys +``` +> Snaps are auto-updated every 6 hours. `Snapd` is required and is available on a wide range of Linux distros. +> Find out which distros have Snapd pre-installed and how to install it in the [Snapcraft Installation Docs](https://snapcraft.io/docs/installing-snapd) +> +> **Note:** `snap connect gh:ssh-keys` is needed for all authentication and SSH needs. [releases page]: https://github.com/cli/cli/releases/latest [arch linux repo]: https://www.archlinux.org/packages/community/x86_64/github-cli diff --git a/git/git.go b/git/git.go index aa08c4d6c..2c2d32e0e 100644 --- a/git/git.go +++ b/git/git.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "io" "net/url" "os" "os/exec" @@ -162,10 +163,10 @@ func CommitBody(sha string) (string, error) { } // Push publishes a git ref to a remote and sets up upstream configuration -func Push(remote string, ref string) error { +func Push(remote string, ref string, cmdOut, cmdErr io.Writer) error { pushCmd := GitCommand("push", "--set-upstream", remote, ref) - pushCmd.Stdout = os.Stdout - pushCmd.Stderr = os.Stderr + pushCmd.Stdout = cmdOut + pushCmd.Stderr = cmdErr return run.PrepareCmd(pushCmd).Run() } diff --git a/go.mod b/go.mod index fe0d39b7b..3e56a0170 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.11.1 github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684 + github.com/cpuguy83/go-md2man/v2 v2.0.0 github.com/enescakir/emoji v1.0.0 github.com/google/go-cmp v0.5.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index 1c804931f..896895d21 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -12,8 +12,7 @@ import ( "github.com/cli/cli/auth" "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/browser" - "github.com/cli/cli/utils" - "github.com/mattn/go-colorable" + "github.com/cli/cli/pkg/iostreams" ) var ( @@ -23,12 +22,13 @@ var ( oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) -func AuthFlowWithConfig(cfg config.Config, hostname, notice string, additionalScopes []string) (string, error) { +func AuthFlowWithConfig(cfg config.Config, IO *iostreams.IOStreams, hostname, notice string, additionalScopes []string) (string, error) { // TODO this probably shouldn't live in this package. It should probably be in a new package that // depends on both iostreams and config. - stderr := colorable.NewColorableStderr() + stderr := IO.ErrOut + cs := IO.ColorScheme() - token, userLogin, err := authFlow(hostname, stderr, notice, additionalScopes) + token, userLogin, err := authFlow(hostname, IO, notice, additionalScopes) if err != nil { return "", err } @@ -48,13 +48,16 @@ func AuthFlowWithConfig(cfg config.Config, hostname, notice string, additionalSc } fmt.Fprintf(stderr, "%s Authentication complete. %s to continue...\n", - utils.GreenCheck(), utils.Bold("Press Enter")) - _ = waitForEnter(os.Stdin) + cs.SuccessIcon(), cs.Bold("Press Enter")) + _ = waitForEnter(IO.In) return token, nil } -func authFlow(oauthHost string, w io.Writer, notice string, additionalScopes []string) (string, string, error) { +func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, additionalScopes []string) (string, string, error) { + w := IO.ErrOut + cs := IO.ColorScheme() + var verboseStream io.Writer if strings.Contains(os.Getenv("DEBUG"), "oauth") { verboseStream = w @@ -75,10 +78,10 @@ func authFlow(oauthHost string, w io.Writer, notice string, additionalScopes []s HTTPClient: http.DefaultClient, OpenInBrowser: func(url, code string) error { if code != "" { - fmt.Fprintf(w, "%s First copy your one-time code: %s\n", utils.Yellow("!"), utils.Bold(code)) + fmt.Fprintf(w, "%s First copy your one-time code: %s\n", cs.Yellow("!"), cs.Bold(code)) } - fmt.Fprintf(w, "- %s to open %s in your browser... ", utils.Bold("Press Enter"), oauthHost) - _ = waitForEnter(os.Stdin) + fmt.Fprintf(w, "- %s to open %s in your browser... ", cs.Bold("Press Enter"), oauthHost) + _ = waitForEnter(IO.In) browseCmd, err := browser.Command(url) if err != nil { @@ -86,7 +89,7 @@ func authFlow(oauthHost string, w io.Writer, notice string, additionalScopes []s } err = browseCmd.Run() if err != nil { - fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", utils.Red("!"), url) + fmt.Fprintf(w, "%s Failed opening a web browser at %s\n", cs.Red("!"), url) fmt.Fprintf(w, " %s\n", err) fmt.Fprint(w, " Please try entering the URL in your browser manually\n") } diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 1a179c41d..40533f211 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -46,9 +46,45 @@ func ConfigOptions() []ConfigOption { return configOptions } -var configValues = map[string][]string{ - "git_protocol": {"ssh", "https"}, - "prompt": {"enabled", "disabled"}, +func ValidateKey(key string) error { + for _, configKey := range configOptions { + if key == configKey.Key { + return nil + } + } + + return fmt.Errorf("invalid key") +} + +type InvalidValueError struct { + ValidValues []string +} + +func (e InvalidValueError) Error() string { + return "invalid value" +} + +func ValidateValue(key, value string) error { + var validValues []string + + for _, v := range configOptions { + if v.Key == key { + validValues = v.AllowedValues + break + } + } + + if validValues == nil { + return nil + } + + for _, v := range validValues { + if v == value { + return nil + } + } + + return &InvalidValueError{ValidValues: validValues} } // This interface describes interacting with some persistent configuration for gh. @@ -306,33 +342,7 @@ func (c *fileConfig) GetWithSource(hostname, key string) (string, string, error) return value, defaultSource, nil } -type InvalidValueError struct { - ValidValues []string -} - -func (e InvalidValueError) Error() string { - return "invalid value" -} - -func validateConfigEntry(key, value string) error { - validValues, found := configValues[key] - if !found { - return nil - } - - for _, v := range validValues { - if v == value { - return nil - } - } - - return &InvalidValueError{ValidValues: validValues} -} - func (c *fileConfig) Set(hostname, key, value string) error { - if err := validateConfigEntry(key, value); err != nil { - return err - } if hostname == "" { return c.SetStringValue(key, value) } else { diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 46b9016f4..47295230f 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -28,7 +28,6 @@ func Test_fileConfig_Set(t *testing.T) { example.com: editor: vim `, hostsBuf.String()) - assert.EqualError(t, c.Set("github.com", "git_protocol", "sshpps"), "invalid value") } func Test_defaultConfig(t *testing.T) { @@ -70,16 +69,33 @@ func Test_defaultConfig(t *testing.T) { assert.Equal(t, expansion, "pr checkout") } -func Test_validateConfigEntry(t *testing.T) { - err := validateConfigEntry("git_protocol", "sshpps") +func Test_ValidateValue(t *testing.T) { + err := ValidateValue("git_protocol", "sshpps") assert.EqualError(t, err, "invalid value") - err = validateConfigEntry("git_protocol", "ssh") + err = ValidateValue("git_protocol", "ssh") assert.Nil(t, err) - err = validateConfigEntry("editor", "vim") + err = ValidateValue("editor", "vim") assert.Nil(t, err) - err = validateConfigEntry("got", "123") + err = ValidateValue("got", "123") assert.Nil(t, err) } + +func Test_ValidateKey(t *testing.T) { + err := ValidateKey("invalid") + assert.EqualError(t, err, "invalid key") + + err = ValidateKey("git_protocol") + assert.NoError(t, err) + + err = ValidateKey("editor") + assert.NoError(t, err) + + err = ValidateKey("prompt") + assert.NoError(t, err) + + err = ValidateKey("pager") + assert.NoError(t, err) +} diff --git a/internal/config/stub.go b/internal/config/stub.go new file mode 100644 index 000000000..57761dac5 --- /dev/null +++ b/internal/config/stub.go @@ -0,0 +1,51 @@ +package config + +import ( + "errors" +) + +type ConfigStub map[string]string + +func genKey(host, key string) string { + if host != "" { + return host + ":" + key + } + return key +} + +func (c ConfigStub) Get(host, key string) (string, error) { + val, _, err := c.GetWithSource(host, key) + return val, err +} + +func (c ConfigStub) GetWithSource(host, key string) (string, string, error) { + if v, found := c[genKey(host, key)]; found { + return v, "(memory)", nil + } + return "", "", errors.New("not found") +} + +func (c ConfigStub) Set(host, key, value string) error { + c[genKey(host, key)] = value + return nil +} + +func (c ConfigStub) Aliases() (*AliasConfig, error) { + return nil, nil +} + +func (c ConfigStub) Hosts() ([]string, error) { + return nil, nil +} + +func (c ConfigStub) UnsetHost(hostname string) { +} + +func (c ConfigStub) CheckWriteable(host, key string) error { + return nil +} + +func (c ConfigStub) Write() error { + c["_written"] = "true" + return nil +} diff --git a/internal/docs/docs_test.go b/internal/docs/docs_test.go new file mode 100644 index 000000000..e6b15062e --- /dev/null +++ b/internal/docs/docs_test.go @@ -0,0 +1,91 @@ +package docs + +import ( + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func emptyRun(*cobra.Command, []string) {} + +func init() { + rootCmd.PersistentFlags().StringP("rootflag", "r", "two", "") + rootCmd.PersistentFlags().StringP("strtwo", "t", "two", "help message for parent flag strtwo") + + echoCmd.PersistentFlags().StringP("strone", "s", "one", "help message for flag strone") + echoCmd.PersistentFlags().BoolP("persistentbool", "p", false, "help message for flag persistentbool") + echoCmd.Flags().IntP("intone", "i", 123, "help message for flag intone") + echoCmd.Flags().BoolP("boolone", "b", true, "help message for flag boolone") + + timesCmd.PersistentFlags().StringP("strtwo", "t", "2", "help message for child flag strtwo") + timesCmd.Flags().IntP("inttwo", "j", 234, "help message for flag inttwo") + timesCmd.Flags().BoolP("booltwo", "c", false, "help message for flag booltwo") + + printCmd.PersistentFlags().StringP("strthree", "s", "three", "help message for flag strthree") + printCmd.Flags().IntP("intthree", "i", 345, "help message for flag intthree") + printCmd.Flags().BoolP("boolthree", "b", true, "help message for flag boolthree") + + echoCmd.AddCommand(timesCmd, echoSubCmd, deprecatedCmd) + rootCmd.AddCommand(printCmd, echoCmd, dummyCmd) +} + +var rootCmd = &cobra.Command{ + Use: "root", + Short: "Root short description", + Long: "Root long description", + Run: emptyRun, +} + +var echoCmd = &cobra.Command{ + Use: "echo [string to echo]", + Aliases: []string{"say"}, + Short: "Echo anything to the screen", + Long: "an utterly useless command for testing", + Example: "Just run cobra-test echo", +} + +var echoSubCmd = &cobra.Command{ + Use: "echosub [string to print]", + Short: "second sub command for echo", + Long: "an absolutely utterly useless command for testing gendocs!.", + Run: emptyRun, +} + +var timesCmd = &cobra.Command{ + Use: "times [# times] [string to echo]", + SuggestFor: []string{"counts"}, + Short: "Echo anything to the screen more times", + Long: `a slightly useless command for testing.`, + Run: emptyRun, +} + +var deprecatedCmd = &cobra.Command{ + Use: "deprecated [can't do anything here]", + Short: "A command which is deprecated", + Long: `an absolutely utterly useless command for testing deprecation!.`, + Deprecated: "Please use echo instead", +} + +var printCmd = &cobra.Command{ + Use: "print [string to print]", + Short: "Print anything to the screen", + Long: `an absolutely utterly useless command for testing.`, +} + +var dummyCmd = &cobra.Command{ + Use: "dummy [action]", + Short: "Performs a dummy action", +} + +func checkStringContains(t *testing.T, got, expected string) { + if !strings.Contains(got, expected) { + t.Errorf("Expected to contain: \n %v\nGot:\n %v\n", expected, got) + } +} + +func checkStringOmits(t *testing.T, got, expected string) { + if strings.Contains(got, expected) { + t.Errorf("Expected to not contain: \n %v\nGot: %v", expected, got) + } +} diff --git a/internal/docs/man.go b/internal/docs/man.go new file mode 100644 index 000000000..570e9c780 --- /dev/null +++ b/internal/docs/man.go @@ -0,0 +1,242 @@ +package docs + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "time" + + "github.com/cpuguy83/go-md2man/v2/md2man" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +// GenManTree will generate a man page for this command and all descendants +// in the directory given. The header may be nil. This function may not work +// correctly if your command names have `-` in them. If you have `cmd` with two +// subcmds, `sub` and `sub-third`, and `sub` has a subcommand called `third` +// it is undefined which help output will be in the file `cmd-sub-third.1`. +func GenManTree(cmd *cobra.Command, header *GenManHeader, dir string) error { + return GenManTreeFromOpts(cmd, GenManTreeOptions{ + Header: header, + Path: dir, + CommandSeparator: "-", + }) +} + +// GenManTreeFromOpts generates a man page for the command and all descendants. +// The pages are written to the opts.Path directory. +func GenManTreeFromOpts(cmd *cobra.Command, opts GenManTreeOptions) error { + header := opts.Header + if header == nil { + header = &GenManHeader{} + } + for _, c := range cmd.Commands() { + if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() { + continue + } + if err := GenManTreeFromOpts(c, opts); err != nil { + return err + } + } + section := "1" + if header.Section != "" { + section = header.Section + } + + separator := "_" + if opts.CommandSeparator != "" { + separator = opts.CommandSeparator + } + basename := strings.Replace(cmd.CommandPath(), " ", separator, -1) + filename := filepath.Join(opts.Path, basename+"."+section) + f, err := os.Create(filename) + if err != nil { + return err + } + defer f.Close() + + headerCopy := *header + return GenMan(cmd, &headerCopy, f) +} + +// GenManTreeOptions is the options for generating the man pages. +// Used only in GenManTreeFromOpts. +type GenManTreeOptions struct { + Header *GenManHeader + Path string + CommandSeparator string +} + +// GenManHeader is a lot like the .TH header at the start of man pages. These +// include the title, section, date, source, and manual. We will use the +// current time if Date is unset. +type GenManHeader struct { + Title string + Section string + Date *time.Time + date string + Source string + Manual string +} + +// GenMan will generate a man page for the given command and write it to +// w. The header argument may be nil, however obviously w may not. +func GenMan(cmd *cobra.Command, header *GenManHeader, w io.Writer) error { + if header == nil { + header = &GenManHeader{} + } + if err := fillHeader(header, cmd.CommandPath()); err != nil { + return err + } + + b := genMan(cmd, header) + _, err := w.Write(md2man.Render(b)) + return err +} + +func fillHeader(header *GenManHeader, name string) error { + if header.Title == "" { + header.Title = strings.ToUpper(strings.Replace(name, " ", "\\-", -1)) + } + if header.Section == "" { + header.Section = "1" + } + if header.Date == nil { + now := time.Now() + if epoch := os.Getenv("SOURCE_DATE_EPOCH"); epoch != "" { + unixEpoch, err := strconv.ParseInt(epoch, 10, 64) + if err != nil { + return fmt.Errorf("invalid SOURCE_DATE_EPOCH: %v", err) + } + now = time.Unix(unixEpoch, 0) + } + header.Date = &now + } + header.date = (*header.Date).Format("Jan 2006") + return nil +} + +func manPreamble(buf *bytes.Buffer, header *GenManHeader, cmd *cobra.Command, dashedName string) { + description := cmd.Long + if len(description) == 0 { + description = cmd.Short + } + + buf.WriteString(fmt.Sprintf(`%% "%s" "%s" "%s" "%s" "%s" +# NAME +`, header.Title, header.Section, header.date, header.Source, header.Manual)) + buf.WriteString(fmt.Sprintf("%s \\- %s\n\n", dashedName, cmd.Short)) + buf.WriteString("# SYNOPSIS\n") + buf.WriteString(fmt.Sprintf("**%s**\n\n", cmd.UseLine())) + buf.WriteString("# DESCRIPTION\n") + buf.WriteString(description + "\n\n") +} + +func manPrintFlags(buf *bytes.Buffer, flags *pflag.FlagSet) { + flags.VisitAll(func(flag *pflag.Flag) { + if len(flag.Deprecated) > 0 || flag.Hidden { + return + } + format := "" + if len(flag.Shorthand) > 0 && len(flag.ShorthandDeprecated) == 0 { + format = fmt.Sprintf("**-%s**, **--%s**", flag.Shorthand, flag.Name) + } else { + format = fmt.Sprintf("**--%s**", flag.Name) + } + if len(flag.NoOptDefVal) > 0 { + format += "[" + } + if flag.Value.Type() == "string" { + // put quotes on the value + format += "=%q" + } else { + format += "=%s" + } + if len(flag.NoOptDefVal) > 0 { + format += "]" + } + format += "\n\t%s\n\n" + buf.WriteString(fmt.Sprintf(format, flag.DefValue, flag.Usage)) + }) +} + +func manPrintOptions(buf *bytes.Buffer, command *cobra.Command) { + flags := command.NonInheritedFlags() + if flags.HasAvailableFlags() { + buf.WriteString("# OPTIONS\n") + manPrintFlags(buf, flags) + buf.WriteString("\n") + } + flags = command.InheritedFlags() + if flags.HasAvailableFlags() { + buf.WriteString("# OPTIONS INHERITED FROM PARENT COMMANDS\n") + manPrintFlags(buf, flags) + buf.WriteString("\n") + } +} + +func genMan(cmd *cobra.Command, header *GenManHeader) []byte { + cmd.InitDefaultHelpCmd() + cmd.InitDefaultHelpFlag() + + // something like `rootcmd-subcmd1-subcmd2` + dashCommandName := strings.Replace(cmd.CommandPath(), " ", "-", -1) + + buf := new(bytes.Buffer) + + manPreamble(buf, header, cmd, dashCommandName) + manPrintOptions(buf, cmd) + if len(cmd.Example) > 0 { + buf.WriteString("# EXAMPLE\n") + buf.WriteString(fmt.Sprintf("```\n%s\n```\n", cmd.Example)) + } + if hasSeeAlso(cmd) { + buf.WriteString("# SEE ALSO\n") + seealsos := make([]string, 0) + if cmd.HasParent() { + parentPath := cmd.Parent().CommandPath() + dashParentPath := strings.Replace(parentPath, " ", "-", -1) + seealso := fmt.Sprintf("**%s(%s)**", dashParentPath, header.Section) + seealsos = append(seealsos, seealso) + } + children := cmd.Commands() + sort.Sort(byName(children)) + for _, c := range children { + if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() { + continue + } + seealso := fmt.Sprintf("**%s-%s(%s)**", dashCommandName, c.Name(), header.Section) + seealsos = append(seealsos, seealso) + } + buf.WriteString(strings.Join(seealsos, ", ") + "\n") + } + return buf.Bytes() +} + +// Test to see if we have a reason to print See Also information in docs +// Basically this is a test for a parent command or a subcommand which is +// both not deprecated and not the autogenerated help command. +func hasSeeAlso(cmd *cobra.Command) bool { + if cmd.HasParent() { + return true + } + for _, c := range cmd.Commands() { + if !c.IsAvailableCommand() || c.IsAdditionalHelpTopicCommand() { + continue + } + return true + } + return false +} + +type byName []*cobra.Command + +func (s byName) Len() int { return len(s) } +func (s byName) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +func (s byName) Less(i, j int) bool { return s[i].Name() < s[j].Name() } diff --git a/internal/docs/man_test.go b/internal/docs/man_test.go new file mode 100644 index 000000000..4e844ad61 --- /dev/null +++ b/internal/docs/man_test.go @@ -0,0 +1,191 @@ +package docs + +import ( + "bufio" + "bytes" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +func translate(in string) string { + return strings.Replace(in, "-", "\\-", -1) +} + +func TestGenManDoc(t *testing.T) { + header := &GenManHeader{ + Title: "Project", + Section: "2", + } + + // We generate on a subcommand so we have both subcommands and parents + buf := new(bytes.Buffer) + if err := GenMan(echoCmd, header, buf); err != nil { + t.Fatal(err) + } + output := buf.String() + + // Make sure parent has - in CommandPath() in SEE ALSO: + parentPath := echoCmd.Parent().CommandPath() + dashParentPath := strings.Replace(parentPath, " ", "-", -1) + expected := translate(dashParentPath) + expected = expected + "(" + header.Section + ")" + checkStringContains(t, output, expected) + + checkStringContains(t, output, translate(echoCmd.Name())) + checkStringContains(t, output, translate(echoCmd.Name())) + checkStringContains(t, output, "boolone") + checkStringContains(t, output, "rootflag") + checkStringContains(t, output, translate(rootCmd.Name())) + checkStringContains(t, output, translate(echoSubCmd.Name())) + checkStringOmits(t, output, translate(deprecatedCmd.Name())) +} + +func TestGenManNoHiddenParents(t *testing.T) { + header := &GenManHeader{ + Title: "Project", + Section: "2", + } + + // We generate on a subcommand so we have both subcommands and parents + for _, name := range []string{"rootflag", "strtwo"} { + f := rootCmd.PersistentFlags().Lookup(name) + f.Hidden = true + defer func() { f.Hidden = false }() + } + buf := new(bytes.Buffer) + if err := GenMan(echoCmd, header, buf); err != nil { + t.Fatal(err) + } + output := buf.String() + + // Make sure parent has - in CommandPath() in SEE ALSO: + parentPath := echoCmd.Parent().CommandPath() + dashParentPath := strings.Replace(parentPath, " ", "-", -1) + expected := translate(dashParentPath) + expected = expected + "(" + header.Section + ")" + checkStringContains(t, output, expected) + + checkStringContains(t, output, translate(echoCmd.Name())) + checkStringContains(t, output, translate(echoCmd.Name())) + checkStringContains(t, output, "boolone") + checkStringOmits(t, output, "rootflag") + checkStringContains(t, output, translate(rootCmd.Name())) + checkStringContains(t, output, translate(echoSubCmd.Name())) + checkStringOmits(t, output, translate(deprecatedCmd.Name())) + checkStringOmits(t, output, "OPTIONS INHERITED FROM PARENT COMMANDS") +} + +func TestGenManSeeAlso(t *testing.T) { + rootCmd := &cobra.Command{Use: "root", Run: emptyRun} + aCmd := &cobra.Command{Use: "aaa", Run: emptyRun, Hidden: true} // #229 + bCmd := &cobra.Command{Use: "bbb", Run: emptyRun} + cCmd := &cobra.Command{Use: "ccc", Run: emptyRun} + rootCmd.AddCommand(aCmd, bCmd, cCmd) + + buf := new(bytes.Buffer) + header := &GenManHeader{} + if err := GenMan(rootCmd, header, buf); err != nil { + t.Fatal(err) + } + scanner := bufio.NewScanner(buf) + + if err := assertLineFound(scanner, ".SH SEE ALSO"); err != nil { + t.Fatalf("Couldn't find SEE ALSO section header: %v", err) + } + if err := assertNextLineEquals(scanner, ".PP"); err != nil { + t.Fatalf("First line after SEE ALSO wasn't break-indent: %v", err) + } + if err := assertNextLineEquals(scanner, `\fBroot\-bbb(1)\fP, \fBroot\-ccc(1)\fP`); err != nil { + t.Fatalf("Second line after SEE ALSO wasn't correct: %v", err) + } +} + +func TestManPrintFlagsHidesShortDeperecated(t *testing.T) { + c := &cobra.Command{} + c.Flags().StringP("foo", "f", "default", "Foo flag") + _ = c.Flags().MarkShorthandDeprecated("foo", "don't use it no more") + + buf := new(bytes.Buffer) + manPrintFlags(buf, c.Flags()) + + got := buf.String() + expected := "**--foo**=\"default\"\n\tFoo flag\n\n" + if got != expected { + t.Errorf("Expected %v, got %v", expected, got) + } +} + +func TestGenManTree(t *testing.T) { + c := &cobra.Command{Use: "do [OPTIONS] arg1 arg2"} + header := &GenManHeader{Section: "2"} + tmpdir, err := ioutil.TempDir("", "test-gen-man-tree") + if err != nil { + t.Fatalf("Failed to create tmpdir: %s", err.Error()) + } + defer os.RemoveAll(tmpdir) + + if err := GenManTree(c, header, tmpdir); err != nil { + t.Fatalf("GenManTree failed: %s", err.Error()) + } + + if _, err := os.Stat(filepath.Join(tmpdir, "do.2")); err != nil { + t.Fatalf("Expected file 'do.2' to exist") + } + + if header.Title != "" { + t.Fatalf("Expected header.Title to be unmodified") + } +} + +func assertLineFound(scanner *bufio.Scanner, expectedLine string) error { + for scanner.Scan() { + line := scanner.Text() + if line == expectedLine { + return nil + } + } + + if err := scanner.Err(); err != nil { + return fmt.Errorf("scan failed: %s", err) + } + + return fmt.Errorf("hit EOF before finding %v", expectedLine) +} + +func assertNextLineEquals(scanner *bufio.Scanner, expectedLine string) error { + if scanner.Scan() { + line := scanner.Text() + if line == expectedLine { + return nil + } + return fmt.Errorf("got %v, not %v", line, expectedLine) + } + + if err := scanner.Err(); err != nil { + return fmt.Errorf("scan failed: %v", err) + } + + return fmt.Errorf("hit EOF before finding %v", expectedLine) +} + +func BenchmarkGenManToFile(b *testing.B) { + file, err := ioutil.TempFile("", "") + if err != nil { + b.Fatal(err) + } + defer os.Remove(file.Name()) + defer file.Close() + + b.ResetTimer() + for i := 0; i < b.N; i++ { + if err := GenMan(rootCmd, nil, file); err != nil { + b.Fatal(err) + } + } +} diff --git a/internal/docs/markdown.go b/internal/docs/markdown.go new file mode 100644 index 000000000..3432e9784 --- /dev/null +++ b/internal/docs/markdown.go @@ -0,0 +1,114 @@ +package docs + +import ( + "bytes" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "github.com/spf13/cobra" +) + +func printOptions(buf *bytes.Buffer, cmd *cobra.Command, name string) error { + flags := cmd.NonInheritedFlags() + flags.SetOutput(buf) + if flags.HasAvailableFlags() { + buf.WriteString("### Options\n\n```\n") + flags.PrintDefaults() + buf.WriteString("```\n\n") + } + + parentFlags := cmd.InheritedFlags() + parentFlags.SetOutput(buf) + if parentFlags.HasAvailableFlags() { + buf.WriteString("### Options inherited from parent commands\n\n```\n") + parentFlags.PrintDefaults() + buf.WriteString("```\n\n") + } + return nil +} + +// GenMarkdown creates markdown output. +func GenMarkdown(cmd *cobra.Command, w io.Writer) error { + return GenMarkdownCustom(cmd, w, func(s string) string { return s }) +} + +// GenMarkdownCustom creates custom markdown output. +func GenMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) string) error { + cmd.InitDefaultHelpCmd() + cmd.InitDefaultHelpFlag() + + buf := new(bytes.Buffer) + name := cmd.CommandPath() + + buf.WriteString("## " + name + "\n\n") + buf.WriteString(cmd.Short + "\n\n") + if len(cmd.Long) > 0 { + buf.WriteString("### Synopsis\n\n") + buf.WriteString(cmd.Long + "\n\n") + } + + if cmd.Runnable() { + buf.WriteString(fmt.Sprintf("```\n%s\n```\n\n", cmd.UseLine())) + } + + if len(cmd.Example) > 0 { + buf.WriteString("### Examples\n\n") + buf.WriteString(fmt.Sprintf("```\n%s\n```\n\n", cmd.Example)) + } + + if err := printOptions(buf, cmd, name); err != nil { + return err + } + _, err := buf.WriteTo(w) + return err +} + +// GenMarkdownTree will generate a markdown page for this command and all +// descendants in the directory given. The header may be nil. +// This function may not work correctly if your command names have `-` in them. +// If you have `cmd` with two subcmds, `sub` and `sub-third`, +// and `sub` has a subcommand called `third`, it is undefined which +// help output will be in the file `cmd-sub-third.1`. +func GenMarkdownTree(cmd *cobra.Command, dir string) error { + identity := func(s string) string { return s } + emptyStr := func(s string) string { return "" } + return GenMarkdownTreeCustom(cmd, dir, emptyStr, identity) +} + +// GenMarkdownTreeCustom is the the same as GenMarkdownTree, but +// with custom filePrepender and linkHandler. +func GenMarkdownTreeCustom(cmd *cobra.Command, dir string, filePrepender, linkHandler func(string) string) error { + for _, c := range cmd.Commands() { + _, forceGeneration := c.Annotations["markdown:generate"] + if c.Hidden && !forceGeneration { + continue + } + + if err := GenMarkdownTreeCustom(c, dir, filePrepender, linkHandler); err != nil { + return err + } + } + + basename := strings.Replace(cmd.CommandPath(), " ", "_", -1) + ".md" + if basenameOverride, found := cmd.Annotations["markdown:basename"]; found { + basename = basenameOverride + ".md" + } + + filename := filepath.Join(dir, basename) + f, err := os.Create(filename) + if err != nil { + return err + } + defer f.Close() + + if _, err := io.WriteString(f, filePrepender(filename)); err != nil { + return err + } + if err := GenMarkdownCustom(cmd, f, linkHandler); err != nil { + return err + } + return nil +} diff --git a/internal/docs/markdown_test.go b/internal/docs/markdown_test.go new file mode 100644 index 000000000..27be95efa --- /dev/null +++ b/internal/docs/markdown_test.go @@ -0,0 +1,99 @@ +package docs + +import ( + "bytes" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "github.com/spf13/cobra" +) + +func TestGenMdDoc(t *testing.T) { + // We generate on subcommand so we have both subcommands and parents. + buf := new(bytes.Buffer) + if err := GenMarkdown(echoCmd, buf); err != nil { + t.Fatal(err) + } + output := buf.String() + + checkStringContains(t, output, echoCmd.Long) + checkStringContains(t, output, echoCmd.Example) + checkStringContains(t, output, "boolone") + checkStringContains(t, output, "rootflag") + checkStringOmits(t, output, rootCmd.Short) + checkStringOmits(t, output, echoSubCmd.Short) + checkStringOmits(t, output, deprecatedCmd.Short) + checkStringContains(t, output, "Options inherited from parent commands") +} + +func TestGenMdDocWithNoLongOrSynopsis(t *testing.T) { + // We generate on subcommand so we have both subcommands and parents. + buf := new(bytes.Buffer) + if err := GenMarkdown(dummyCmd, buf); err != nil { + t.Fatal(err) + } + output := buf.String() + + checkStringContains(t, output, dummyCmd.Example) + checkStringContains(t, output, dummyCmd.Short) + checkStringContains(t, output, "Options inherited from parent commands") + checkStringOmits(t, output, "### Synopsis") +} + +func TestGenMdNoHiddenParents(t *testing.T) { + // We generate on subcommand so we have both subcommands and parents. + for _, name := range []string{"rootflag", "strtwo"} { + f := rootCmd.PersistentFlags().Lookup(name) + f.Hidden = true + defer func() { f.Hidden = false }() + } + buf := new(bytes.Buffer) + if err := GenMarkdown(echoCmd, buf); err != nil { + t.Fatal(err) + } + output := buf.String() + + checkStringContains(t, output, echoCmd.Long) + checkStringContains(t, output, echoCmd.Example) + checkStringContains(t, output, "boolone") + checkStringOmits(t, output, "rootflag") + checkStringOmits(t, output, rootCmd.Short) + checkStringOmits(t, output, echoSubCmd.Short) + checkStringOmits(t, output, deprecatedCmd.Short) + checkStringOmits(t, output, "Options inherited from parent commands") +} + +func TestGenMdTree(t *testing.T) { + c := &cobra.Command{Use: "do [OPTIONS] arg1 arg2"} + tmpdir, err := ioutil.TempDir("", "test-gen-md-tree") + if err != nil { + t.Fatalf("Failed to create tmpdir: %v", err) + } + defer os.RemoveAll(tmpdir) + + if err := GenMarkdownTree(c, tmpdir); err != nil { + t.Fatalf("GenMarkdownTree failed: %v", err) + } + + if _, err := os.Stat(filepath.Join(tmpdir, "do.md")); err != nil { + t.Fatalf("Expected file 'do.md' to exist") + } +} + +func BenchmarkGenMarkdownToFile(b *testing.B) { + file, err := ioutil.TempFile("", "") + if err != nil { + b.Fatal(err) + } + defer os.Remove(file.Name()) + defer file.Close() + + b.ResetTimer() + for i := 0; i < b.N; i++ { + if err := GenMarkdown(rootCmd, file); err != nil { + b.Fatal(err) + } + } +} diff --git a/pkg/cmd/alias/delete/delete.go b/pkg/cmd/alias/delete/delete.go index ccf98ca68..a0ff1973b 100644 --- a/pkg/cmd/alias/delete/delete.go +++ b/pkg/cmd/alias/delete/delete.go @@ -6,7 +6,6 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -63,7 +62,7 @@ func deleteRun(opts *DeleteOptions) error { } if opts.IO.IsStdoutTTY() { - redCheck := utils.Red("✓") + redCheck := opts.IO.ColorScheme().Red("✓") fmt.Fprintf(opts.IO.ErrOut, "%s Deleted alias %s; was %s\n", redCheck, opts.Name, expansion) } diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index a9f628471..294a4bd0e 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -8,7 +8,6 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -84,6 +83,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command } func setRun(opts *SetOptions) error { + cs := opts.IO.ColorScheme() cfg, err := opts.Config() if err != nil { return err @@ -96,7 +96,7 @@ func setRun(opts *SetOptions) error { isTerminal := opts.IO.IsStdoutTTY() if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", utils.Bold(opts.Name), utils.Bold(opts.Expansion)) + fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", cs.Bold(opts.Name), cs.Bold(opts.Expansion)) } expansion := opts.Expansion @@ -114,13 +114,13 @@ func setRun(opts *SetOptions) error { return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion) } - successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓")) + successMsg := fmt.Sprintf("%s Added alias.", cs.SuccessIcon()) if oldExpansion, ok := aliasCfg.Get(opts.Name); ok { successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", - utils.Green("✓"), - utils.Bold(opts.Name), - utils.Bold(oldExpansion), - utils.Bold(expansion), + cs.SuccessIcon(), + cs.Bold(opts.Name), + cs.Bold(oldExpansion), + cs.Bold(expansion), ) } diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 929d6b61d..d702d0a1a 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -16,7 +16,6 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -228,7 +227,7 @@ func loginRun(opts *LoginOptions) error { } if authMode == 0 { - _, err := authflow.AuthFlowWithConfig(cfg, hostname, "", opts.Scopes) + _, err := authflow.AuthFlowWithConfig(cfg, opts.IO, hostname, "", opts.Scopes) if err != nil { return fmt.Errorf("failed to authenticate via web browser: %w", err) } @@ -258,6 +257,8 @@ func loginRun(opts *LoginOptions) error { } } + cs := opts.IO.ColorScheme() + gitProtocol := "https" if opts.Interactive { err = prompt.SurveyAskOne(&survey.Select{ @@ -279,7 +280,7 @@ func loginRun(opts *LoginOptions) error { return err } - fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", utils.GreenCheck()) + fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", cs.SuccessIcon()) } apiClient, err := client.ClientFromCfg(hostname, cfg) @@ -302,7 +303,7 @@ func loginRun(opts *LoginOptions) error { return err } - fmt.Fprintf(opts.IO.ErrOut, "%s Logged in as %s\n", utils.GreenCheck(), utils.Bold(username)) + fmt.Fprintf(opts.IO.ErrOut, "%s Logged in as %s\n", cs.SuccessIcon(), cs.Bold(username)) return nil } diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index 78a3bb98f..699de3968 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -12,7 +12,6 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -151,8 +150,9 @@ func logoutRun(opts *LogoutOptions) error { isTTY := opts.IO.IsStdinTTY() && opts.IO.IsStdoutTTY() if isTTY { + cs := opts.IO.ColorScheme() fmt.Fprintf(opts.IO.ErrOut, "%s Logged out of %s%s\n", - utils.GreenCheck(), utils.Bold(hostname), usernameStr) + cs.SuccessIcon(), cs.Bold(hostname), usernameStr) } return nil diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index e2a7829b4..dd7862f3f 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -20,15 +20,15 @@ type RefreshOptions struct { Hostname string Scopes []string - AuthFlow func(config.Config, string, []string) error + AuthFlow func(config.Config, *iostreams.IOStreams, string, []string) error } func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra.Command { opts := &RefreshOptions{ IO: f.IOStreams, Config: f.Config, - AuthFlow: func(cfg config.Config, hostname string, scopes []string) error { - _, err := authflow.AuthFlowWithConfig(cfg, hostname, "", scopes) + AuthFlow: func(cfg config.Config, io *iostreams.IOStreams, hostname string, scopes []string) error { + _, err := authflow.AuthFlowWithConfig(cfg, io, hostname, "", scopes) return err }, } @@ -118,5 +118,5 @@ func refreshRun(opts *RefreshOptions) error { return err } - return opts.AuthFlow(cfg, hostname, opts.Scopes) + return opts.AuthFlow(cfg, opts.IO, hostname, opts.Scopes) } diff --git a/pkg/cmd/auth/refresh/refresh_test.go b/pkg/cmd/auth/refresh/refresh_test.go index 46db2af52..065dd3fa2 100644 --- a/pkg/cmd/auth/refresh/refresh_test.go +++ b/pkg/cmd/auth/refresh/refresh_test.go @@ -213,7 +213,7 @@ func Test_refreshRun(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { aa := authArgs{} - tt.opts.AuthFlow = func(_ config.Config, hostname string, scopes []string) error { + tt.opts.AuthFlow = func(_ config.Config, _ *iostreams.IOStreams, hostname string, scopes []string) error { aa.hostname = hostname aa.scopes = scopes return nil diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index d5be70441..59603981d 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -64,12 +63,14 @@ func statusRun(opts *StatusOptions) error { stderr := opts.IO.ErrOut + cs := opts.IO.ColorScheme() + statusInfo := map[string][]string{} hostnames, err := cfg.Hosts() if len(hostnames) == 0 || err != nil { fmt.Fprintf(stderr, - "You are not logged into any GitHub hosts. Run %s to authenticate.\n", utils.Bold("gh auth login")) + "You are not logged into any GitHub hosts. Run %s to authenticate.\n", cs.Bold("gh auth login")) return cmdutil.SilentError } @@ -98,39 +99,39 @@ func statusRun(opts *StatusOptions) error { if err != nil { var missingScopes *api.MissingScopesError if errors.As(err, &missingScopes) { - addMsg("%s %s: the token in %s is %s", utils.Red("X"), hostname, tokenSource, err) + addMsg("%s %s: the token in %s is %s", cs.Red("X"), hostname, tokenSource, err) if tokenIsWriteable { addMsg("- To request missing scopes, run: %s %s\n", - utils.Bold("gh auth refresh -h"), - utils.Bold(hostname)) + cs.Bold("gh auth refresh -h"), + cs.Bold(hostname)) } } else { - addMsg("%s %s: authentication failed", utils.Red("X"), hostname) - addMsg("- The %s token in %s is no longer valid.", utils.Bold(hostname), tokenSource) + addMsg("%s %s: authentication failed", cs.Red("X"), hostname) + addMsg("- The %s token in %s is no longer valid.", cs.Bold(hostname), tokenSource) if tokenIsWriteable { addMsg("- To re-authenticate, run: %s %s", - utils.Bold("gh auth login -h"), utils.Bold(hostname)) + cs.Bold("gh auth login -h"), cs.Bold(hostname)) addMsg("- To forget about this host, run: %s %s", - utils.Bold("gh auth logout -h"), utils.Bold(hostname)) + cs.Bold("gh auth logout -h"), cs.Bold(hostname)) } } failed = true } else { username, err := api.CurrentLoginName(apiClient, hostname) if err != nil { - addMsg("%s %s: api call failed: %s", utils.Red("X"), hostname, err) + addMsg("%s %s: api call failed: %s", cs.Red("X"), hostname, err) } - addMsg("%s Logged in to %s as %s (%s)", utils.GreenCheck(), hostname, utils.Bold(username), tokenSource) + addMsg("%s Logged in to %s as %s (%s)", cs.SuccessIcon(), hostname, cs.Bold(username), tokenSource) proto, _ := cfg.Get(hostname, "git_protocol") if proto != "" { addMsg("%s Git operations for %s configured to use %s protocol.", - utils.GreenCheck(), hostname, utils.Bold(proto)) + cs.SuccessIcon(), hostname, cs.Bold(proto)) } tokenDisplay := "*******************" if opts.ShowToken { tokenDisplay = token } - addMsg("%s Token: %s", utils.GreenCheck(), tokenDisplay) + addMsg("%s Token: %s", cs.SuccessIcon(), tokenDisplay) } addMsg("") @@ -143,7 +144,7 @@ func statusRun(opts *StatusOptions) error { if !ok { continue } - fmt.Fprintf(stderr, "%s\n", utils.Bold(hostname)) + fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname)) for _, line := range lines { fmt.Fprintf(stderr, " %s\n", line) } diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go index 72dbb6955..7cecb66cb 100644 --- a/pkg/cmd/config/config.go +++ b/pkg/cmd/config/config.go @@ -1,12 +1,12 @@ package config import ( - "errors" "fmt" "strings" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" + cmdGet "github.com/cli/cli/pkg/cmd/config/get" + cmdSet "github.com/cli/cli/pkg/cmd/config/set" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -31,100 +31,8 @@ func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { cmdutil.DisableAuthCheck(cmd) - cmd.AddCommand(NewCmdConfigGet(f)) - cmd.AddCommand(NewCmdConfigSet(f)) - - return cmd -} - -func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command { - var hostname string - - cmd := &cobra.Command{ - Use: "get ", - Short: "Print the value of a given configuration key", - Example: heredoc.Doc(` - $ gh config get git_protocol - https - `), - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := f.Config() - if err != nil { - return err - } - - val, err := cfg.Get(hostname, args[0]) - if err != nil { - return err - } - - if val != "" { - fmt.Fprintf(f.IOStreams.Out, "%s\n", val) - } - return nil - }, - } - - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting") - - return cmd -} - -func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { - var hostname string - - cmd := &cobra.Command{ - Use: "set ", - Short: "Update configuration with a value for the given key", - Example: heredoc.Doc(` - $ gh config set editor vim - $ gh config set editor "code --wait" - $ gh config set git_protocol ssh --host github.com - $ gh config set prompt disabled - `), - Args: cobra.ExactArgs(2), - RunE: func(cmd *cobra.Command, args []string) error { - cfg, err := f.Config() - if err != nil { - return err - } - - key, value := args[0], args[1] - knownKey := false - for _, configKey := range config.ConfigOptions() { - if key == configKey.Key { - knownKey = true - break - } - } - if !knownKey { - iostreams := f.IOStreams - warningIcon := iostreams.ColorScheme().WarningIcon() - fmt.Fprintf(iostreams.ErrOut, "%s warning: '%s' is not a known configuration key\n", warningIcon, key) - } - err = cfg.Set(hostname, key, value) - if err != nil { - var invalidValue *config.InvalidValueError - if errors.As(err, &invalidValue) { - var values []string - for _, v := range invalidValue.ValidValues { - values = append(values, fmt.Sprintf("'%s'", v)) - } - return fmt.Errorf("failed to set %q to %q: valid values are %v", key, value, strings.Join(values, ", ")) - } - return fmt.Errorf("failed to set %q to %q: %w", key, value, err) - } - - err = cfg.Write() - if err != nil { - return fmt.Errorf("failed to write config to disk: %w", err) - } - return nil - }, - } - - cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting") + cmd.AddCommand(cmdGet.NewCmdConfigGet(f, nil)) + cmd.AddCommand(cmdSet.NewCmdConfigSet(f, nil)) return cmd } diff --git a/pkg/cmd/config/config_test.go b/pkg/cmd/config/config_test.go deleted file mode 100644 index 3a3d9c83a..000000000 --- a/pkg/cmd/config/config_test.go +++ /dev/null @@ -1,177 +0,0 @@ -package config - -import ( - "errors" - "testing" - - "github.com/cli/cli/internal/config" - "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/iostreams" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -type configStub map[string]string - -func genKey(host, key string) string { - if host != "" { - return host + ":" + key - } - return key -} - -func (c configStub) Get(host, key string) (string, error) { - val, _, err := c.GetWithSource(host, key) - return val, err -} - -func (c configStub) GetWithSource(host, key string) (string, string, error) { - if v, found := c[genKey(host, key)]; found { - return v, "(memory)", nil - } - return "", "", errors.New("not found") -} - -func (c configStub) Set(host, key, value string) error { - c[genKey(host, key)] = value - return nil -} - -func (c configStub) Aliases() (*config.AliasConfig, error) { - return nil, nil -} - -func (c configStub) Hosts() ([]string, error) { - return nil, nil -} - -func (c configStub) UnsetHost(hostname string) { -} - -func (c configStub) CheckWriteable(host, key string) error { - return nil -} - -func (c configStub) Write() error { - c["_written"] = "true" - return nil -} - -func TestConfigGet(t *testing.T) { - tests := []struct { - name string - config configStub - args []string - stdout string - stderr string - }{ - { - name: "get key", - config: configStub{ - "editor": "ed", - }, - args: []string{"editor"}, - stdout: "ed\n", - stderr: "", - }, - { - name: "get key scoped by host", - config: configStub{ - "editor": "ed", - "github.com:editor": "vim", - }, - args: []string{"editor", "-h", "github.com"}, - stdout: "vim\n", - stderr: "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - io, _, stdout, stderr := iostreams.Test() - f := &cmdutil.Factory{ - IOStreams: io, - Config: func() (config.Config, error) { - return tt.config, nil - }, - } - - cmd := NewCmdConfigGet(f) - cmd.Flags().BoolP("help", "x", false, "") - cmd.SetArgs(tt.args) - cmd.SetOut(stdout) - cmd.SetErr(stderr) - - _, err := cmd.ExecuteC() - require.NoError(t, err) - - assert.Equal(t, tt.stdout, stdout.String()) - assert.Equal(t, tt.stderr, stderr.String()) - assert.Equal(t, "", tt.config["_written"]) - }) - } -} - -func TestConfigSet(t *testing.T) { - tests := []struct { - name string - config configStub - args []string - expectKey string - expectVal string - stdout string - stderr string - }{ - { - name: "set key", - config: configStub{}, - args: []string{"editor", "vim"}, - expectKey: "editor", - expectVal: "vim", - stdout: "", - stderr: "", - }, - { - name: "set key scoped by host", - config: configStub{}, - args: []string{"editor", "vim", "-h", "github.com"}, - expectKey: "github.com:editor", - expectVal: "vim", - stdout: "", - stderr: "", - }, - { - name: "set key", - config: configStub{}, - args: []string{"unknownKey", "someValue"}, - expectKey: "unknownKey", - expectVal: "someValue", - stdout: "", - stderr: "! warning: 'unknownKey' is not a known configuration key\n", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - io, _, stdout, stderr := iostreams.Test() - f := &cmdutil.Factory{ - IOStreams: io, - Config: func() (config.Config, error) { - return tt.config, nil - }, - } - - cmd := NewCmdConfigSet(f) - cmd.Flags().BoolP("help", "x", false, "") - cmd.SetArgs(tt.args) - cmd.SetOut(stdout) - cmd.SetErr(stderr) - - _, err := cmd.ExecuteC() - require.NoError(t, err) - - assert.Equal(t, tt.stdout, stdout.String()) - assert.Equal(t, tt.stderr, stderr.String()) - assert.Equal(t, tt.expectVal, tt.config[tt.expectKey]) - assert.Equal(t, "true", tt.config["_written"]) - }) - } -} diff --git a/pkg/cmd/config/get/get.go b/pkg/cmd/config/get/get.go new file mode 100644 index 000000000..a3ce55176 --- /dev/null +++ b/pkg/cmd/config/get/get.go @@ -0,0 +1,65 @@ +package get + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type GetOptions struct { + IO *iostreams.IOStreams + Config config.Config + + Hostname string + Key string +} + +func NewCmdConfigGet(f *cmdutil.Factory, runF func(*GetOptions) error) *cobra.Command { + opts := &GetOptions{ + IO: f.IOStreams, + } + + cmd := &cobra.Command{ + Use: "get ", + Short: "Print the value of a given configuration key", + Example: heredoc.Doc(` + $ gh config get git_protocol + https + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + config, err := f.Config() + if err != nil { + return err + } + opts.Config = config + opts.Key = args[0] + + if runF != nil { + return runF(opts) + } + + return getRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Hostname, "host", "h", "", "Get per-host setting") + + return cmd +} + +func getRun(opts *GetOptions) error { + val, err := opts.Config.Get(opts.Hostname, opts.Key) + if err != nil { + return err + } + + if val != "" { + fmt.Fprintf(opts.IO.Out, "%s\n", val) + } + return nil +} diff --git a/pkg/cmd/config/get/get_test.go b/pkg/cmd/config/get/get_test.go new file mode 100644 index 000000000..1e2104dbd --- /dev/null +++ b/pkg/cmd/config/get/get_test.go @@ -0,0 +1,122 @@ +package get + +import ( + "bytes" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdConfigGet(t *testing.T) { + tests := []struct { + name string + input string + output GetOptions + wantsErr bool + }{ + { + name: "no arguments", + input: "", + output: GetOptions{}, + wantsErr: true, + }, + { + name: "get key", + input: "key", + output: GetOptions{Key: "key"}, + wantsErr: false, + }, + { + name: "get key with host", + input: "key --host test.com", + output: GetOptions{Hostname: "test.com", Key: "key"}, + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.ConfigStub{}, nil + }, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *GetOptions + cmd := NewCmdConfigGet(f, func(opts *GetOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.output.Key, gotOpts.Key) + }) + } +} + +func Test_getRun(t *testing.T) { + tests := []struct { + name string + input *GetOptions + stdout string + stderr string + wantErr bool + }{ + { + name: "get key", + input: &GetOptions{ + Key: "editor", + Config: config.ConfigStub{ + "editor": "ed", + }, + }, + stdout: "ed\n", + }, + { + name: "get key scoped by host", + input: &GetOptions{ + Hostname: "github.com", + Key: "editor", + Config: config.ConfigStub{ + "editor": "ed", + "github.com:editor": "vim", + }, + }, + stdout: "vim\n", + }, + } + + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + tt.input.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := getRun(tt.input) + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + _, err = tt.input.Config.Get("", "_written") + assert.Error(t, err) + }) + } +} diff --git a/pkg/cmd/config/set/set.go b/pkg/cmd/config/set/set.go new file mode 100644 index 000000000..d1a2d5c59 --- /dev/null +++ b/pkg/cmd/config/set/set.go @@ -0,0 +1,90 @@ +package set + +import ( + "errors" + "fmt" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type SetOptions struct { + IO *iostreams.IOStreams + Config config.Config + + Key string + Value string + Hostname string +} + +func NewCmdConfigSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { + opts := &SetOptions{ + IO: f.IOStreams, + } + + cmd := &cobra.Command{ + Use: "set ", + Short: "Update configuration with a value for the given key", + Example: heredoc.Doc(` + $ gh config set editor vim + $ gh config set editor "code --wait" + $ gh config set git_protocol ssh --host github.com + $ gh config set prompt disabled + `), + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + config, err := f.Config() + if err != nil { + return err + } + opts.Config = config + opts.Key = args[0] + opts.Value = args[1] + + if runF != nil { + return runF(opts) + } + + return setRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Hostname, "host", "h", "", "Set per-host setting") + + return cmd +} + +func setRun(opts *SetOptions) error { + err := config.ValidateKey(opts.Key) + if err != nil { + warningIcon := opts.IO.ColorScheme().WarningIcon() + fmt.Fprintf(opts.IO.ErrOut, "%s warning: '%s' is not a known configuration key\n", warningIcon, opts.Key) + } + + err = config.ValidateValue(opts.Key, opts.Value) + if err != nil { + var invalidValue *config.InvalidValueError + if errors.As(err, &invalidValue) { + var values []string + for _, v := range invalidValue.ValidValues { + values = append(values, fmt.Sprintf("'%s'", v)) + } + return fmt.Errorf("failed to set %q to %q: valid values are %v", opts.Key, opts.Value, strings.Join(values, ", ")) + } + } + + err = opts.Config.Set(opts.Hostname, opts.Key, opts.Value) + if err != nil { + return fmt.Errorf("failed to set %q to %q: %w", opts.Key, opts.Value, err) + } + + err = opts.Config.Write() + if err != nil { + return fmt.Errorf("failed to write config to disk: %w", err) + } + return nil +} diff --git a/pkg/cmd/config/set/set_test.go b/pkg/cmd/config/set/set_test.go new file mode 100644 index 000000000..56efc2ebd --- /dev/null +++ b/pkg/cmd/config/set/set_test.go @@ -0,0 +1,157 @@ +package set + +import ( + "bytes" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdConfigSet(t *testing.T) { + tests := []struct { + name string + input string + output SetOptions + wantsErr bool + }{ + { + name: "no arguments", + input: "", + output: SetOptions{}, + wantsErr: true, + }, + { + name: "no value argument", + input: "key", + output: SetOptions{}, + wantsErr: true, + }, + { + name: "set key value", + input: "key value", + output: SetOptions{Key: "key", Value: "value"}, + wantsErr: false, + }, + { + name: "set key value with host", + input: "key value --host test.com", + output: SetOptions{Hostname: "test.com", Key: "key", Value: "value"}, + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.ConfigStub{}, nil + }, + } + + argv, err := shlex.Split(tt.input) + assert.NoError(t, err) + + var gotOpts *SetOptions + cmd := NewCmdConfigSet(f, func(opts *SetOptions) error { + gotOpts = opts + return nil + }) + cmd.Flags().BoolP("help", "x", false, "") + + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + + assert.NoError(t, err) + assert.Equal(t, tt.output.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.output.Key, gotOpts.Key) + assert.Equal(t, tt.output.Value, gotOpts.Value) + }) + } +} + +func Test_setRun(t *testing.T) { + tests := []struct { + name string + input *SetOptions + expectedValue string + stdout string + stderr string + wantsErr bool + errMsg string + }{ + { + name: "set key value", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "editor", + Value: "vim", + }, + expectedValue: "vim", + }, + { + name: "set key value scoped by host", + input: &SetOptions{ + Config: config.ConfigStub{}, + Hostname: "github.com", + Key: "editor", + Value: "vim", + }, + expectedValue: "vim", + }, + { + name: "set unknown key", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "unknownKey", + Value: "someValue", + }, + expectedValue: "someValue", + stderr: "! warning: 'unknownKey' is not a known configuration key\n", + }, + { + name: "set invalid value", + input: &SetOptions{ + Config: config.ConfigStub{}, + Key: "git_protocol", + Value: "invalid", + }, + wantsErr: true, + errMsg: "failed to set \"git_protocol\" to \"invalid\": valid values are 'https', 'ssh'", + }, + } + for _, tt := range tests { + io, _, stdout, stderr := iostreams.Test() + tt.input.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := setRun(tt.input) + if tt.wantsErr { + assert.EqualError(t, err, tt.errMsg) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + + val, err := tt.input.Config.Get(tt.input.Hostname, tt.input.Key) + assert.NoError(t, err) + assert.Equal(t, tt.expectedValue, val) + + val, err = tt.input.Config.Get("", "_written") + assert.NoError(t, err) + assert.Equal(t, "true", val) + }) + } +} diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index 85fcb158f..dc7d4b257 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -19,7 +19,6 @@ import ( "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -116,8 +115,10 @@ func createRun(opts *CreateOptions) error { completionMessage = fmt.Sprintf("Created gist %s", gistName) } + cs := opts.IO.ColorScheme() + errOut := opts.IO.ErrOut - fmt.Fprintf(errOut, "%s %s\n", utils.Gray("-"), processMessage) + fmt.Fprintf(errOut, "%s %s\n", cs.Gray("-"), processMessage) httpClient, err := opts.HttpClient() if err != nil { @@ -132,10 +133,10 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate by doing `gh config set -h github.com oauth_token ''` and running the command again.") } } - return fmt.Errorf("%s Failed to create gist: %w", utils.Red("X"), err) + return fmt.Errorf("%s Failed to create gist: %w", cs.Red("X"), err) } - fmt.Fprintf(errOut, "%s %s\n", utils.Green("✓"), completionMessage) + fmt.Fprintf(errOut, "%s %s\n", cs.SuccessIcon(), completionMessage) fmt.Fprintln(opts.IO.Out, gist.HTMLURL) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index be24cba54..701975c84 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -117,7 +117,7 @@ func viewRun(opts *ViewOptions) error { content := gistFile.Content if strings.Contains(gistFile.Type, "markdown") && !opts.Raw { style := markdown.GetStyle(opts.IO.DetectTerminalTheme()) - rendered, err := markdown.Render(gistFile.Content, style) + rendered, err := markdown.Render(gistFile.Content, style, "") if err == nil { content = rendered } diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 66314a86d..a6aa04bc6 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/pkg/cmd/issue/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -53,6 +52,8 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm } func closeRun(opts *CloseOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -65,7 +66,7 @@ func closeRun(opts *CloseOptions) error { } if issue.Closed { - fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already closed\n", utils.Yellow("!"), issue.Number, issue.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already closed\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } @@ -74,7 +75,7 @@ func closeRun(opts *CloseOptions) error { return err } - fmt.Fprintf(opts.IO.ErrOut, "%s Closed issue #%d (%s)\n", utils.Red("✔"), issue.Number, issue.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Closed issue #%d (%s)\n", cs.Red("✔"), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go index 72a052503..e65e2eceb 100644 --- a/pkg/cmd/issue/reopen/reopen.go +++ b/pkg/cmd/issue/reopen/reopen.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/pkg/cmd/issue/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -53,6 +52,8 @@ func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Co } func reopenRun(opts *ReopenOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -65,7 +66,7 @@ func reopenRun(opts *ReopenOptions) error { } if !issue.Closed { - fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already open\n", utils.Yellow("!"), issue.Number, issue.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already open\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } @@ -74,7 +75,7 @@ func reopenRun(opts *ReopenOptions) error { return err } - fmt.Fprintf(opts.IO.ErrOut, "%s Reopened issue #%d (%s)\n", utils.Green("✔"), issue.Number, issue.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Reopened issue #%d (%s)\n", cs.SuccessIcon(), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/shared/display.go b/pkg/cmd/issue/shared/display.go index bd6bc72d2..c774a010d 100644 --- a/pkg/cmd/issue/shared/display.go +++ b/pkg/cmd/issue/shared/display.go @@ -14,6 +14,7 @@ import ( ) func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues []api.Issue) { + cs := io.ColorScheme() table := utils.NewTablePrinter(io) for _, issue := range issues { issueNum := strconv.Itoa(issue.Number) @@ -27,14 +28,14 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues } now := time.Now() ago := now.Sub(issue.UpdatedAt) - table.AddField(issueNum, nil, prShared.ColorFuncForState(issue.State)) + table.AddField(issueNum, nil, cs.ColorFromString(prShared.ColorForState(issue.State))) if !table.IsTTY() { table.AddField(issue.State, nil, nil) } table.AddField(text.ReplaceExcessiveWhitespace(issue.Title), nil, nil) - table.AddField(labels, truncateLabels, utils.Gray) + table.AddField(labels, truncateLabels, cs.Gray) if table.IsTTY() { - table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray) + table.AddField(utils.FuzzyAgo(ago), nil, cs.Gray) } else { table.AddField(issue.UpdatedAt.String(), nil, nil) } @@ -43,7 +44,7 @@ func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues _ = table.Render() remaining := totalCount - len(issues) if remaining > 0 { - fmt.Fprintf(io.Out, utils.Gray("%sAnd %d more\n"), prefix, remaining) + fmt.Fprintf(io.Out, cs.Gray("%sAnd %d more\n"), prefix, remaining) } } diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go index 42ba91823..218d5d38a 100644 --- a/pkg/cmd/issue/status/status.go +++ b/pkg/cmd/issue/status/status.go @@ -80,28 +80,28 @@ func statusRun(opts *StatusOptions) error { fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") - prShared.PrintHeader(out, "Issues assigned to you") + prShared.PrintHeader(opts.IO, "Issues assigned to you") if issuePayload.Assigned.TotalCount > 0 { issueShared.PrintIssues(opts.IO, " ", issuePayload.Assigned.TotalCount, issuePayload.Assigned.Issues) } else { message := " There are no issues assigned to you" - prShared.PrintMessage(out, message) + prShared.PrintMessage(opts.IO, message) } fmt.Fprintln(out) - prShared.PrintHeader(out, "Issues mentioning you") + prShared.PrintHeader(opts.IO, "Issues mentioning you") if issuePayload.Mentioned.TotalCount > 0 { issueShared.PrintIssues(opts.IO, " ", issuePayload.Mentioned.TotalCount, issuePayload.Mentioned.Issues) } else { - prShared.PrintMessage(out, " There are no issues mentioning you") + prShared.PrintMessage(opts.IO, " There are no issues mentioning you") } fmt.Fprintln(out) - prShared.PrintHeader(out, "Issues opened by you") + prShared.PrintHeader(opts.IO, "Issues opened by you") if issuePayload.Authored.TotalCount > 0 { issueShared.PrintIssues(opts.IO, " ", issuePayload.Authored.TotalCount, issuePayload.Authored.Issues) } else { - prShared.PrintMessage(out, " There are no issues opened by you") + prShared.PrintMessage(opts.IO, " There are no issues opened by you") } fmt.Fprintln(out) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 66156898e..2bada530c 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -129,11 +129,12 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { out := io.Out now := time.Now() ago := now.Sub(issue.CreatedAt) + cs := io.ColorScheme() // Header (Title and State) - fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprint(out, issueStateTitleWithColor(issue.State)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + fmt.Fprintln(out, cs.Bold(issue.Title)) + fmt.Fprint(out, issueStateTitleWithColor(cs, issue.State)) + fmt.Fprintln(out, cs.Gray(fmt.Sprintf( " • %s opened %s • %s", issue.Author.Login, utils.FuzzyAgo(ago), @@ -143,19 +144,19 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { // Metadata fmt.Fprintln(out) if assignees := issueAssigneeList(*issue); assignees != "" { - fmt.Fprint(out, utils.Bold("Assignees: ")) + fmt.Fprint(out, cs.Bold("Assignees: ")) fmt.Fprintln(out, assignees) } if labels := shared.IssueLabelList(*issue); labels != "" { - fmt.Fprint(out, utils.Bold("Labels: ")) + fmt.Fprint(out, cs.Bold("Labels: ")) fmt.Fprintln(out, labels) } if projects := issueProjectList(*issue); projects != "" { - fmt.Fprint(out, utils.Bold("Projects: ")) + fmt.Fprint(out, cs.Bold("Projects: ")) fmt.Fprintln(out, projects) } if issue.Milestone.Title != "" { - fmt.Fprint(out, utils.Bold("Milestone: ")) + fmt.Fprint(out, cs.Bold("Milestone: ")) fmt.Fprintln(out, issue.Milestone.Title) } @@ -163,7 +164,7 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { if issue.Body != "" { fmt.Fprintln(out) style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(issue.Body, style) + md, err := markdown.Render(issue.Body, style, "") if err != nil { return err } @@ -172,12 +173,12 @@ func printHumanIssuePreview(io *iostreams.IOStreams, issue *api.Issue) error { fmt.Fprintln(out) // Footer - fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) + fmt.Fprintf(out, cs.Gray("View this issue on GitHub: %s\n"), issue.URL) return nil } -func issueStateTitleWithColor(state string) string { - colorFunc := prShared.ColorFuncForState(state) +func issueStateTitleWithColor(cs *iostreams.ColorScheme, state string) string { + colorFunc := cs.ColorFromString(prShared.ColorForState(state)) return colorFunc(strings.Title(strings.ToLower(state))) } diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index 30a59a7ca..096be87a9 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -1,7 +1,6 @@ package checkout import ( - "errors" "fmt" "net/http" "os" @@ -44,12 +43,7 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr cmd := &cobra.Command{ Use: "checkout { | | }", Short: "Check out a pull request in git", - Args: func(cmd *cobra.Command, args []string) error { - if len(args) == 0 { - return &cmdutil.FlagError{Err: errors.New("argument required")} - } - return nil - }, + Args: cmdutil.MinimumArgs(1, "argument required"), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/pr/checks/checks.go b/pkg/cmd/pr/checks/checks.go index f267c9854..f93cbe6d1 100644 --- a/pkg/cmd/pr/checks/checks.go +++ b/pkg/cmd/pr/checks/checks.go @@ -76,12 +76,12 @@ func checksRun(opts *ChecksOptions) error { } if len(pr.Commits.Nodes) == 0 { - return nil + return fmt.Errorf("no commit found on the pull request") } rollup := pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes if len(rollup) == 0 { - return nil + return fmt.Errorf("no checks reported on the '%s' branch", pr.BaseRefName) } passing := 0 @@ -97,13 +97,15 @@ func checksRun(opts *ChecksOptions) error { markColor func(string) string } + cs := opts.IO.ColorScheme() + outputs := []output{} for _, c := range pr.Commits.Nodes[0].Commit.StatusCheckRollup.Contexts.Nodes { mark := "✓" bucket := "pass" state := c.State - markColor := utils.Green + markColor := cs.Green if state == "" { if c.Status == "COMPLETED" { state = c.Conclusion @@ -116,12 +118,12 @@ func checksRun(opts *ChecksOptions) error { passing++ case "ERROR", "FAILURE", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED": mark = "X" - markColor = utils.Red + markColor = cs.Red failing++ bucket = "fail" case "EXPECTED", "REQUESTED", "QUEUED", "PENDING", "IN_PROGRESS", "STALE": mark = "-" - markColor = utils.Yellow + markColor = cs.Yellow pending++ bucket = "pending" default: @@ -206,7 +208,7 @@ func checksRun(opts *ChecksOptions) error { "%d failing, %d successful, and %d pending checks", failing, passing, pending) - summary = fmt.Sprintf("%s\n%s", utils.Bold(summary), tallies) + summary = fmt.Sprintf("%s\n%s", cs.Bold(summary), tallies) } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/pr/checks/checks_test.go b/pkg/cmd/pr/checks/checks_test.go index 43dbf8cda..324f8a74f 100644 --- a/pkg/cmd/pr/checks/checks_test.go +++ b/pkg/cmd/pr/checks/checks_test.go @@ -66,69 +66,59 @@ func Test_checksRun(t *testing.T) { name string fixture string stubs func(*httpmock.Registry) - wantOut string nontty bool - wantErr bool + wantOut string + wantErr string }{ { name: "no commits", stubs: func(reg *httpmock.Registry) { reg.Register( httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.JSONResponse( - bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 123 } - } } } - `))) + httpmock.StringResponse(` + { "data": { "repository": { + "pullRequest": { "number": 123 } + } } } + `)) }, + wantOut: "", + wantErr: "no commit found on the pull request", }, { name: "no checks", stubs: func(reg *httpmock.Registry) { reg.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]} } + "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" } } } } `)) }, + wantOut: "", + wantErr: "no checks reported on the 'master' branch", }, { name: "some failing", fixture: "./fixtures/someFailing.json", wantOut: "Some checks were not successful\n1 failing, 1 successful, and 1 pending checks\n\nX sad tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n- slow tests 1m26s sweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "some pending", fixture: "./fixtures/somePending.json", wantOut: "Some checks are still pending\n0 failing, 2 successful, and 1 pending checks\n\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n- slow tests 1m26s sweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "all passing", fixture: "./fixtures/allPassing.json", wantOut: "All checks were successful\n0 failing, 3 successful, and 0 pending checks\n\n✓ awesome tests 1m26s sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n", + wantErr: "", }, { name: "with statuses", fixture: "./fixtures/withStatuses.json", wantOut: "Some checks were not successful\n1 failing, 2 successful, and 0 pending checks\n\nX a status sweet link\n✓ cool tests 1m26s sweet link\n✓ rad tests 1m26s sweet link\n", - wantErr: true, - }, - { - name: "no commits", - nontty: true, - stubs: func(reg *httpmock.Registry) { - reg.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), - httpmock.JSONResponse( - bytes.NewBufferString(` - { "data": { "repository": { - "pullRequest": { "number": 123 } - } } } - `))) - }, + wantErr: "SilentError", }, { name: "no checks", @@ -136,37 +126,40 @@ func Test_checksRun(t *testing.T) { stubs: func(reg *httpmock.Registry) { reg.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { - "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]} } + "pullRequest": { "number": 123, "commits": { "nodes": [{"commit": {"oid": "abc"}}]}, "baseRefName": "master" } } } } `)) }, + wantOut: "", + wantErr: "no checks reported on the 'master' branch", }, { name: "some failing", nontty: true, fixture: "./fixtures/someFailing.json", wantOut: "sad tests\tfail\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "some pending", nontty: true, fixture: "./fixtures/somePending.json", wantOut: "cool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\nslow tests\tpending\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, { name: "all passing", nontty: true, fixture: "./fixtures/allPassing.json", wantOut: "awesome tests\tpass\t1m26s\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n", + wantErr: "", }, { name: "with statuses", nontty: true, fixture: "./fixtures/withStatuses.json", wantOut: "a status\tfail\t0\tsweet link\ncool tests\tpass\t1m26s\tsweet link\nrad tests\tpass\t1m26s\tsweet link\n", - wantErr: true, + wantErr: "SilentError", }, } @@ -184,13 +177,12 @@ func Test_checksRun(t *testing.T) { } reg := &httpmock.Registry{} + defer reg.Verify(t) + if tt.stubs != nil { tt.stubs(reg) } else if tt.fixture != "" { - reg.Register( - httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.FileResponse(tt.fixture)) - } else { - panic("need either stubs or fixture key") + reg.Register(httpmock.GraphQL(`query PullRequestByNumber\b`), httpmock.FileResponse(tt.fixture)) } opts.HttpClient = func() (*http.Client, error) { @@ -198,14 +190,13 @@ func Test_checksRun(t *testing.T) { } err := checksRun(opts) - if tt.wantErr { - assert.Equal(t, "SilentError", err.Error()) - } else { - assert.NoError(t, err) + if err != nil { + assert.Equal(t, tt.wantErr, err.Error()) + } else if tt.wantErr != "" { + t.Errorf("expected %q, got nil error", tt.wantErr) } assert.Equal(t, tt.wantOut, stdout.String()) - reg.Verify(t) }) } } diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index 7ff257855..6b6cc8c9a 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -11,7 +11,6 @@ import ( "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -61,6 +60,8 @@ func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Comm } func closeRun(opts *CloseOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -73,10 +74,10 @@ func closeRun(opts *CloseOptions) error { } if pr.State == "MERGED" { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be closed because it was already merged", utils.Red("!"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be closed because it was already merged", cs.Red("!"), pr.Number, pr.Title) return cmdutil.SilentError } else if pr.Closed { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already closed\n", utils.Yellow("!"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already closed\n", cs.Yellow("!"), pr.Number, pr.Title) return nil } @@ -85,7 +86,7 @@ func closeRun(opts *CloseOptions) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", utils.Red("✔"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", cs.Red("✔"), pr.Number, pr.Title) crossRepoPR := pr.HeadRepositoryOwner.Login != baseRepo.RepoOwner() @@ -114,24 +115,24 @@ func closeRun(opts *CloseOptions) error { if localBranchExists { err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { - err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err) + err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) return err } } if branchToSwitchTo != "" { - branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) + branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) } } if !crossRepoPR { err = api.BranchDeleteRemote(apiClient, baseRepo, pr.HeadRefName) if err != nil { - err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) + err = fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err) return err } } - fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) + fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.Red("✔"), cs.Cyan(pr.HeadRefName), branchSwitchString) } return nil diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 38950cb65..f8d67cb15 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "net/url" + "regexp" "strings" "time" @@ -127,6 +128,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co } func createRun(opts *CreateOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -323,11 +326,11 @@ func createRun(opts *CreateOptions) error { if isTerminal { fmt.Fprintf(opts.IO.ErrOut, message, - utils.Cyan(headBranchLabel), - utils.Cyan(baseBranch), + cs.Cyan(headBranchLabel), + cs.Cyan(baseBranch), ghrepo.FullName(baseRepo)) if (title == "" || body == "") && defaultsErr != nil { - fmt.Fprintf(opts.IO.ErrOut, "%s warning: could not compute title or body defaults: %s\n", utils.Yellow("!"), defaultsErr) + fmt.Fprintf(opts.IO.ErrOut, "%s warning: could not compute title or body defaults: %s\n", cs.Yellow("!"), defaultsErr) } } } @@ -426,21 +429,33 @@ func createRun(opts *CreateOptions) error { // automatically push the branch if it hasn't been pushed anywhere yet if isPushEnabled { - pushTries := 0 - maxPushTries := 3 - for { - if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch)); err != nil { - if didForkRepo && pushTries < maxPushTries { - pushTries++ - // first wait 2 seconds after forking, then 4s, then 6s - waitSeconds := 2 * pushTries - fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second")) - time.Sleep(time.Duration(waitSeconds) * time.Second) - continue + pushBranch := func() error { + pushTries := 0 + maxPushTries := 3 + for { + r := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "") + defer r.Flush() + cmdErr := r + cmdOut := opts.IO.Out + if err := git.Push(headRemote.Name, fmt.Sprintf("HEAD:%s", headBranch), cmdOut, cmdErr); err != nil { + if didForkRepo && pushTries < maxPushTries { + pushTries++ + // first wait 2 seconds after forking, then 4s, then 6s + waitSeconds := 2 * pushTries + fmt.Fprintf(opts.IO.ErrOut, "waiting %s before retrying...\n", utils.Pluralize(waitSeconds, "second")) + time.Sleep(time.Duration(waitSeconds) * time.Second) + continue + } + return err } - return err + break } - break + return nil + } + + err := pushBranch() + if err != nil { + return err } } @@ -561,3 +576,5 @@ func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assi } return url, nil } + +var gitPushRegexp = regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$") diff --git a/pkg/cmd/pr/create/regexp_writer.go b/pkg/cmd/pr/create/regexp_writer.go new file mode 100644 index 000000000..500637d7c --- /dev/null +++ b/pkg/cmd/pr/create/regexp_writer.go @@ -0,0 +1,64 @@ +package create + +import ( + "bytes" + "io" + "regexp" +) + +func NewRegexpWriter(out io.Writer, re *regexp.Regexp, repl string) *RegexpWriter { + return &RegexpWriter{out: out, re: *re, repl: repl} +} + +type RegexpWriter struct { + out io.Writer + re regexp.Regexp + repl string + buf []byte +} + +func (s *RegexpWriter) Write(data []byte) (int, error) { + if len(data) == 0 { + return 0, nil + } + + filtered := []byte{} + repl := []byte(s.repl) + lines := bytes.SplitAfter(data, []byte("\n")) + + if len(s.buf) > 0 { + lines[0] = append(s.buf, lines[0]...) + } + + for i, line := range lines { + if i == len(lines) { + s.buf = line + } else { + f := s.re.ReplaceAll(line, repl) + if len(f) > 0 { + filtered = append(filtered, f...) + } + } + } + + if len(filtered) != 0 { + _, err := s.out.Write(filtered) + if err != nil { + return 0, err + } + } + + return len(data), nil +} + +func (s *RegexpWriter) Flush() (int, error) { + if len(s.buf) > 0 { + repl := []byte(s.repl) + filtered := s.re.ReplaceAll(s.buf, repl) + if len(filtered) > 0 { + return s.out.Write(filtered) + } + } + + return 0, nil +} diff --git a/pkg/cmd/pr/create/regexp_writer_test.go b/pkg/cmd/pr/create/regexp_writer_test.go new file mode 100644 index 000000000..fd772a760 --- /dev/null +++ b/pkg/cmd/pr/create/regexp_writer_test.go @@ -0,0 +1,160 @@ +package create + +import ( + "bytes" + "regexp" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/stretchr/testify/assert" +) + +func Test_Write(t *testing.T) { + type input struct { + in []string + re *regexp.Regexp + repl string + } + type output struct { + wantsErr bool + out string + length int + } + tests := []struct { + name string + input input + output output + }{ + { + name: "single line input", + input: input{ + in: []string{"some input line that has wrong information"}, + re: regexp.MustCompile("wrong"), + repl: "right", + }, + output: output{ + wantsErr: false, + out: "some input line that has right information", + length: 42, + }, + }, + { + name: "multiple line input", + input: input{ + in: []string{"multiple lines\nin this\ninput lines"}, + re: regexp.MustCompile("lines"), + repl: "tests", + }, + output: output{ + wantsErr: false, + out: "multiple tests\nin this\ninput tests", + length: 34, + }, + }, + { + name: "no matches", + input: input{ + in: []string{"this line has no matches"}, + re: regexp.MustCompile("wrong"), + repl: "right", + }, + output: output{ + wantsErr: false, + out: "this line has no matches", + length: 24, + }, + }, + { + name: "no output", + input: input{ + in: []string{"remove this whole line"}, + re: regexp.MustCompile("^remove.*$"), + repl: "", + }, + output: output{ + wantsErr: false, + out: "", + length: 22, + }, + }, + { + name: "no input", + input: input{ + in: []string{""}, + re: regexp.MustCompile("remove"), + repl: "", + }, + output: output{ + wantsErr: false, + out: "", + length: 0, + }, + }, + { + name: "multiple lines removed", + input: input{ + in: []string{"begining line\nremove this whole line\nremove this one also\nnot this one"}, + re: regexp.MustCompile("(?s)^remove.*$"), + repl: "", + }, + output: output{ + wantsErr: false, + out: "begining line\nnot this one", + length: 70, + }, + }, + { + name: "removes remote from git push output", + input: input{ + in: []string{heredoc.Doc(` + output: some information + remote: + remote: Create a pull request for 'regex' on GitHub by visiting: + remote: https://github.com/owner/repo/pull/new/regex + remote: + output: more information + `)}, + re: regexp.MustCompile("^remote: (Create a pull request.*by visiting|[[:space:]]*https://.*/pull/new/).*\n?$"), + repl: "", + }, + output: output{ + wantsErr: false, + out: "output: some information\nremote:\nremote:\noutput: more information\n", + length: 189, + }, + }, + { + name: "multiple writes", + input: input{ + in: []string{"first write\n", "second write ", "third write"}, + re: regexp.MustCompile("write"), + repl: "read", + }, + output: output{ + wantsErr: false, + out: "first read\nsecond read third read", + length: 36, + }, + }, + } + + for _, tt := range tests { + out := &bytes.Buffer{} + writer := NewRegexpWriter(out, tt.input.re, tt.input.repl) + t.Run(tt.name, func(t *testing.T) { + length := 0 + for _, in := range tt.input.in { + l, err := writer.Write([]byte(in)) + length = length + l + if tt.output.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + } + writer.Flush() + assert.Equal(t, tt.output.out, out.String()) + assert.Equal(t, tt.output.length, length) + }) + } +} diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index e4fe318c8..3ed4b1ea1 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -145,15 +145,16 @@ func listRun(opts *ListOptions) error { fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } + cs := opts.IO.ColorScheme() table := utils.NewTablePrinter(opts.IO) for _, pr := range listResult.PullRequests { prNum := strconv.Itoa(pr.Number) if table.IsTTY() { prNum = "#" + prNum } - table.AddField(prNum, nil, shared.ColorFuncForPR(pr)) + table.AddField(prNum, nil, cs.ColorFromString(shared.ColorForPR(pr))) table.AddField(text.ReplaceExcessiveWhitespace(pr.Title), nil, nil) - table.AddField(pr.HeadLabel(), nil, utils.Cyan) + table.AddField(pr.HeadLabel(), nil, cs.Cyan) if !table.IsTTY() { table.AddField(prStateWithDraft(&pr), nil, nil) } diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index bc22fdac1..2eafe9712 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -16,7 +16,6 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -111,6 +110,8 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm } func mergeRun(opts *MergeOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -123,13 +124,13 @@ func mergeRun(opts *MergeOptions) error { } if pr.Mergeable == "CONFLICTING" { - err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", utils.Red("!"), pr.Number, pr.Title) + err := fmt.Errorf("%s Pull request #%d (%s) has conflicts and isn't mergeable ", cs.Red("!"), pr.Number, pr.Title) return err } else if pr.Mergeable == "UNKNOWN" { - err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", utils.Red("!"), pr.Number, pr.Title) + err := fmt.Errorf("%s Pull request #%d (%s) can't be merged right now; try again in a few seconds", cs.Red("!"), pr.Number, pr.Title) return err } else if pr.State == "MERGED" { - err := fmt.Errorf("%s Pull request #%d (%s) was already merged", utils.Red("!"), pr.Number, pr.Title) + err := fmt.Errorf("%s Pull request #%d (%s) was already merged", cs.Red("!"), pr.Number, pr.Title) return err } @@ -170,7 +171,7 @@ func mergeRun(opts *MergeOptions) error { isTerminal := opts.IO.IsStdoutTTY() if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", utils.Magenta("✔"), action, pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s %s pull request #%d (%s)\n", cs.Magenta("✔"), action, pr.Number, pr.Title) } if deleteBranch { @@ -198,13 +199,13 @@ func mergeRun(opts *MergeOptions) error { if localBranchExists { err = git.DeleteLocalBranch(pr.HeadRefName) if err != nil { - err = fmt.Errorf("failed to delete local branch %s: %w", utils.Cyan(pr.HeadRefName), err) + err = fmt.Errorf("failed to delete local branch %s: %w", cs.Cyan(pr.HeadRefName), err) return err } } if branchToSwitchTo != "" { - branchSwitchString = fmt.Sprintf(" and switched to branch %s", utils.Cyan(branchToSwitchTo)) + branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) } } @@ -213,13 +214,13 @@ func mergeRun(opts *MergeOptions) error { var httpErr api.HTTPError // The ref might have already been deleted by GitHub if err != nil && (!errors.As(err, &httpErr) || httpErr.StatusCode != 422) { - err = fmt.Errorf("failed to delete remote branch %s: %w", utils.Cyan(pr.HeadRefName), err) + err = fmt.Errorf("failed to delete remote branch %s: %w", cs.Cyan(pr.HeadRefName), err) return err } } if isTerminal { - fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", utils.Red("✔"), utils.Cyan(pr.HeadRefName), branchSwitchString) + fmt.Fprintf(opts.IO.ErrOut, "%s Deleted branch %s%s\n", cs.Red("✔"), cs.Cyan(pr.HeadRefName), branchSwitchString) } } diff --git a/pkg/cmd/pr/ready/ready.go b/pkg/cmd/pr/ready/ready.go index 933dec082..d160eccf4 100644 --- a/pkg/cmd/pr/ready/ready.go +++ b/pkg/cmd/pr/ready/ready.go @@ -12,7 +12,6 @@ import ( "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -63,6 +62,8 @@ func NewCmdReady(f *cmdutil.Factory, runF func(*ReadyOptions) error) *cobra.Comm } func readyRun(opts *ReadyOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -75,10 +76,10 @@ func readyRun(opts *ReadyOptions) error { } if pr.Closed { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", utils.Red("!"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", cs.Red("!"), pr.Number) return cmdutil.SilentError } else if !pr.IsDraft { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is already \"ready for review\"\n", utils.Yellow("!"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is already \"ready for review\"\n", cs.Yellow("!"), pr.Number) return nil } @@ -87,7 +88,7 @@ func readyRun(opts *ReadyOptions) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is marked as \"ready for review\"\n", utils.Green("✔"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d is marked as \"ready for review\"\n", cs.SuccessIcon(), pr.Number) return nil } diff --git a/pkg/cmd/pr/reopen/reopen.go b/pkg/cmd/pr/reopen/reopen.go index 78f4a36ec..afbc1c37f 100644 --- a/pkg/cmd/pr/reopen/reopen.go +++ b/pkg/cmd/pr/reopen/reopen.go @@ -10,7 +10,6 @@ import ( "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -53,6 +52,8 @@ func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Co } func reopenRun(opts *ReopenOptions) error { + cs := opts.IO.ColorScheme() + httpClient, err := opts.HttpClient() if err != nil { return err @@ -65,12 +66,12 @@ func reopenRun(opts *ReopenOptions) error { } if pr.State == "MERGED" { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be reopened because it was already merged", utils.Red("!"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) can't be reopened because it was already merged", cs.Red("!"), pr.Number, pr.Title) return cmdutil.SilentError } if !pr.Closed { - fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already open\n", utils.Yellow("!"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Pull request #%d (%s) is already open\n", cs.Yellow("!"), pr.Number, pr.Title) return nil } @@ -79,7 +80,7 @@ func reopenRun(opts *ReopenOptions) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(opts.IO.ErrOut, "%s Reopened pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Reopened pull request #%d (%s)\n", cs.SuccessIcon(), pr.Number, pr.Title) return nil } diff --git a/pkg/cmd/pr/review/review.go b/pkg/cmd/pr/review/review.go index 1f0916bf7..4bb3317cf 100644 --- a/pkg/cmd/pr/review/review.go +++ b/pkg/cmd/pr/review/review.go @@ -17,7 +17,6 @@ import ( "github.com/cli/cli/pkg/markdown" "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -172,13 +171,15 @@ func reviewRun(opts *ReviewOptions) error { return nil } + cs := opts.IO.ColorScheme() + switch reviewData.State { case api.ReviewComment: - fmt.Fprintf(opts.IO.ErrOut, "%s Reviewed pull request #%d\n", utils.Gray("-"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Reviewed pull request #%d\n", cs.Gray("-"), pr.Number) case api.ReviewApprove: - fmt.Fprintf(opts.IO.ErrOut, "%s Approved pull request #%d\n", utils.Green("✓"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Approved pull request #%d\n", cs.SuccessIcon(), pr.Number) case api.ReviewRequestChanges: - fmt.Fprintf(opts.IO.ErrOut, "%s Requested changes to pull request #%d\n", utils.Red("+"), pr.Number) + fmt.Fprintf(opts.IO.ErrOut, "%s Requested changes to pull request #%d\n", cs.Red("+"), pr.Number) } return nil @@ -254,7 +255,7 @@ func reviewSurvey(io *iostreams.IOStreams, editorCommand string) (*api.PullReque if len(bodyAnswers.Body) > 0 { style := markdown.GetStyle(io.DetectTerminalTheme()) - renderedBody, err := markdown.Render(bodyAnswers.Body, style) + renderedBody, err := markdown.Render(bodyAnswers.Body, style, "") if err != nil { return nil, err } diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index 31e7228b6..061fb9664 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -2,48 +2,49 @@ package shared import ( "fmt" - "io" "strings" "github.com/cli/cli/api" + "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/utils" ) -func StateTitleWithColor(pr api.PullRequest) string { - prStateColorFunc := ColorFuncForPR(pr) +func StateTitleWithColor(cs *iostreams.ColorScheme, pr api.PullRequest) string { + prStateColorFunc := cs.ColorFromString(ColorForPR(pr)) + if pr.State == "OPEN" && pr.IsDraft { return prStateColorFunc(strings.Title(strings.ToLower("Draft"))) } return prStateColorFunc(strings.Title(strings.ToLower(pr.State))) } -func ColorFuncForPR(pr api.PullRequest) func(string) string { +func ColorForPR(pr api.PullRequest) string { if pr.State == "OPEN" && pr.IsDraft { - return utils.Gray + return "gray" } - return ColorFuncForState(pr.State) + return ColorForState(pr.State) } -// ColorFuncForState returns a color function for a PR/Issue state -func ColorFuncForState(state string) func(string) string { +// ColorForState returns a color constant for a PR/Issue state +func ColorForState(state string) string { switch state { case "OPEN": - return utils.Green + return "green" case "CLOSED": - return utils.Red + return "red" case "MERGED": - return utils.Magenta + return "magenta" default: - return nil + return "" } } -func PrintHeader(w io.Writer, s string) { - fmt.Fprintln(w, utils.Bold(s)) +func PrintHeader(io *iostreams.IOStreams, s string) { + fmt.Fprintln(io.Out, io.ColorScheme().Bold(s)) } -func PrintMessage(w io.Writer, s string) { - fmt.Fprintln(w, utils.Gray(s)) +func PrintMessage(io *iostreams.IOStreams, s string) { + fmt.Fprintln(io.Out, io.ColorScheme().Gray(s)) } func ListHeader(repoName string, itemName string, matchCount int, totalMatchCount int, hasFilters bool) string { diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 2c012d712..bec49f259 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -3,7 +3,6 @@ package status import ( "errors" "fmt" - "io" "net/http" "regexp" "strconv" @@ -18,7 +17,6 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/text" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -104,38 +102,39 @@ func statusRun(opts *StatusOptions) error { defer opts.IO.StopPager() out := opts.IO.Out + cs := opts.IO.ColorScheme() fmt.Fprintln(out, "") fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") - shared.PrintHeader(out, "Current branch") + shared.PrintHeader(opts.IO, "Current branch") currentPR := prPayload.CurrentPR if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { currentPR = nil } if currentPR != nil { - printPrs(out, 1, *currentPR) + printPrs(opts.IO, 1, *currentPR) } else if currentPRHeadRef == "" { - shared.PrintMessage(out, " There is no current branch") + shared.PrintMessage(opts.IO, " There is no current branch") } else { - shared.PrintMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) + shared.PrintMessage(opts.IO, fmt.Sprintf(" There is no pull request associated with %s", cs.Cyan("["+currentPRHeadRef+"]"))) } fmt.Fprintln(out) - shared.PrintHeader(out, "Created by you") + shared.PrintHeader(opts.IO, "Created by you") if prPayload.ViewerCreated.TotalCount > 0 { - printPrs(out, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) + printPrs(opts.IO, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) } else { - shared.PrintMessage(out, " You have no open pull requests") + shared.PrintMessage(opts.IO, " You have no open pull requests") } fmt.Fprintln(out) - shared.PrintHeader(out, "Requesting a code review from you") + shared.PrintHeader(opts.IO, "Requesting a code review from you") if prPayload.ReviewRequested.TotalCount > 0 { - printPrs(out, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) + printPrs(opts.IO, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) } else { - shared.PrintMessage(out, " You have no pull requests to review") + shared.PrintMessage(opts.IO, " You have no pull requests to review") } fmt.Fprintln(out) @@ -179,20 +178,16 @@ func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem return } -func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { +func printPrs(io *iostreams.IOStreams, totalCount int, prs ...api.PullRequest) { + w := io.Out + cs := io.ColorScheme() + for _, pr := range prs { prNumber := fmt.Sprintf("#%d", pr.Number) - prStateColorFunc := utils.Green - if pr.IsDraft { - prStateColorFunc = utils.Gray - } else if pr.State == "MERGED" { - prStateColorFunc = utils.Magenta - } else if pr.State == "CLOSED" { - prStateColorFunc = utils.Red - } + prStateColorFunc := cs.ColorFromString(shared.ColorForPR(pr)) - fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, text.ReplaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) + fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, text.ReplaceExcessiveWhitespace(pr.Title)), cs.Cyan("["+pr.HeadLabel()+"]")) checks := pr.ChecksStatus() reviews := pr.ReviewStatus() @@ -208,14 +203,14 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { var summary string if checks.Failing > 0 { if checks.Failing == checks.Total { - summary = utils.Red("× All checks failing") + summary = cs.Red("× All checks failing") } else { - summary = utils.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) + summary = cs.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) } } else if checks.Pending > 0 { - summary = utils.Yellow("- Checks pending") + summary = cs.Yellow("- Checks pending") } else if checks.Passing == checks.Total { - summary = utils.Green("✓ Checks passing") + summary = cs.Green("✓ Checks passing") } fmt.Fprint(w, summary) } @@ -226,20 +221,20 @@ func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { } if reviews.ChangesRequested { - fmt.Fprint(w, utils.Red("+ Changes requested")) + fmt.Fprint(w, cs.Red("+ Changes requested")) } else if reviews.ReviewRequired { - fmt.Fprint(w, utils.Yellow("- Review required")) + fmt.Fprint(w, cs.Yellow("- Review required")) } else if reviews.Approved { - fmt.Fprint(w, utils.Green("✓ Approved")) + fmt.Fprint(w, cs.Green("✓ Approved")) } } else { - fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(pr)) + fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(cs, pr)) } fmt.Fprint(w, "\n") } remaining := totalCount - len(prs) if remaining > 0 { - fmt.Fprintf(w, utils.Gray(" And %d more\n"), remaining) + fmt.Fprintf(w, cs.Gray(" And %d more\n"), remaining) } } diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 4476a84c0..1bb0eff1b 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -3,7 +3,6 @@ package view import ( "errors" "fmt" - "io" "net/http" "sort" "strings" @@ -111,11 +110,15 @@ func viewRun(opts *ViewOptions) error { if connectedToTerminal { return printHumanPrPreview(opts.IO, pr) } - return printRawPrPreview(opts.IO.Out, pr) + + return printRawPrPreview(opts.IO, pr) } -func printRawPrPreview(out io.Writer, pr *api.PullRequest) error { - reviewers := prReviewerList(*pr) +func printRawPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { + out := io.Out + cs := io.ColorScheme() + + reviewers := prReviewerList(*pr, cs) assignees := prAssigneeList(*pr) labels := prLabelList(*pr) projects := prProjectList(*pr) @@ -140,10 +143,12 @@ func printRawPrPreview(out io.Writer, pr *api.PullRequest) error { func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { out := io.Out + cs := io.ColorScheme() + // Header (Title and State) - fmt.Fprintln(out, utils.Bold(pr.Title)) - fmt.Fprintf(out, "%s", shared.StateTitleWithColor(*pr)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + fmt.Fprintln(out, cs.Bold(pr.Title)) + fmt.Fprintf(out, "%s", shared.StateTitleWithColor(cs, *pr)) + fmt.Fprintln(out, cs.Gray(fmt.Sprintf( " • %s wants to merge %s into %s from %s", pr.Author.Login, utils.Pluralize(pr.Commits.TotalCount, "commit"), @@ -153,24 +158,24 @@ func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { fmt.Fprintln(out) // Metadata - if reviewers := prReviewerList(*pr); reviewers != "" { - fmt.Fprint(out, utils.Bold("Reviewers: ")) + if reviewers := prReviewerList(*pr, cs); reviewers != "" { + fmt.Fprint(out, cs.Bold("Reviewers: ")) fmt.Fprintln(out, reviewers) } if assignees := prAssigneeList(*pr); assignees != "" { - fmt.Fprint(out, utils.Bold("Assignees: ")) + fmt.Fprint(out, cs.Bold("Assignees: ")) fmt.Fprintln(out, assignees) } if labels := prLabelList(*pr); labels != "" { - fmt.Fprint(out, utils.Bold("Labels: ")) + fmt.Fprint(out, cs.Bold("Labels: ")) fmt.Fprintln(out, labels) } if projects := prProjectList(*pr); projects != "" { - fmt.Fprint(out, utils.Bold("Projects: ")) + fmt.Fprint(out, cs.Bold("Projects: ")) fmt.Fprintln(out, projects) } if pr.Milestone.Title != "" { - fmt.Fprint(out, utils.Bold("Milestone: ")) + fmt.Fprint(out, cs.Bold("Milestone: ")) fmt.Fprintln(out, pr.Milestone.Title) } @@ -178,7 +183,7 @@ func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { if pr.Body != "" { fmt.Fprintln(out) style := markdown.GetStyle(io.TerminalTheme()) - md, err := markdown.Render(pr.Body, style) + md, err := markdown.Render(pr.Body, style, "") if err != nil { return err } @@ -187,7 +192,7 @@ func printHumanPrPreview(io *iostreams.IOStreams, pr *api.PullRequest) error { fmt.Fprintln(out) // Footer - fmt.Fprintf(out, utils.Gray("View this pull request on GitHub: %s\n"), pr.URL) + fmt.Fprintf(out, cs.Gray("View this pull request on GitHub: %s\n"), pr.URL) return nil } @@ -205,43 +210,39 @@ type reviewerState struct { State string } -// colorFuncForReviewerState returns a color function for a reviewer state -func colorFuncForReviewerState(state string) func(string) string { - switch state { - case requestedReviewState: - return utils.Yellow - case approvedReviewState: - return utils.Green - case changesRequestedReviewState: - return utils.Red - case commentedReviewState: - return func(str string) string { return str } // Do nothing - default: - return nil - } -} - // formattedReviewerState formats a reviewerState with state color -func formattedReviewerState(reviewer *reviewerState) string { +func formattedReviewerState(cs *iostreams.ColorScheme, reviewer *reviewerState) string { state := reviewer.State if state == dismissedReviewState { // Show "DISMISSED" review as "COMMENTED", since "dimissed" only makes // sense when displayed in an events timeline but not in the final tally. state = commentedReviewState } - stateColorFunc := colorFuncForReviewerState(state) - return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(state)), "_", " "))) + + var colorFunc func(string) string + switch state { + case requestedReviewState: + colorFunc = cs.Yellow + case approvedReviewState: + colorFunc = cs.Green + case changesRequestedReviewState: + colorFunc = cs.Red + default: + colorFunc = func(str string) string { return str } // Do nothing + } + + return fmt.Sprintf("%s (%s)", reviewer.Name, colorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(state)), "_", " "))) } // prReviewerList generates a reviewer list with their last state -func prReviewerList(pr api.PullRequest) string { +func prReviewerList(pr api.PullRequest, cs *iostreams.ColorScheme) string { reviewerStates := parseReviewers(pr) reviewers := make([]string, 0, len(reviewerStates)) sortReviewerStates(reviewerStates) for _, reviewer := range reviewerStates { - reviewers = append(reviewers, formattedReviewerState(reviewer)) + reviewers = append(reviewers, formattedReviewerState(cs, reviewer)) } reviewerList := strings.Join(reviewers, ", ") diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index 5534821c1..e8a1ce853 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -76,7 +76,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co # upload a release asset with a display label $ gh release create v1.2.3 '/path/to/asset.zip#My display label' `), - Args: cobra.MinimumNArgs(1), + Args: cmdutil.MinimumArgs(1, "could not create: no tag name provided"), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index 806801ec2..8bfb2d325 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -158,7 +158,7 @@ func Test_NewCmdCreate(t *testing.T) { name: "no arguments", args: "", isTTY: true, - wantErr: "requires at least 1 arg(s), only received 0", + wantErr: "could not create: no tag name provided", }, } for _, tt := range tests { diff --git a/pkg/cmd/release/view/view.go b/pkg/cmd/release/view/view.go index 4dcfdeee2..2218a6d1c 100644 --- a/pkg/cmd/release/view/view.go +++ b/pkg/cmd/release/view/view.go @@ -124,7 +124,7 @@ func renderReleaseTTY(io *iostreams.IOStreams, release *shared.Release) error { } style := markdown.GetStyle(io.DetectTerminalTheme()) - renderedDescription, err := markdown.Render(release.Body, style) + renderedDescription, err := markdown.Render(release.Body, style, "") if err != nil { return err } diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index 6dc225538..1ea216b54 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -23,7 +23,6 @@ type CloneOptions struct { IO *iostreams.IOStreams GitArgs []string - Directory string Repository string } @@ -38,7 +37,7 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm DisableFlagsInUseLine: true, Use: "clone [] [-- ...]", - Args: cobra.MinimumNArgs(1), + Args: cmdutil.MinimumArgs(1, "cannot clone: repository argument required"), Short: "Clone a repository locally", Long: heredoc.Doc(` Clone a GitHub repository locally. diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index f696cd354..e7aa57b08 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -12,8 +12,87 @@ import ( "github.com/cli/cli/test" "github.com/google/shlex" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +func TestNewCmdClone(t *testing.T) { + testCases := []struct { + name string + args string + wantOpts CloneOptions + wantErr string + }{ + { + name: "no arguments", + args: "", + wantErr: "cannot clone: repository argument required", + }, + { + name: "repo argument", + args: "OWNER/REPO", + wantOpts: CloneOptions{ + Repository: "OWNER/REPO", + GitArgs: []string{}, + }, + }, + { + name: "directory argument", + args: "OWNER/REPO mydir", + wantOpts: CloneOptions{ + Repository: "OWNER/REPO", + GitArgs: []string{"mydir"}, + }, + }, + { + name: "git clone arguments", + args: "OWNER/REPO -- --depth 1 --recurse-submodules", + wantOpts: CloneOptions{ + Repository: "OWNER/REPO", + GitArgs: []string{"--depth", "1", "--recurse-submodules"}, + }, + }, + { + name: "unknown argument", + args: "OWNER/REPO --depth 1", + wantErr: "unknown flag: --depth\nSeparate git clone flags with '--'.", + }, + } + for _, tt := range testCases { + t.Run(tt.name, func(t *testing.T) { + io, stdin, stdout, stderr := iostreams.Test() + fac := &cmdutil.Factory{IOStreams: io} + + var opts *CloneOptions + cmd := NewCmdClone(fac, func(co *CloneOptions) error { + opts = co + return nil + }) + + argv, err := shlex.Split(tt.args) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(stdin) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + + _, err = cmd.ExecuteC() + if err != nil { + assert.Equal(t, tt.wantErr, err.Error()) + return + } else if tt.wantErr != "" { + t.Errorf("expected error %q, got nil", tt.wantErr) + } + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, tt.wantOpts.Repository, opts.Repository) + assert.Equal(t, tt.wantOpts.GitArgs, opts.GitArgs) + }) + } +} + func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) { io, stdin, stdout, stderr := iostreams.Test() fac := &cmdutil.Factory{ @@ -203,10 +282,3 @@ func Test_RepoClone_withoutUsername(t *testing.T) { assert.Equal(t, 1, cs.Count) assert.Equal(t, "git clone https://github.com/OWNER/REPO.git", strings.Join(cs.Calls[0].Args, " ")) } - -func Test_RepoClone_flagError(t *testing.T) { - _, err := runCloneCommand(nil, "--depth 1 OWNER/REPO") - if err == nil || err.Error() != "unknown flag: --depth\nSeparate git clone flags with '--'." { - t.Errorf("unexpected error %v", err) - } -} diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 77ddcaf33..50c4a4a8f 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -18,7 +18,6 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" - "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -246,11 +245,11 @@ func createRun(opts *CreateOptions) error { stderr := opts.IO.ErrOut stdout := opts.IO.Out - greenCheck := utils.Green("✓") + cs := opts.IO.ColorScheme() isTTY := opts.IO.IsStdoutTTY() if isTTY { - fmt.Fprintf(stderr, "%s Created repository %s on GitHub\n", greenCheck, ghrepo.FullName(repo)) + fmt.Fprintf(stderr, "%s Created repository %s on GitHub\n", cs.SuccessIcon(), ghrepo.FullName(repo)) } else { fmt.Fprintln(stdout, repo.URL) } @@ -272,7 +271,7 @@ func createRun(opts *CreateOptions) error { return err } if isTTY { - fmt.Fprintf(stderr, "%s Added remote %s\n", greenCheck, remoteURL) + fmt.Fprintf(stderr, "%s Added remote %s\n", cs.SuccessIcon(), remoteURL) } } else if opts.IO.CanPrompt() { doSetup := createLocalDirectory @@ -300,7 +299,7 @@ func createRun(opts *CreateOptions) error { return err } - fmt.Fprintf(stderr, "%s Initialized repository in './%s/'\n", utils.GreenCheck(), path) + fmt.Fprintf(stderr, "%s Initialized repository in './%s/'\n", cs.SuccessIcon(), path) } } diff --git a/pkg/cmd/repo/credits/credits.go b/pkg/cmd/repo/credits/credits.go index c69ce1f3e..e792b54d6 100644 --- a/pkg/cmd/repo/credits/credits.go +++ b/pkg/cmd/repo/credits/credits.go @@ -45,10 +45,10 @@ func NewCmdCredits(f *cmdutil.Factory, runF func(*CreditsOptions) error) *cobra. 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 `), @@ -154,6 +154,7 @@ func creditsRun(opts *CreditsOptions) error { static := opts.Static || isWindows out := opts.IO.Out + cs := opts.IO.ColorScheme() if isTTY && static { fmt.Fprintln(out, "THANK YOU CONTRIBUTORS!!! <3") @@ -167,7 +168,7 @@ func creditsRun(opts *CreditsOptions) error { } if isTTY && !static { - logins = append(logins, getColor(x)(c.Login)) + logins = append(logins, cs.ColorFromString(getColor(x))(c.Login)) } else { fmt.Fprintf(out, "%s\n", c.Login) } @@ -183,14 +184,13 @@ func creditsRun(opts *CreditsOptions) error { thankLines := strings.Split(thankYou, "\n") for x, tl := range thankLines { - lines = append(lines, getColor(x)(tl)) + lines = append(lines, cs.ColorFromString(getColor(x))(tl)) } lines = append(lines, "") lines = append(lines, logins...) lines = append(lines, "( <3 press ctrl-c to quit <3 )") termWidth, termHeight, err := utils.TerminalSize(out) - //termWidth, termHeight, err := terminal.GetSize(int(outFile.Fd())) if err != nil { return err } @@ -277,14 +277,14 @@ func twinkle(starLine string) string { return starLine } -func getColor(x int) func(string) string { - rainbow := []func(string) string{ - utils.Magenta, - utils.Red, - utils.Yellow, - utils.Green, - utils.Cyan, - utils.Blue, +func getColor(x int) string { + rainbow := []string{ + "magenta", + "red", + "yellow", + "green", + "cyan", + "blue", } ix := x % len(rainbow) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index b22e2501c..ddb376a17 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -125,14 +125,15 @@ func forkRun(opts *ForkOptions) error { connectedToTerminal := opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() && opts.IO.IsStdinTTY() + cs := opts.IO.ColorScheme() stderr := opts.IO.ErrOut s := utils.Spinner(stderr) stopSpinner := func() {} if connectedToTerminal { - loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...") + loading := cs.Gray("Forking ") + cs.Bold(cs.Gray(ghrepo.FullName(repoToFork))) + cs.Gray("...") s.Suffix = " " + loading - s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading)) + s.FinalMSG = cs.Gray(fmt.Sprintf("- %s\n", loading)) utils.StartSpinner(s) stopSpinner = func() { utils.StopSpinner(s) @@ -163,8 +164,8 @@ func forkRun(opts *ForkOptions) error { if createdAgo > time.Minute { if connectedToTerminal { fmt.Fprintf(stderr, "%s %s %s\n", - utils.Yellow("!"), - utils.Bold(ghrepo.FullName(forkedRepo)), + cs.Yellow("!"), + cs.Bold(ghrepo.FullName(forkedRepo)), "already exists") } else { fmt.Fprintf(stderr, "%s already exists", ghrepo.FullName(forkedRepo)) @@ -172,7 +173,7 @@ func forkRun(opts *ForkOptions) error { } } else { if connectedToTerminal { - fmt.Fprintf(stderr, "%s Created fork %s\n", utils.GreenCheck(), utils.Bold(ghrepo.FullName(forkedRepo))) + fmt.Fprintf(stderr, "%s Created fork %s\n", cs.SuccessIcon(), cs.Bold(ghrepo.FullName(forkedRepo))) } } @@ -196,7 +197,7 @@ func forkRun(opts *ForkOptions) error { } if remote, err := remotes.FindByRepo(forkedRepo.RepoOwner(), forkedRepo.RepoName()); err == nil { if connectedToTerminal { - fmt.Fprintf(stderr, "%s Using existing remote %s\n", utils.GreenCheck(), utils.Bold(remote.Name)) + fmt.Fprintf(stderr, "%s Using existing remote %s\n", cs.SuccessIcon(), cs.Bold(remote.Name)) } return nil } @@ -223,7 +224,7 @@ func forkRun(opts *ForkOptions) error { return err } if connectedToTerminal { - fmt.Fprintf(stderr, "%s Renamed %s remote to %s\n", utils.GreenCheck(), utils.Bold(remoteName), utils.Bold(renameTarget)) + fmt.Fprintf(stderr, "%s Renamed %s remote to %s\n", cs.SuccessIcon(), cs.Bold(remoteName), cs.Bold(renameTarget)) } } @@ -235,7 +236,7 @@ func forkRun(opts *ForkOptions) error { } if connectedToTerminal { - fmt.Fprintf(stderr, "%s Added remote %s\n", utils.GreenCheck(), utils.Bold(remoteName)) + fmt.Fprintf(stderr, "%s Added remote %s\n", cs.SuccessIcon(), cs.Bold(remoteName)) } } } else { @@ -260,7 +261,7 @@ func forkRun(opts *ForkOptions) error { } if connectedToTerminal { - fmt.Fprintf(stderr, "%s Cloned fork\n", utils.GreenCheck()) + fmt.Fprintf(stderr, "%s Cloned fork\n", cs.SuccessIcon()) } } } diff --git a/pkg/cmd/repo/garden/garden.go b/pkg/cmd/repo/garden/garden.go index 8ab347594..359d8d898 100644 --- a/pkg/cmd/repo/garden/garden.go +++ b/pkg/cmd/repo/garden/garden.go @@ -4,7 +4,6 @@ import ( "bytes" "errors" "fmt" - "io" "math/rand" "net/http" "os" @@ -122,6 +121,7 @@ func NewCmdGarden(f *cmdutil.Factory, runF func(*GardenOptions) error) *cobra.Co } func gardenRun(opts *GardenOptions) error { + cs := opts.IO.ColorScheme() out := opts.IO.Out if runtime.GOOS == "windows" { @@ -201,7 +201,7 @@ func gardenRun(opts *GardenOptions) error { if err != nil { return err } - player := &Player{0, 0, utils.Bold("@"), geo, 0} + player := &Player{0, 0, cs.Bold("@"), geo, 0} garden := plantGarden(commits, geo) if len(garden) < geo.Height { @@ -213,7 +213,7 @@ func gardenRun(opts *GardenOptions) error { geo.Width = 0 } clear(opts.IO) - drawGarden(out, garden, player) + drawGarden(opts.IO, garden, player) // thanks stackoverflow https://stackoverflow.com/a/17278776 _ = exec.Command("stty", sttyFileArg, "/dev/tty", "cbreak", "min", "1").Run() @@ -224,7 +224,7 @@ func gardenRun(opts *GardenOptions) error { fmt.Fprint(out, "\033[?25h") _ = exec.Command("stty", sttyFileArg, "/dev/tty", strings.TrimSpace(string(oldTTYSettings))).Run() fmt.Fprintln(out) - fmt.Fprintln(out, utils.Bold("You turn and walk away from the wildflower garden...")) + fmt.Fprintln(out, cs.Bold("You turn and walk away from the wildflower garden...")) } c := make(chan os.Signal) @@ -303,7 +303,7 @@ func gardenRun(opts *GardenOptions) error { } // status line stuff - sl := statusLine(garden, player) + sl := statusLine(garden, player, opts.IO) fmt.Fprint(out, "\033[;H") // move to top left for y := 0; y < player.Geo.Height-1; y++ { @@ -312,7 +312,7 @@ func gardenRun(opts *GardenOptions) error { fmt.Fprintln(out) fmt.Fprintln(out) - fmt.Fprint(out, utils.Bold(sl)) + fmt.Fprint(out, cs.Bold(sl)) } walkAway() @@ -423,7 +423,10 @@ func plantGarden(commits []*Commit, geo *Geometry) [][]*Cell { return garden } -func drawGarden(out io.Writer, garden [][]*Cell, player *Player) { +func drawGarden(io *iostreams.IOStreams, garden [][]*Cell, player *Player) { + out := io.Out + cs := io.ColorScheme() + fmt.Fprint(out, "\033[?25l") // hide cursor. it needs to be restored at command exit. sl := "" for y, gardenRow := range garden { @@ -432,7 +435,7 @@ func drawGarden(out io.Writer, garden [][]*Cell, player *Player) { underPlayer := (player.X == x && player.Y == y) if underPlayer { sl = gardenCell.StatusLine - char = utils.Bold(player.Char) + char = cs.Bold(player.Char) if strings.Contains(gardenCell.StatusLine, "stream") { player.ShoeMoistureContent = 5 @@ -447,20 +450,29 @@ func drawGarden(out io.Writer, garden [][]*Cell, player *Player) { } fmt.Println() - fmt.Fprintln(out, utils.Bold(sl)) + fmt.Fprintln(out, cs.Bold(sl)) } -func statusLine(garden [][]*Cell, player *Player) string { - statusLine := garden[player.Y][player.X].StatusLine + " " +func statusLine(garden [][]*Cell, player *Player, io *iostreams.IOStreams) string { + width := io.TerminalWidth() + statusLines := []string{garden[player.Y][player.X].StatusLine} + if player.ShoeMoistureContent > 1 { - statusLine += "\nYour shoes squish with water from the stream." + statusLines = append(statusLines, "Your shoes squish with water from the stream.") } else if player.ShoeMoistureContent == 1 { - statusLine += "\nYour shoes seem to have dried out." + statusLines = append(statusLines, "Your shoes seem to have dried out.") } else { - statusLine += "\n " + statusLines = append(statusLines, "") } - return statusLine + for i, line := range statusLines { + if len(line) < width { + paddingSize := width - len(line) + statusLines[i] = line + strings.Repeat(" ", paddingSize) + } + } + + return strings.Join(statusLines, "\n") } func shaToColorFunc(sha string) func(string) string { diff --git a/pkg/cmd/repo/view/http.go b/pkg/cmd/repo/view/http.go index b7e3096ce..24eaaa676 100644 --- a/pkg/cmd/repo/view/http.go +++ b/pkg/cmd/repo/view/http.go @@ -15,6 +15,7 @@ var NotFoundError = errors.New("not found") type RepoReadme struct { Filename string Content string + BaseURL string } func RepositoryReadme(client *http.Client, repo ghrepo.Interface, branch string) (*RepoReadme, error) { @@ -22,6 +23,7 @@ func RepositoryReadme(client *http.Client, repo ghrepo.Interface, branch string) var response struct { Name string Content string + HTMLURL string `json:"html_url"` } err := apiClient.REST(repo.RepoHost(), "GET", getReadmePath(repo, branch), nil, &response) @@ -41,6 +43,7 @@ func RepositoryReadme(client *http.Client, repo ghrepo.Interface, branch string) return &RepoReadme{ Filename: response.Name, Content: string(decoded), + BaseURL: response.HTMLURL, }, nil } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 6cb978e5e..94dc9a526 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -151,13 +151,15 @@ func viewRun(opts *ViewOptions) error { return err } + cs := opts.IO.ColorScheme() + var readmeContent string if readme == nil { - readmeContent = utils.Gray("This repository does not have a README") + readmeContent = cs.Gray("This repository does not have a README") } else if isMarkdownFile(readme.Filename) { var err error style := markdown.GetStyle(opts.IO.TerminalTheme()) - readmeContent, err = markdown.Render(readme.Content, style) + readmeContent, err = markdown.Render(readme.Content, style, readme.BaseURL) if err != nil { return fmt.Errorf("error rendering markdown: %w", err) } @@ -168,7 +170,7 @@ func viewRun(opts *ViewOptions) error { description := repo.Description if description == "" { - description = utils.Gray("No description provided") + description = cs.Gray("No description provided") } repoData := struct { @@ -177,10 +179,10 @@ func viewRun(opts *ViewOptions) error { Readme string View string }{ - FullName: utils.Bold(fullName), + FullName: cs.Bold(fullName), Description: description, Readme: readmeContent, - View: utils.Gray(fmt.Sprintf("View this repository on GitHub: %s", openURL)), + View: cs.Gray(fmt.Sprintf("View this repository on GitHub: %s", openURL)), } err = tmpl.Execute(stdout, repoData) diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index ae57c1328..a3be5a27a 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -6,8 +6,8 @@ import ( "strings" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/text" - "github.com/cli/cli/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" ) @@ -80,7 +80,7 @@ func isRootCmd(command *cobra.Command) bool { return command != nil && !command.HasParent() } -func rootHelpFunc(command *cobra.Command, args []string) { +func rootHelpFunc(cs *iostreams.ColorScheme, command *cobra.Command, args []string) { if isRootCmd(command.Parent()) && len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { nestedSuggestFunc(command, args[1]) hasFailed = true @@ -158,7 +158,7 @@ Read the manual at https://cli.github.com/manual`}) 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)) + fmt.Fprintln(out, cs.Bold(e.Title)) fmt.Fprintln(out, text.Indent(strings.Trim(e.Body, "\r\n"), " ")) } else { // If there is no title print the body as is diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index d88109ad3..cffd452b9 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -5,52 +5,60 @@ import ( "github.com/spf13/cobra" ) +var HelpTopics = map[string]map[string]string{ + "environment": { + "short": "Environment variables that can be used with gh", + "long": heredoc.Doc(` + GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids + being prompted to authenticate and takes precedence over previously stored credentials. + + GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. + + GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands + that otherwise operate on a local repository. + + GH_HOST: specify the GitHub hostname for commands that would otherwise assume + the "github.com" host when not in a context of an existing repository. + + GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use + for authoring text. + + BROWSER: the web browser to use for opening links. + + DEBUG: set to any value to enable verbose output to standard error. Include values "api" + or "oauth" to print detailed information about HTTP requests or authentication flow. + + GH_PAGER, PAGER (in order of precedence): a terminal paging program to send standard output to, e.g. "less". + + GLAMOUR_STYLE: the style to use for rendering Markdown. See + https://github.com/charmbracelet/glamour#styles + + NO_COLOR: set to any value to avoid printing ANSI escape sequences for color output. + + CLICOLOR: set to "0" to disable printing ANSI colors in output. + + CLICOLOR_FORCE: set to a value other than "0" to keep ANSI colors in output + even when the output is piped. + + GH_NO_UPDATE_NOTIFIER: set to any value to disable update notifications. By default, gh + checks for new releases once every 24 hours and displays an upgrade notice on standard + error if a newer version was found. + `), + }, +} + func NewHelpTopic(topic string) *cobra.Command { - topicContent := make(map[string]string) - - topicContent["environment"] = heredoc.Doc(` - GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids - being prompted to authenticate and takes precedence over previously stored credentials. - - GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. - - GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands - that otherwise operate on a local repository. - - GH_HOST: specify the GitHub hostname for commands that would otherwise assume - the "github.com" host when not in a context of an existing repository. - - GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use - for authoring text. - - BROWSER: the web browser to use for opening links. - - DEBUG: set to any value to enable verbose output to standard error. Include values "api" - or "oauth" to print detailed information about HTTP requests or authentication flow. - - GH_PAGER, PAGER (in order of precedence): a terminal paging program to send standard output to, e.g. "less". - - GLAMOUR_STYLE: the style to use for rendering Markdown. See - https://github.com/charmbracelet/glamour#styles - - NO_COLOR: set to any value to avoid printing ANSI escape sequences for color output. - - CLICOLOR: set to "0" to disable printing ANSI colors in output. - - CLICOLOR_FORCE: set to a value other than "0" to keep ANSI colors in output - even when the output is piped. - - GH_NO_UPDATE_NOTIFIER: set to any value to disable update notifications. By default, gh - checks for new releases once every 24 hours and displays an upgrade notice on standard - error if a newer version was found. - `) - cmd := &cobra.Command{ Use: topic, - Long: topicContent[topic], + Short: HelpTopics[topic]["short"], + Long: HelpTopics[topic]["long"], Hidden: true, Args: cobra.NoArgs, Run: helpTopicHelpFunc, + Annotations: map[string]string{ + "markdown:generate": "true", + "markdown:basename": "gh_help_" + topic, + }, } cmd.SetHelpFunc(helpTopicHelpFunc) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index d1b5d9a9d..906223d15 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -50,8 +50,14 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.SetOut(f.IOStreams.Out) cmd.SetErr(f.IOStreams.ErrOut) + cs := f.IOStreams.ColorScheme() + + helpHelper := func(command *cobra.Command, args []string) { + rootHelpFunc(cs, command, args) + } + cmd.PersistentFlags().Bool("help", false, "Show help for command") - cmd.SetHelpFunc(rootHelpFunc) + cmd.SetHelpFunc(helpHelper) cmd.SetUsageFunc(rootUsageFunc) cmd.SetFlagErrorFunc(rootFlagErrrorFunc) diff --git a/pkg/cmdutil/args.go b/pkg/cmdutil/args.go index c203fb691..65f3ade51 100644 --- a/pkg/cmdutil/args.go +++ b/pkg/cmdutil/args.go @@ -8,6 +8,19 @@ import ( "github.com/spf13/pflag" ) +func MinimumArgs(n int, msg string) cobra.PositionalArgs { + if msg == "" { + return cobra.MinimumNArgs(1) + } + + return func(cmd *cobra.Command, args []string) error { + if len(args) < n { + return &FlagError{Err: errors.New(msg)} + } + return nil + } +} + func NoArgsQuoteReminder(cmd *cobra.Command, args []string) error { if len(args) < 1 { return nil diff --git a/pkg/cmdutil/args_test.go b/pkg/cmdutil/args_test.go new file mode 100644 index 000000000..db0b96510 --- /dev/null +++ b/pkg/cmdutil/args_test.go @@ -0,0 +1,50 @@ +package cmdutil + +import "testing" + +func TestMinimumArgs(t *testing.T) { + tests := []struct { + N int + Args []string + }{ + { + N: 1, + Args: []string{"v1.2.3"}, + }, + { + N: 2, + Args: []string{"v1.2.3", "cli/cli"}, + }, + } + + for _, test := range tests { + if got := MinimumArgs(test.N, "")(nil, test.Args); got != nil { + t.Errorf("Got: %v, Want: (nil)", got) + } + } +} + +func TestMinimumNs_with_error(t *testing.T) { + tests := []struct { + N int + CustomMessage string + WantMessage string + }{ + { + N: 1, + CustomMessage: "A custom msg", + WantMessage: "A custom msg", + }, + { + N: 1, + CustomMessage: "", + WantMessage: "requires at least 1 arg(s), only received 0", + }, + } + + for _, test := range tests { + if got := MinimumArgs(test.N, test.CustomMessage)(nil, nil); got.Error() != test.WantMessage { + t.Errorf("Got: %v, Want: %v", got, test.WantMessage) + } + } +} diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 2efe93c8b..7e429e407 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -121,3 +121,32 @@ func (c *ColorScheme) SuccessIcon() string { func (c *ColorScheme) WarningIcon() string { return c.Yellow("!") } + +func (c *ColorScheme) ColorFromString(s string) func(string) string { + s = strings.ToLower(s) + var fn func(string) string + switch s { + case "bold": + fn = c.Bold + case "red": + fn = c.Red + case "yellow": + fn = c.Yellow + case "green": + fn = c.Green + case "gray": + fn = c.Gray + case "magenta": + fn = c.Magenta + case "cyan": + fn = c.Cyan + case "blue": + fn = c.Blue + default: + fn = func(s string) string { + return s + } + } + + return fn +} diff --git a/pkg/markdown/markdown.go b/pkg/markdown/markdown.go index c5fa46d6b..844e06811 100644 --- a/pkg/markdown/markdown.go +++ b/pkg/markdown/markdown.go @@ -7,14 +7,14 @@ import ( "github.com/charmbracelet/glamour" ) -func Render(text, style string) (string, error) { +func Render(text, style string, baseURL string) (string, error) { // Glamour rendering preserves carriage return characters in code blocks, but // we need to ensure that no such characters are present in the output. text = strings.ReplaceAll(text, "\r\n", "\n") tr, err := glamour.NewTermRenderer( glamour.WithStylePath(style), - // glamour.WithBaseURL(""), // TODO: make configurable + glamour.WithBaseURL(baseURL), // glamour.WithWordWrap(80), // TODO: make configurable ) if err != nil { diff --git a/pkg/markdown/markdown_test.go b/pkg/markdown/markdown_test.go index 079b5ff7a..7a2b3063e 100644 --- a/pkg/markdown/markdown_test.go +++ b/pkg/markdown/markdown_test.go @@ -43,7 +43,7 @@ func Test_Render(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := Render(tt.input.text, tt.input.style) + _, err := Render(tt.input.text, tt.input.style, "") if tt.output.wantsErr { assert.Error(t, err) return diff --git a/utils/color.go b/utils/color.go deleted file mode 100644 index 3e875acc4..000000000 --- a/utils/color.go +++ /dev/null @@ -1,57 +0,0 @@ -package utils - -import ( - "fmt" - "io" - "os" - - "github.com/cli/cli/pkg/iostreams" - "github.com/mattn/go-colorable" - "github.com/mgutz/ansi" -) - -var ( - // 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") -) - -// NewColorable returns an output stream that handles ANSI color sequences on Windows -func NewColorable(w io.Writer) io.Writer { - if f, isFile := w.(*os.File); isFile { - return colorable.NewColorable(f) - } - return w -} - -func makeColorFunc(color string) func(string) string { - cf := ansi.ColorFunc(color) - return func(arg string) string { - if isColorEnabled() { - if color == "black+h" && iostreams.Is256ColorSupported() { - return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, arg) - } - return cf(arg) - } - return arg - } -} - -func isColorEnabled() bool { - if iostreams.EnvColorForced() { - return true - } - - if iostreams.EnvColorDisabled() { - return false - } - - // TODO ignores cmd.OutOrStdout - return IsTerminal(os.Stdout) -} diff --git a/utils/utils.go b/utils/utils.go index 69e48c150..602b8ab1e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -97,15 +97,3 @@ func DisplayURL(urlStr string) string { } return u.Hostname() + u.Path } - -func GreenCheck() string { - return Green("✓") -} - -func YellowDash() string { - return Yellow("-") -} - -func RedX() string { - return Red("X") -}