From 30c25f325f03c5bba3b1c88a3e6723fc953c20b6 Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Mon, 29 Jul 2019 23:32:32 -0700 Subject: [PATCH 1/6] Add ignore of cobra posix binary and all of intellij generated files --- .gitignore | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 3b053c59e..b2b848e77 100644 --- a/.gitignore +++ b/.gitignore @@ -32,7 +32,8 @@ Session.vim tags *.exe - +cobra cobra.test -.idea/* +.idea/ +*.iml From d0edbca1431d5df2ade6e548fde33f63e62a6d1c Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Mon, 29 Jul 2019 23:34:42 -0700 Subject: [PATCH 2/6] Add idiomatic handling of go error in distinct main func --- cobra/cmd/root.go | 4 ++-- cobra/main.go | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cobra/cmd/root.go b/cobra/cmd/root.go index 624c717c1..97f404bbb 100644 --- a/cobra/cmd/root.go +++ b/cobra/cmd/root.go @@ -36,8 +36,8 @@ to quickly create a Cobra application.`, ) // Execute executes the root command. -func Execute() { - rootCmd.Execute() +func Execute() error { + return rootCmd.Execute() } func init() { diff --git a/cobra/main.go b/cobra/main.go index c3a9d9cb0..8add69a88 100644 --- a/cobra/main.go +++ b/cobra/main.go @@ -13,8 +13,18 @@ package main -import "github.com/spf13/cobra/cobra/cmd" +import ( + "os" + + "github.com/spf13/cobra/cobra/cmd" +) func main() { - cmd.Execute() + if err := runMain(); err != nil { + os.Exit(1) + } +} + +func runMain() error { + return cmd.Execute() } From 8b780f474d2a5ca401a3d11ebdbb6d5f90afa56f Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Mon, 29 Jul 2019 23:36:50 -0700 Subject: [PATCH 3/6] Return an error in the case of unrunnable subcommand * credit to @chriswhelix for initial commit --- command.go | 16 ++++++++++++++-- command_test.go | 4 ++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/command.go b/command.go index c7e898303..05fd9f34d 100644 --- a/command.go +++ b/command.go @@ -17,6 +17,7 @@ package cobra import ( "bytes" + "errors" "fmt" "io" "os" @@ -27,6 +28,9 @@ import ( flag "github.com/spf13/pflag" ) +// NotRunnable defines subcommand error +var NotRunnable = errors.New("subcommand is required") + // FParseErrWhitelist configures Flag parse errors to be ignored type FParseErrWhitelist flag.ParseErrorsWhitelist @@ -369,7 +373,7 @@ func (c *Command) HelpFunc() func(*Command, []string) { } return func(c *Command, a []string) { c.mergePersistentFlags() - err := tmpl(c.OutOrStdout(), c.HelpTemplate(), c) + err := tmpl(c.OutOrStderr(), c.HelpTemplate(), c) if err != nil { c.Println(err) } @@ -786,7 +790,7 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - return flag.ErrHelp + return NotRunnable } c.preRun() @@ -920,6 +924,14 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, nil } + // If command wasn't runnable, show full help, but do return the error. + // This will result in apps by default returning a non-success exit code, but also gives them the option to + // handle specially. + if err == NotRunnable { + cmd.HelpFunc()(cmd, args) + return cmd, err + } + // If root command has SilentErrors flagged, // all subcommands should respect it if !cmd.SilenceErrors && !c.SilenceErrors { diff --git a/command_test.go b/command_test.go index 2fa2003cb..e7961c8f6 100644 --- a/command_test.go +++ b/command_test.go @@ -836,8 +836,8 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) { rootCmd.AddCommand(childCmd) output, err := executeCommand(rootCmd, "child") - if err != nil { - t.Errorf("Unexpected error: %v", err) + if err != NotRunnable { + t.Errorf("Expected error") } checkStringContains(t, output, childCmd.Long) From 15a2441afc76dfa919cbff4d52ff3af005c848ee Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Tue, 30 Jul 2019 15:26:11 -0700 Subject: [PATCH 4/6] Correct all complaints from golint * i.e. * go get golang.org/x/lint/golint * go list ./... | xargs golint --- args.go | 1 + cobra.go | 4 ++-- cobra/cmd/project.go | 7 +++++-- cobra/tpl/main.go | 3 +++ command.go | 13 +++++++------ command_test.go | 2 +- 6 files changed, 19 insertions(+), 11 deletions(-) diff --git a/args.go b/args.go index c4d820b85..e5a34c167 100644 --- a/args.go +++ b/args.go @@ -4,6 +4,7 @@ import ( "fmt" ) +// PositionalArgs defines positional arguments callback type PositionalArgs func(cmd *Command, args []string) error // Legacy arg validation has the following behaviour: diff --git a/cobra.go b/cobra.go index 6505c070b..d01becc8f 100644 --- a/cobra.go +++ b/cobra.go @@ -52,7 +52,7 @@ var EnableCommandSorting = true // if the CLI is started from explorer.exe. // To disable the mousetrap, just set this variable to blank string (""). // Works only on Microsoft Windows. -var MousetrapHelpText string = `This is a command line tool. +var MousetrapHelpText = `This is a command line tool. You need to open cmd.exe and run it from there. ` @@ -61,7 +61,7 @@ You need to open cmd.exe and run it from there. // if the CLI is started from explorer.exe. Set to 0 to wait for the return key to be pressed. // To disable the mousetrap, just set MousetrapHelpText to blank string (""). // Works only on Microsoft Windows. -var MousetrapDisplayDuration time.Duration = 5 * time.Second +var MousetrapDisplayDuration = 5 * time.Second // AddTemplateFunc adds a template function that's available to Usage and Help // template generation. diff --git a/cobra/cmd/project.go b/cobra/cmd/project.go index dd2f7ea2f..a71cae317 100644 --- a/cobra/cmd/project.go +++ b/cobra/cmd/project.go @@ -2,9 +2,10 @@ package cmd import ( "fmt" - "github.com/spf13/cobra/cobra/tpl" "os" "text/template" + + "github.com/spf13/cobra/cobra/tpl" ) // Project contains name, license and paths to projects. @@ -18,14 +19,15 @@ type Project struct { AppName string } +// Command structure type Command struct { CmdName string CmdParent string *Project } +// Create project receiver func (p *Project) Create() error { - // check if AbsolutePath exists if _, err := os.Stat(p.AbsolutePath); os.IsNotExist(err) { // create directory @@ -80,6 +82,7 @@ func (p *Project) createLicenseFile() error { return licenseTemplate.Execute(licenseFile, data) } +// Create command receiver func (c *Command) Create() error { cmdFile, err := os.Create(fmt.Sprintf("%s/cmd/%s.go", c.AbsolutePath, c.CmdName)) if err != nil { diff --git a/cobra/tpl/main.go b/cobra/tpl/main.go index 5e5a0fae2..f6fe8977e 100644 --- a/cobra/tpl/main.go +++ b/cobra/tpl/main.go @@ -1,5 +1,6 @@ package tpl +// MainTemplate defines main template string func MainTemplate() []byte { return []byte(`/* {{ .Copyright }} @@ -15,6 +16,7 @@ func main() { `) } +// RootTemplate defines root template string func RootTemplate() []byte { return []byte(`/* {{ .Copyright }} @@ -108,6 +110,7 @@ func initConfig() { `) } +// AddCommandTemplate defines add command template string func AddCommandTemplate() []byte { return []byte(`/* {{ .Project.Copyright }} diff --git a/command.go b/command.go index 05fd9f34d..ca6dc1395 100644 --- a/command.go +++ b/command.go @@ -28,8 +28,8 @@ import ( flag "github.com/spf13/pflag" ) -// NotRunnable defines subcommand error -var NotRunnable = errors.New("subcommand is required") +// ErrSubCommandRequired defines subcommand error +var ErrSubCommandRequired = errors.New("subcommand is required") // FParseErrWhitelist configures Flag parse errors to be ignored type FParseErrWhitelist flag.ParseErrorsWhitelist @@ -232,7 +232,7 @@ func (c *Command) SetErr(newErr io.Writer) { c.errWriter = newErr } -// SetOut sets the source for input data +// SetIn sets the source for input data // If newIn is nil, os.Stdin is used. func (c *Command) SetIn(newIn io.Reader) { c.inReader = newIn @@ -301,7 +301,7 @@ func (c *Command) ErrOrStderr() io.Writer { return c.getErr(os.Stderr) } -// ErrOrStderr returns output to stderr +// InOrStdin returns output to stderr func (c *Command) InOrStdin() io.Reader { return c.getIn(os.Stdin) } @@ -790,7 +790,7 @@ func (c *Command) execute(a []string) (err error) { } if !c.Runnable() { - return NotRunnable + return ErrSubCommandRequired } c.preRun() @@ -927,7 +927,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { // If command wasn't runnable, show full help, but do return the error. // This will result in apps by default returning a non-success exit code, but also gives them the option to // handle specially. - if err == NotRunnable { + if err == ErrSubCommandRequired { cmd.HelpFunc()(cmd, args) return cmd, err } @@ -947,6 +947,7 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } +// ValidateArgs validates arguments func (c *Command) ValidateArgs(args []string) error { if c.Args == nil { return nil diff --git a/command_test.go b/command_test.go index e7961c8f6..b26bd4abe 100644 --- a/command_test.go +++ b/command_test.go @@ -836,7 +836,7 @@ func TestHelpExecutedOnNonRunnableChild(t *testing.T) { rootCmd.AddCommand(childCmd) output, err := executeCommand(rootCmd, "child") - if err != NotRunnable { + if err != ErrSubCommandRequired { t.Errorf("Expected error") } From 40b62f616f2fc0406bc0e3f0a66006fc830a0f11 Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Tue, 30 Jul 2019 15:32:58 -0700 Subject: [PATCH 5/6] Correct all complaints from goimports * i.e. * go get golang.org/x/tools/cmd/goimports * goimports -w *.go * goimports -w cobra/ --- cobra/cmd/init.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cobra/cmd/init.go b/cobra/cmd/init.go index 63397d119..dcf5ada4f 100644 --- a/cobra/cmd/init.go +++ b/cobra/cmd/init.go @@ -15,10 +15,11 @@ package cmd import ( "fmt" - "github.com/spf13/cobra" - "github.com/spf13/viper" "os" "path" + + "github.com/spf13/cobra" + "github.com/spf13/viper" ) var ( From dfb2abaa2bc1db30e6ea7ee65ff83761d40b7362 Mon Sep 17 00:00:00 2001 From: Bruce Downs Date: Thu, 1 Aug 2019 09:47:14 -0700 Subject: [PATCH 6/6] Adjustments per PR review feedback from @bogem --- args.go | 1 - cobra/cmd/project.go | 3 --- cobra/main.go | 6 +----- cobra/tpl/main.go | 3 --- command.go | 2 -- 5 files changed, 1 insertion(+), 14 deletions(-) diff --git a/args.go b/args.go index e5a34c167..c4d820b85 100644 --- a/args.go +++ b/args.go @@ -4,7 +4,6 @@ import ( "fmt" ) -// PositionalArgs defines positional arguments callback type PositionalArgs func(cmd *Command, args []string) error // Legacy arg validation has the following behaviour: diff --git a/cobra/cmd/project.go b/cobra/cmd/project.go index a71cae317..a53893ccd 100644 --- a/cobra/cmd/project.go +++ b/cobra/cmd/project.go @@ -19,14 +19,12 @@ type Project struct { AppName string } -// Command structure type Command struct { CmdName string CmdParent string *Project } -// Create project receiver func (p *Project) Create() error { // check if AbsolutePath exists if _, err := os.Stat(p.AbsolutePath); os.IsNotExist(err) { @@ -82,7 +80,6 @@ func (p *Project) createLicenseFile() error { return licenseTemplate.Execute(licenseFile, data) } -// Create command receiver func (c *Command) Create() error { cmdFile, err := os.Create(fmt.Sprintf("%s/cmd/%s.go", c.AbsolutePath, c.CmdName)) if err != nil { diff --git a/cobra/main.go b/cobra/main.go index 8add69a88..eeaf9824e 100644 --- a/cobra/main.go +++ b/cobra/main.go @@ -20,11 +20,7 @@ import ( ) func main() { - if err := runMain(); err != nil { + if err := cmd.Execute(); err != nil { os.Exit(1) } } - -func runMain() error { - return cmd.Execute() -} diff --git a/cobra/tpl/main.go b/cobra/tpl/main.go index f6fe8977e..5e5a0fae2 100644 --- a/cobra/tpl/main.go +++ b/cobra/tpl/main.go @@ -1,6 +1,5 @@ package tpl -// MainTemplate defines main template string func MainTemplate() []byte { return []byte(`/* {{ .Copyright }} @@ -16,7 +15,6 @@ func main() { `) } -// RootTemplate defines root template string func RootTemplate() []byte { return []byte(`/* {{ .Copyright }} @@ -110,7 +108,6 @@ func initConfig() { `) } -// AddCommandTemplate defines add command template string func AddCommandTemplate() []byte { return []byte(`/* {{ .Project.Copyright }} diff --git a/command.go b/command.go index ca6dc1395..42e500de5 100644 --- a/command.go +++ b/command.go @@ -28,7 +28,6 @@ import ( flag "github.com/spf13/pflag" ) -// ErrSubCommandRequired defines subcommand error var ErrSubCommandRequired = errors.New("subcommand is required") // FParseErrWhitelist configures Flag parse errors to be ignored @@ -947,7 +946,6 @@ func (c *Command) ExecuteC() (cmd *Command, err error) { return cmd, err } -// ValidateArgs validates arguments func (c *Command) ValidateArgs(args []string) error { if c.Args == nil { return nil