From c903f1ecd09d585b5d5c89d671b034b07993c213 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 16 Jun 2021 01:18:52 -0400 Subject: [PATCH 01/11] Ask for and print help even if logged out You have to explicitly ask for help using the help flags. Otherwise, `gh` will just print the authentication message. --- cmd/gh/main.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 891d4b98f..8f1efd77f 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -172,6 +172,18 @@ func mainRun() exitCode { fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) fmt.Fprintln(stderr) fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + + // Print help only if you explicitly ask for help. + lastArgument := expandedArgs[len(expandedArgs)-1] + askingForHelp := lastArgument == "--help" || lastArgument == "-h" + if askingForHelp { + fmt.Fprintln(stderr) + + helpHeading := fmt.Sprintf("Printing help for `%s`...\n", cmd.UseLine()) + fmt.Fprintf(stderr, cs.Bold(helpHeading)) + + cmd.Help() + } return exitAuth } From 558ff2dff088a12a3e178d3449de38eb64de61e9 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 16 Jun 2021 14:25:08 -0400 Subject: [PATCH 02/11] Skip authentication message if asking for help Currently, this still checks authentication, but we skip the authentication message and exit normally. --- cmd/gh/main.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 8f1efd77f..76bdc8d92 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -169,21 +169,18 @@ func mainRun() exitCode { cs := cmdFactory.IOStreams.ColorScheme() if cmd != nil && cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { - fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) - fmt.Fprintln(stderr) - fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") - // Print help only if you explicitly ask for help. lastArgument := expandedArgs[len(expandedArgs)-1] askingForHelp := lastArgument == "--help" || lastArgument == "-h" if askingForHelp { - fmt.Fprintln(stderr) - - helpHeading := fmt.Sprintf("Printing help for `%s`...\n", cmd.UseLine()) - fmt.Fprintf(stderr, cs.Bold(helpHeading)) - cmd.Help() + return exitOK } + + fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr) + fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + return exitAuth } From 1e3bba5ff64c1bfba52295d0e38acd38d4ed78b4 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Wed, 16 Jun 2021 22:28:41 -0400 Subject: [PATCH 03/11] Add comment about cmd.Help() The linter picked up that the error value from cmd.Help() isn't checked. Even though cmd.Help() returns an error value, it's always nil. The inner HelpFunc() function directly prints the error message instead of returning an error value. --- cmd/gh/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 76bdc8d92..7aaedf09a 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -173,6 +173,10 @@ func mainRun() exitCode { lastArgument := expandedArgs[len(expandedArgs)-1] askingForHelp := lastArgument == "--help" || lastArgument == "-h" if askingForHelp { + // Even though cmd.Help() returns an error, the error value is + // always nil. The inner function HelpFunc() prints an error + // directly if something's wrong with the text template, rather than + // returning the error value. cmd.Help() return exitOK } From 883943946a377547dba322cebabf9ed76b576ad3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Jun 2021 15:33:07 +0200 Subject: [PATCH 04/11] Add a global pre-run hook to handle auth check and repo override With auth check being done via Cobra hooks, it is automatically skipped for non-runnable commands and `-h/--help` flag usage. --- cmd/gh/main.go | 39 ++++++++++++++++++++++-------------- pkg/cmdutil/auth_check.go | 5 +---- pkg/cmdutil/repo_override.go | 16 --------------- 3 files changed, 25 insertions(+), 35 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 7aaedf09a..2d0fcbf66 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -168,24 +168,31 @@ func mainRun() exitCode { cs := cmdFactory.IOStreams.ColorScheme() - if cmd != nil && cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { - // Print help only if you explicitly ask for help. - lastArgument := expandedArgs[len(expandedArgs)-1] - askingForHelp := lastArgument == "--help" || lastArgument == "-h" - if askingForHelp { - // Even though cmd.Help() returns an error, the error value is - // always nil. The inner function HelpFunc() prints an error - // directly if something's wrong with the text template, rather than - // returning the error value. - cmd.Help() - return exitOK + authError := errors.New("authError") + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + // require that the user is authenticated before running most commands + if cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { + fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) + fmt.Fprintln(stderr) + fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + return authError } - fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) - fmt.Fprintln(stderr) - fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") + // enable `--repo` override + if cmd.Flags().Lookup("repo") != nil { + repoOverride, _ := cmd.Flags().GetString("repo") + if repoFromEnv := os.Getenv("GH_REPO"); repoOverride == "" && repoFromEnv != "" { + repoOverride = repoFromEnv + } + if repoOverride != "" { + // FIXME: reimplement the repo override to avoid having to mutate the factory + cmdFactory.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName(repoOverride) + } + } + } - return exitAuth + return nil } rootCmd.SetArgs(expandedArgs) @@ -199,6 +206,8 @@ func mainRun() exitCode { fmt.Fprint(stderr, "\n") } return exitCancel + } else if errors.Is(err, authError) { + return exitAuth } printError(stderr, err, cmd, hasDebug) diff --git a/pkg/cmdutil/auth_check.go b/pkg/cmdutil/auth_check.go index 10df9fade..ab536f6b2 100644 --- a/pkg/cmdutil/auth_check.go +++ b/pkg/cmdutil/auth_check.go @@ -5,9 +5,6 @@ import ( "github.com/spf13/cobra" ) -// TODO can have this set a PersistentPreRun so we don't have to set for all child commands of auth, -// config - func DisableAuthCheck(cmd *cobra.Command) { if cmd.Annotations == nil { cmd.Annotations = map[string]string{} @@ -37,7 +34,7 @@ func CheckAuth(cfg config.Config) bool { } func IsAuthCheckEnabled(cmd *cobra.Command) bool { - if !cmd.Runnable() { + if cmd.Name() == "help" { return false } for c := cmd; c.Parent() != nil; c = c.Parent() { diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 8b3d36489..783955e58 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -1,25 +1,9 @@ package cmdutil import ( - "os" - - "github.com/cli/cli/internal/ghrepo" "github.com/spf13/cobra" ) func EnableRepoOverride(cmd *cobra.Command, f *Factory) { cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `[HOST/]OWNER/REPO` format") - - cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { - repoOverride, _ := cmd.Flags().GetString("repo") - if repoFromEnv := os.Getenv("GH_REPO"); repoOverride == "" && repoFromEnv != "" { - repoOverride = repoFromEnv - } - if repoOverride != "" { - // NOTE: this mutates the factory - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName(repoOverride) - } - } - } } From 94c1646209d5cb9a95918ceb8647f54918b4a312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Jun 2021 16:02:00 +0200 Subject: [PATCH 05/11] Simplify `gh actions` implemenation The command is now non-runnable, meaning it's exempt from auth check. --- pkg/cmd/actions/actions.go | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 356ca1581..b7c9cf625 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -9,27 +9,20 @@ import ( "github.com/spf13/cobra" ) -type ActionsOptions struct { - IO *iostreams.IOStreams -} - func NewCmdActions(f *cmdutil.Factory) *cobra.Command { - opts := ActionsOptions{ - IO: f.IOStreams, - } - cmd := &cobra.Command{ Use: "actions", Short: "Learn about working with GitHub actions", - Long: actionsExplainer(nil), - Run: func(cmd *cobra.Command, args []string) { - actionsRun(opts) - }, Annotations: map[string]string{ "IsActions": "true", }, } + cmd.SetHelpFunc(func(c *cobra.Command, s []string) { + cs := f.IOStreams.ColorScheme() + fmt.Fprintln(f.IOStreams.Out, actionsExplainer(cs)) + }) + return cmd } @@ -70,8 +63,3 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { `, header, runHeader, workflowHeader) } - -func actionsRun(opts ActionsOptions) { - cs := opts.IO.ColorScheme() - fmt.Fprintln(opts.IO.Out, actionsExplainer(cs)) -} From 8ff42bf28c2d0fca0f5be893318db4418f225de2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 17 Jun 2021 17:58:46 +0200 Subject: [PATCH 06/11] Fix repo override --- cmd/gh/main.go | 6 ------ pkg/cmdutil/repo_override.go | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 578ba3767..236943475 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -169,12 +169,6 @@ func mainRun() exitCode { return authError } - // enable `--repo` override - if cmd.Flags().Lookup("repo") != nil { - repoOverride, _ := cmd.Flags().GetString("repo") - cmdFactory.BaseRepo = cmdutil.OverrideBaseRepoFunc(cmdFactory, repoOverride) - } - return nil } diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 8ae5a4760..6ce68011b 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -7,8 +7,27 @@ import ( "github.com/spf13/cobra" ) +func executeParentHooks(cmd *cobra.Command, args []string) error { + for cmd.HasParent() { + cmd = cmd.Parent() + if cmd.PersistentPreRunE != nil { + return cmd.PersistentPreRunE(cmd, args) + } + } + return nil +} + func EnableRepoOverride(cmd *cobra.Command, f *Factory) { cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `[HOST/]OWNER/REPO` format") + + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + if err := executeParentHooks(cmd, args); err != nil { + return err + } + repoOverride, _ := cmd.Flags().GetString("repo") + f.BaseRepo = OverrideBaseRepoFunc(f, repoOverride) + return nil + } } func OverrideBaseRepoFunc(f *Factory, override string) func() (ghrepo.Interface, error) { From 682c15d52c387e4af9d065ce8c536a45f813a6a7 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Thu, 17 Jun 2021 12:48:53 -0400 Subject: [PATCH 07/11] Whitespace --- pkg/cmd/actions/actions.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index b7c9cf625..69af75993 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -41,20 +41,20 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { GitHub CLI integrates with Actions to help you manage runs and workflows. - %s - gh run list: List recent workflow runs - gh run view: View details for a workflow run or one of its jobs - gh run watch: Watch a workflow run while it executes - gh run rerun: Rerun a failed workflow run + %s + gh run list: List recent workflow runs + gh run view: View details for a workflow run or one of its jobs + gh run watch: Watch a workflow run while it executes + gh run rerun: Rerun a failed workflow run gh run download: Download artifacts generated by runs To see more help, run 'gh help run ' - %s - gh workflow list: List all the workflow files in your repository - gh workflow view: View details for a workflow file - gh workflow enable: Enable a workflow file - gh workflow disable: Disable a workflow file + %s + gh workflow list: List all the workflow files in your repository + gh workflow view: View details for a workflow file + gh workflow enable: Enable a workflow file + gh workflow disable: Disable a workflow file gh workflow run: Trigger a workflow_dispatch run for a workflow file To see more help, run 'gh help workflow ' From 89ce78e48be593058ec2035d5f6dbe78c2fc706d Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Thu, 17 Jun 2021 13:30:26 -0400 Subject: [PATCH 08/11] Restore help footer At the moment, the "help footer" doesn't add any new information, but if additional flags are added later, they should appear in the footer. This change restores this help footer: ```shell USAGE gh actions [flags] INHERITED FLAGS --help Show help for command LEARN MORE Use 'gh --help' for more information about a command. Read the manual at https://cli.github.com/manual ``` --- pkg/cmd/actions/actions.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 69af75993..b9b17a14a 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -1,8 +1,6 @@ package actions import ( - "fmt" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -10,19 +8,17 @@ import ( ) func NewCmdActions(f *cmdutil.Factory) *cobra.Command { + cs := f.IOStreams.ColorScheme() + cmd := &cobra.Command{ Use: "actions", Short: "Learn about working with GitHub actions", + Long: actionsExplainer(cs), Annotations: map[string]string{ "IsActions": "true", }, } - cmd.SetHelpFunc(func(c *cobra.Command, s []string) { - cs := f.IOStreams.ColorScheme() - fmt.Fprintln(f.IOStreams.Out, actionsExplainer(cs)) - }) - return cmd } From 052d6588eaa505b500fc40614aa56cdb9d99b366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 18 Jun 2021 15:16:48 +0200 Subject: [PATCH 09/11] Revert "Whitespace" Trailing whitespace is significant in Markdown. This reverts commit 682c15d52c387e4af9d065ce8c536a45f813a6a7. --- pkg/cmd/actions/actions.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index b9b17a14a..5cdbae14f 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -37,20 +37,20 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { GitHub CLI integrates with Actions to help you manage runs and workflows. - %s - gh run list: List recent workflow runs - gh run view: View details for a workflow run or one of its jobs - gh run watch: Watch a workflow run while it executes - gh run rerun: Rerun a failed workflow run + %s + gh run list: List recent workflow runs + gh run view: View details for a workflow run or one of its jobs + gh run watch: Watch a workflow run while it executes + gh run rerun: Rerun a failed workflow run gh run download: Download artifacts generated by runs To see more help, run 'gh help run ' - %s - gh workflow list: List all the workflow files in your repository - gh workflow view: View details for a workflow file - gh workflow enable: Enable a workflow file - gh workflow disable: Disable a workflow file + %s + gh workflow list: List all the workflow files in your repository + gh workflow view: View details for a workflow file + gh workflow enable: Enable a workflow file + gh workflow disable: Disable a workflow file gh workflow run: Trigger a workflow_dispatch run for a workflow file To see more help, run 'gh help workflow ' From 1c103e20ac0ef0f69a503dd523e3a5cdc47944e8 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 18 Jun 2021 23:07:31 -0400 Subject: [PATCH 10/11] Always try to render bold font It looks like a similar check is done in ColorScheme.Bold() where it checks whether the scheme is enabled or not. --- pkg/cmd/actions/actions.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 5cdbae14f..dd731d0ce 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -23,14 +23,9 @@ func NewCmdActions(f *cmdutil.Factory) *cobra.Command { } func actionsExplainer(cs *iostreams.ColorScheme) string { - header := "Welcome to GitHub Actions on the command line." - runHeader := "Interacting with workflow runs" - workflowHeader := "Interacting with workflow files" - if cs != nil { - header = cs.Bold(header) - runHeader = cs.Bold(runHeader) - workflowHeader = cs.Bold(workflowHeader) - } + header := cs.Bold("Welcome to GitHub Actions on the command line.") + runHeader := cs.Bold("Interacting with workflow runs") + workflowHeader := cs.Bold("Interacting with workflow files") return heredoc.Docf(` %s From b0f58d0cf05ecc9255568eb7550d6fd88c047f92 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Fri, 18 Jun 2021 23:17:41 -0400 Subject: [PATCH 11/11] Disable authentication check, but keep runnable In this branch, we originally avoided the authentication check by getting rid of the run method attached to the command. Instead of that, this commit makes the `gh actions` command runnable again, but the authentication is disabled with `cmdutil.DisableAuthCheck`; this mirrors what's done for `gh version`. `gh actions` and `gh actions [-h | --help]` all work while being logged out. In addition, this commit restores some original behavior. Before this commit, the help footer (usage, inherited flags, etc.) is appended whether you use `gh actions` or `gh actions --help`. This commit restores the original behavior where `gh actions` prints just the text for the actions explanation, but `gh actions --help` appends the help footer. --- pkg/cmd/actions/actions.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index dd731d0ce..7880a9a6c 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -1,6 +1,8 @@ package actions import ( + "fmt" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -14,11 +16,16 @@ func NewCmdActions(f *cmdutil.Factory) *cobra.Command { Use: "actions", Short: "Learn about working with GitHub actions", Long: actionsExplainer(cs), + Run: func(cmd *cobra.Command, args []string) { + fmt.Fprintln(f.IOStreams.Out, actionsExplainer(cs)) + }, Annotations: map[string]string{ "IsActions": "true", }, } + cmdutil.DisableAuthCheck(cmd) + return cmd }