WIP eliminate package-level state in commands, context

mislav · mislav · commit 8370602f4910 · 2019-10-25T21:49:08.000+02:00
diff --git a/command/pr.go b/command/pr.go
@@ -5,7 +5,6 @@ import (
 	"strconv"
 
 	"github.com/github/gh-cli/api"
-	"github.com/github/gh-cli/context"
 	"github.com/github/gh-cli/utils"
 	"github.com/spf13/cobra"
 )
@@ -38,6 +37,7 @@ work with pull requests.`,
 }
 
 func prList(cmd *cobra.Command, args []string) error {
+	ctx := contextForCommand(cmd)
 	prPayload, err := api.PullRequests()
 	if err != nil {
 		return err
@@ -47,7 +47,7 @@ func prList(cmd *cobra.Command, args []string) error {
 	if prPayload.CurrentPR != nil {
 		printPrs(*prPayload.CurrentPR)
 	} else {
-		currentBranch, err := context.Current().Branch()
+		currentBranch, err := ctx.Branch()
 		if err != nil {
 			return err
 		}
@@ -76,7 +76,8 @@ func prList(cmd *cobra.Command, args []string) error {
 }
 
 func prView(cmd *cobra.Command, args []string) error {
-	baseRepo, err := context.Current().BaseRepo()
+	ctx := contextForCommand(cmd)
+	baseRepo, err := ctx.BaseRepo()
 	if err != nil {
 		return err
 	}
@@ -94,7 +95,7 @@ func prView(cmd *cobra.Command, args []string) error {
 		if err != nil {
 			return err
 		} else if prPayload.CurrentPR == nil {
-			branch, err := context.Current().Branch()
+			branch, err := ctx.Branch()
 			if err != nil {
 				return err
 			}
diff --git a/command/root.go b/command/root.go
@@ -5,27 +5,13 @@ import (
 	"os"
 
 	"github.com/github/gh-cli/context"
-	"github.com/spf13/cobra"
-)
 
-var (
-	currentRepo   string
-	currentBranch string
+	"github.com/spf13/cobra"
 )
 
 func init() {
-	RootCmd.PersistentFlags().StringVarP(&currentRepo, "repo", "R", "", "current GitHub repository")
-	RootCmd.PersistentFlags().StringVarP(&currentBranch, "current-branch", "B", "", "current git branch")
-}
-
-func initContext() {
-	ctx := context.InitDefaultContext()
-	ctx.SetBranch(currentBranch)
-	repo := currentRepo
-	if repo == "" {
-		repo = os.Getenv("GH_REPO")
-	}
-	ctx.SetBaseRepo(repo)
+	RootCmd.PersistentFlags().StringP("repo", "R", "", "current GitHub repository")
+	RootCmd.PersistentFlags().StringP("current-branch", "B", "", "current git branch")
 }
 
 // RootCmd is the entry point of command-line execution
@@ -34,10 +20,21 @@ var RootCmd = &cobra.Command{
 	Short: "GitHub CLI",
 	Long:  `Do things with GitHub from your terminal`,
 	Args:  cobra.MinimumNArgs(1),
-	PersistentPreRun: func(cmd *cobra.Command, args []string) {
-		initContext()
-	},
 	Run: func(cmd *cobra.Command, args []string) {
 		fmt.Println("root")
 	},
 }
+
+func contextForCommand(cmd *cobra.Command) context.Context {
+	ctx := context.New()
+	if repo := os.Getenv("GH_REPO"); repo != "" {
+		ctx.SetBaseRepo(repo)
+	}
+	if repo, err := cmd.Flags().GetString("repo"); err == nil {
+		ctx.SetBaseRepo(repo)
+	}
+	if branch, err := cmd.Flags().GetString("current-branch"); err == nil {
+		ctx.SetBranch(branch)
+	}
+	return ctx
+}
diff --git a/context/blank_context.go b/context/blank_context.go
@@ -5,13 +5,8 @@ import (
 	"strings"
 )
 
-// InitBlankContext initializes a blank context for testing
-func InitBlankContext() Context {
-	currentContext = &blankContext{
-		authToken: "OTOKEN",
-		authLogin: "monalisa",
-	}
-	return currentContext
+func NewBlank() Context {
+	return &blankContext{}
 }
 
 // A Context implementation that queries the filesystem
diff --git a/context/context.go b/context/context.go
@@ -19,20 +19,8 @@ type Context interface {
 	SetBaseRepo(string)
 }
 
-var currentContext Context
-
-// Current returns the currently initialized Context instance
-func Current() Context {
-	return currentContext
-}
-
-// InitDefaultContext initializes the default filesystem context
-func InitDefaultContext() Context {
-	ctx := &fsContext{}
-	if currentContext == nil {
-		currentContext = ctx
-	}
-	return ctx
+func New() Context {
+	return &blankContext{}
 }
 
 // A Context implementation that queries the filesystem

Original file line number	Diff line number	Diff line change
`@@ -5,13 +5,8 @@ import (`
`5`	`5`	`"strings"`
`6`	`6`	`)`
`7`	`7`
`8`		`-// InitBlankContext initializes a blank context for testing`
`9`		`-func InitBlankContext() Context {`
`10`		`- currentContext = &blankContext{`
`11`		`- authToken: "OTOKEN",`
`12`		`- authLogin: "monalisa",`
`13`		`- }`
`14`		`- return currentContext`
	`8`	`+func NewBlank() Context {`
	`9`	`+ return &blankContext{}`
`15`	`10`	`}`
`16`	`11`
`17`	`12`	`// A Context implementation that queries the filesystem`