From 883943946a377547dba322cebabf9ed76b576ad3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mislav=20Marohni=C4=87?= <mislav@github.com>
Date: Thu, 17 Jun 2021 15:33:07 +0200
Subject: [PATCH] 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)
-			}
-		}
-	}
 }