From 0083ae8732e4b5d6911729b5e60c99a19d6179ac Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 9 Jan 2017 15:57:49 -0800 Subject: [PATCH 1/5] Usage/Description/ArgsUsage correctly copied when using subcommand --- command.go | 8 +++----- help.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/command.go b/command.go index 2628fbf..68c760e 100644 --- a/command.go +++ b/command.go @@ -230,11 +230,9 @@ func (c Command) startApp(ctx *Context) error { app.HelpName = app.Name } - if c.Description != "" { - app.Usage = c.Description - } else { - app.Usage = c.Usage - } + app.Usage = c.Usage + app.Description = c.Description + app.ArgsUsage = c.ArgsUsage // set CommandNotFound app.CommandNotFound = ctx.App.CommandNotFound diff --git a/help.go b/help.go index c8c1aee..d00e4da 100644 --- a/help.go +++ b/help.go @@ -64,7 +64,7 @@ OPTIONS: // cli.go uses text/template to render templates. You can // render custom help text by setting this variable. var SubcommandHelpTemplate = `NAME: - {{.HelpName}} - {{.Usage}} + {{.HelpName}} - {{if .Description}}{{.Description}}{{else}}{{.Usage}}{{end}} USAGE: {{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} From 4ed366e2011dfb9efa3399c709af0976d5e87db4 Mon Sep 17 00:00:00 2001 From: Robert Bittle Date: Wed, 18 Jan 2017 23:29:26 -0500 Subject: [PATCH 2/5] Pass the ErrWriter on the root app to subcommands --- command.go | 1 + command_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/command.go b/command.go index d297eb9..4a409d4 100644 --- a/command.go +++ b/command.go @@ -264,6 +264,7 @@ func (c Command) startApp(ctx *Context) error { app.Author = ctx.App.Author app.Email = ctx.App.Email app.Writer = ctx.App.Writer + app.ErrWriter = ctx.App.ErrWriter app.categories = CommandCategories{} for _, command := range c.Subcommands { diff --git a/command_test.go b/command_test.go index 5e0e8de..5710184 100644 --- a/command_test.go +++ b/command_test.go @@ -153,3 +153,32 @@ func TestCommand_OnUsageError_WithWrongFlagValue(t *testing.T) { t.Errorf("Expect an intercepted error, but got \"%v\"", err) } } + +func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) { + app := NewApp() + app.ErrWriter = ioutil.Discard + app.Commands = []Command{ + { + Name: "bar", + Usage: "this is for testing", + Subcommands: []Command{ + { + Name: "baz", + Usage: "this is for testing", + Action: func(c *Context) error { + if c.App.ErrWriter != ioutil.Discard { + return fmt.Errorf("ErrWriter not passed") + } + + return nil + }, + }, + }, + }, + } + + err := app.Run([]string{"foo", "bar", "baz"}) + if err != nil { + t.Fatal(err) + } +} From 2526b57c56f30b50466c96c4133b1a4ad0f0191f Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 14 Feb 2017 21:17:05 -0800 Subject: [PATCH 3/5] Allow slightly longer lines in Python scripts --- .flake8 | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..6deafc2 --- /dev/null +++ b/.flake8 @@ -0,0 +1,2 @@ +[flake8] +max-line-length = 120 From 8d8f927bcb0447918aaa09c5b87160ddf2e5842b Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 4 Mar 2017 14:28:24 -0800 Subject: [PATCH 4/5] Change flag test error checking to use regexp rather than strings As the error messages changed in 1.8 --- flag_test.go | 92 +++++++++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/flag_test.go b/flag_test.go index 0dd8654..1ccb639 100644 --- a/flag_test.go +++ b/flag_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "reflect" + "regexp" "runtime" "strings" "testing" @@ -31,57 +32,57 @@ func TestBoolFlagHelpOutput(t *testing.T) { func TestFlagsFromEnv(t *testing.T) { var flagTests = []struct { - input string - output interface{} - flag Flag - err error + input string + output interface{} + flag Flag + errRegexp string }{ - {"", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"1", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"false", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"foobar", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Errorf(`could not parse foobar as bool value for flag debug: strconv.ParseBool: parsing "foobar": invalid syntax`)}, + {"", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"1", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"false", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"foobar", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Sprintf(`could not parse foobar as bool value for flag debug: .*`)}, - {"", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"1", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"false", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"foobar", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Errorf(`could not parse foobar as bool value for flag debug: strconv.ParseBool: parsing "foobar": invalid syntax`)}, + {"", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"1", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"false", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"foobar", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Sprintf(`could not parse foobar as bool value for flag debug: .*`)}, - {"1s", 1 * time.Second, DurationFlag{Name: "time", EnvVar: "TIME"}, nil}, - {"foobar", false, DurationFlag{Name: "time", EnvVar: "TIME"}, fmt.Errorf(`could not parse foobar as duration for flag time: time: invalid duration foobar`)}, + {"1s", 1 * time.Second, DurationFlag{Name: "time", EnvVar: "TIME"}, ""}, + {"foobar", false, DurationFlag{Name: "time", EnvVar: "TIME"}, fmt.Sprintf(`could not parse foobar as duration for flag time: .*`)}, - {"1.2", 1.2, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1", 1.0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"foobar", 0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as float64 value for flag seconds: strconv.ParseFloat: parsing "foobar": invalid syntax`)}, + {"1.2", 1.2, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1", 1.0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"foobar", 0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as float64 value for flag seconds: .*`)}, - {"1", int64(1), Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as int value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1", int64(1), Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as int value for flag seconds: .*`)}, + {"foobar", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int value for flag seconds: .*`)}, - {"1", 1, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as int value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1", 1, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as int value for flag seconds: .*`)}, + {"foobar", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int value for flag seconds: .*`)}, - {"1,2", IntSlice{1, 2}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2,2", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2,2 as int slice value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int slice value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1,2", IntSlice{1, 2}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2,2", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2,2 as int slice value for flag seconds: .*`)}, + {"foobar", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int slice value for flag seconds: .*`)}, - {"1,2", Int64Slice{1, 2}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2,2", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2,2 as int64 slice value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int64 slice value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1,2", Int64Slice{1, 2}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2,2", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2,2 as int64 slice value for flag seconds: .*`)}, + {"foobar", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int64 slice value for flag seconds: .*`)}, - {"foo", "foo", StringFlag{Name: "name", EnvVar: "NAME"}, nil}, + {"foo", "foo", StringFlag{Name: "name", EnvVar: "NAME"}, ""}, - {"foo,bar", StringSlice{"foo", "bar"}, StringSliceFlag{Name: "names", EnvVar: "NAMES"}, nil}, + {"foo,bar", StringSlice{"foo", "bar"}, StringSliceFlag{Name: "names", EnvVar: "NAMES"}, ""}, - {"1", uint(1), UintFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as uint value for flag seconds: strconv.ParseUint: parsing "1.2": invalid syntax`)}, - {"foobar", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as uint value for flag seconds: strconv.ParseUint: parsing "foobar": invalid syntax`)}, + {"1", uint(1), UintFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as uint value for flag seconds: .*`)}, + {"foobar", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as uint value for flag seconds: .*`)}, - {"1", uint64(1), Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as uint64 value for flag seconds: strconv.ParseUint: parsing "1.2": invalid syntax`)}, - {"foobar", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as uint64 value for flag seconds: strconv.ParseUint: parsing "foobar": invalid syntax`)}, + {"1", uint64(1), Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as uint64 value for flag seconds: .*`)}, + {"foobar", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as uint64 value for flag seconds: .*`)}, - {"foo,bar", &Parser{"foo", "bar"}, GenericFlag{Name: "names", Value: &Parser{}, EnvVar: "NAMES"}, nil}, + {"foo,bar", &Parser{"foo", "bar"}, GenericFlag{Name: "names", Value: &Parser{}, EnvVar: "NAMES"}, ""}, } for _, test := range flagTests { @@ -98,8 +99,19 @@ func TestFlagsFromEnv(t *testing.T) { } err := a.Run([]string{"run"}) - if !reflect.DeepEqual(test.err, err) { - t.Errorf("expected error %s, got error %s", test.err, err) + + if test.errRegexp != "" { + if err == nil { + t.Errorf("expected error to match %s, got none", test.errRegexp) + } else { + if matched, _ := regexp.MatchString(test.errRegexp, err.Error()); !matched { + t.Errorf("expected error to match %s, got error %s", test.errRegexp, err) + } + } + } else { + if err != nil && test.errRegexp == "" { + t.Errorf("expected no error got %s", err) + } } } } From 9e5b04886c4bfee2ceba1465b8121057355c4e53 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 4 Mar 2017 14:33:36 -0800 Subject: [PATCH 5/5] Remove logic that exited even if the error was not an OsExiter This was introduced in #496, but was discovered to be a regression in the behavior of the library. --- errors.go | 9 --------- errors_test.go | 52 +------------------------------------------------- 2 files changed, 1 insertion(+), 60 deletions(-) diff --git a/errors.go b/errors.go index f9d648e..562b295 100644 --- a/errors.go +++ b/errors.go @@ -97,15 +97,6 @@ func HandleExitCoder(err error) { OsExiter(code) return } - - if err.Error() != "" { - if _, ok := err.(ErrorFormatter); ok { - fmt.Fprintf(ErrWriter, "%+v\n", err) - } else { - fmt.Fprintln(ErrWriter, err) - } - } - OsExiter(1) } func handleMultiError(multiErr MultiError) int { diff --git a/errors_test.go b/errors_test.go index d9e0da6..9b609c5 100644 --- a/errors_test.go +++ b/errors_test.go @@ -67,56 +67,6 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { expect(t, called, true) } -func TestHandleExitCoder_ErrorWithMessage(t *testing.T) { - exitCode := 0 - called := false - - OsExiter = func(rc int) { - if !called { - exitCode = rc - called = true - } - } - ErrWriter = &bytes.Buffer{} - - defer func() { - OsExiter = fakeOsExiter - ErrWriter = fakeErrWriter - }() - - err := errors.New("gourd havens") - HandleExitCoder(err) - - expect(t, exitCode, 1) - expect(t, called, true) - expect(t, ErrWriter.(*bytes.Buffer).String(), "gourd havens\n") -} - -func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { - exitCode := 0 - called := false - - OsExiter = func(rc int) { - if !called { - exitCode = rc - called = true - } - } - ErrWriter = &bytes.Buffer{} - - defer func() { - OsExiter = fakeOsExiter - ErrWriter = fakeErrWriter - }() - - err := errors.New("") - HandleExitCoder(err) - - expect(t, exitCode, 1) - expect(t, called, true) - expect(t, ErrWriter.(*bytes.Buffer).String(), "") -} - // make a stub to not import pkg/errors type ErrorWithFormat struct { error @@ -145,7 +95,7 @@ func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { ErrWriter = fakeErrWriter }() - err := NewErrorWithFormat("I am formatted") + err := NewExitError(NewErrorWithFormat("I am formatted"), 1) HandleExitCoder(err) expect(t, called, true)