From ce428377024188ae8ce8aae411881b7e993f9553 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 30 Sep 2016 19:23:44 +0900 Subject: [PATCH 01/19] Call HandleExitCoder for all members of MultiError.Errors --- errors.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/errors.go b/errors.go index c7d8c2f..fd67b96 100644 --- a/errors.go +++ b/errors.go @@ -86,8 +86,9 @@ func HandleExitCoder(err error) { if multiErr, ok := err.(MultiError); ok { for _, merr := range multiErr.Errors { - HandleExitCoder(merr) + fmt.Fprintln(ErrWriter, merr) } + OsExiter(1) return } From 6c50b15a273d29dc3820b5e4d50d78eeb113d335 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 11 Nov 2016 13:11:50 +0900 Subject: [PATCH 02/19] Exit with the code of ExitCoder if exists --- errors.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/errors.go b/errors.go index fd67b96..583f89f 100644 --- a/errors.go +++ b/errors.go @@ -85,10 +85,8 @@ func HandleExitCoder(err error) { } if multiErr, ok := err.(MultiError); ok { - for _, merr := range multiErr.Errors { - fmt.Fprintln(ErrWriter, merr) - } - OsExiter(1) + code := handleMultiError(multiErr) + OsExiter(code) return } @@ -97,3 +95,18 @@ func HandleExitCoder(err error) { } OsExiter(1) } + +func handleMultiError(multiErr MultiError) int { + code := 1 + for _, merr := range multiErr.Errors { + if multiErr2, ok := merr.(MultiError); ok { + code = handleMultiError(multiErr2) + } else { + fmt.Fprintln(ErrWriter, merr) + if exitErr, ok := merr.(ExitCoder); ok { + code = exitErr.ExitCode() + } + } + } + return code +} From 4267cd827cbfb066cf031bcb01ddbecdf7d0c49e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 4 Jan 2017 10:28:58 +0800 Subject: [PATCH 03/19] [ci skip] Fix template syntax error --- README.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index bb5f61e..6ba60c4 100644 --- a/README.md +++ b/README.md @@ -940,16 +940,13 @@ SUPPORT: support@awesometown.example.com cli.AppHelpTemplate = `NAME: {{.Name}} - {{.Usage}} USAGE: - {{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command -[command options]{{end}} {{if -.ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} + {{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} {{if len .Authors}} -AUTHOR(S): +AUTHOR: {{range .Authors}}{{ . }}{{end}} {{end}}{{if .Commands}} COMMANDS: -{{range .Commands}}{{if not .HideHelp}} {{join .Names ", "}}{{ "\t" -}}{{.Usage}}{{ "\n" }}{{end}}{{end}}{{end}}{{if .VisibleFlags}} +{{range .Commands}}{{if not .HideHelp}} {{join .Names ", "}}{{ "\t"}}{{.Usage}}{{ "\n" }}{{end}}{{end}}{{end}}{{if .VisibleFlags}} GLOBAL OPTIONS: {{range .VisibleFlags}}{{.}} {{end}}{{end}}{{if .Copyright }} From ac772237b96509d5150567da31ba6c6c3897d452 Mon Sep 17 00:00:00 2001 From: Maximilian Meister Date: Sun, 18 Dec 2016 17:43:48 +0100 Subject: [PATCH 04/19] command: enable ordering commands by name --- README.md | 24 ++++++++++++++++++++++-- command.go | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6ba60c4..2bbbd8e 100644 --- a/README.md +++ b/README.md @@ -455,13 +455,13 @@ error. Flags for the application and commands are shown in the order they are defined. However, it's possible to sort them from outside this library by using `FlagsByName` -with `sort`. +or `CommandsByName` with `sort`. For example this: ``` go package main @@ -488,7 +488,27 @@ func main() { }, } + app.Commands = []cli.Command{ + { + Name: "complete", + Aliases: []string{"c"}, + Usage: "complete a task on the list", + Action: func(c *cli.Context) error { + return nil + }, + }, + { + Name: "add", + Aliases: []string{"a"}, + Usage: "add a task to the list", + Action: func(c *cli.Context) error { + return nil + }, + }, + } + sort.Sort(cli.FlagsByName(app.Flags)) + sort.Sort(cli.CommandsByName(app.Commands)) app.Run(os.Args) } diff --git a/command.go b/command.go index 2628fbf..d297eb9 100644 --- a/command.go +++ b/command.go @@ -61,6 +61,20 @@ type Command struct { commandNamePath []string } +type CommandsByName []Command + +func (c CommandsByName) Len() int { + return len(c) +} + +func (c CommandsByName) Less(i, j int) bool { + return c[i].Name < c[j].Name +} + +func (c CommandsByName) Swap(i, j int) { + c[i], c[j] = c[j], c[i] +} + // FullName returns the full name of the command. // For subcommands this ensures that parent commands are part of the command path func (c Command) FullName() string { From 0083ae8732e4b5d6911729b5e60c99a19d6179ac Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 9 Jan 2017 15:57:49 -0800 Subject: [PATCH 05/19] 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 71ced406af64ee9961533d518dab28d455a66666 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Thu, 12 Jan 2017 18:59:38 +0900 Subject: [PATCH 06/19] Treat `rc` first called as exit code Because default OsExiter is os.Exit. --- errors_test.go | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/errors_test.go b/errors_test.go index 131bd38..d3764b7 100644 --- a/errors_test.go +++ b/errors_test.go @@ -12,8 +12,10 @@ func TestHandleExitCoder_nil(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -29,8 +31,10 @@ func TestHandleExitCoder_ExitCoder(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -46,8 +50,10 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -65,8 +71,10 @@ func TestHandleExitCoder_ErrorWithMessage(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } ErrWriter = &bytes.Buffer{} @@ -88,8 +96,10 @@ func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } ErrWriter = &bytes.Buffer{} @@ -123,7 +133,9 @@ func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { called := false OsExiter = func(rc int) { - called = true + if !called { + called = true + } } ErrWriter = &bytes.Buffer{} @@ -143,7 +155,9 @@ func TestHandleExitCoder_MultiErrorWithFormat(t *testing.T) { called := false OsExiter = func(rc int) { - called = true + if !called { + called = true + } } ErrWriter = &bytes.Buffer{} From 286b4b56d988e0ac6da075615f1da496db94da87 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Thu, 12 Jan 2017 19:12:17 +0900 Subject: [PATCH 07/19] Document on exit code in case of MultiError is given --- errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors.go b/errors.go index 8aa8d9e..f9d648e 100644 --- a/errors.go +++ b/errors.go @@ -74,7 +74,7 @@ func (ee *ExitError) ExitCode() int { // HandleExitCoder checks if the error fulfills the ExitCoder interface, and if // so prints the error to stderr (if it is non-empty) and calls OsExiter with the // given exit code. If the given error is a MultiError, then this func is -// called on all members of the Errors slice. +// called on all members of the Errors slice and calls OsExiter with the last exit code. func HandleExitCoder(err error) { if err == nil { return From adec15acf537c7b696c299223a16c60cb3eeeed8 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 13 Jan 2017 15:37:09 +0900 Subject: [PATCH 08/19] Add another ExitCoder to assert that it uses last one --- errors_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/errors_test.go b/errors_test.go index d3764b7..d9e0da6 100644 --- a/errors_test.go +++ b/errors_test.go @@ -59,10 +59,11 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { defer func() { OsExiter = fakeOsExiter }() exitErr := NewExitError("galactic perimeter breach", 9) - err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr) + exitErr2 := NewExitError("last ExitCoder", 11) + err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr, exitErr2) HandleExitCoder(err) - expect(t, exitCode, 9) + expect(t, exitCode, 11) expect(t, called, true) } From 7b912d9f8f8a78f192061ed5b0f40918dfda5a07 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 16:58:46 +0100 Subject: [PATCH 09/19] allow to load YAML configuration files on Windows The funtion `loadDataFrom` does not take care of Windows users since any of the conditions met and it returns an error. The change looks for the runtime where it's running and then if the filePath contains a `\` --- altsrc/yaml_file_loader.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 335356f..433023d 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -78,6 +78,11 @@ func loadDataFrom(filePath string) ([]byte, error) { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } return ioutil.ReadFile(filePath) + } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { + return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) + } + return ioutil.ReadFile(filePath) } else { return nil, fmt.Errorf("unable to determine how to load from path %s", filePath) } From 341ca93b01a20fe6fc361e005b45d968fd559aa5 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 17:08:01 +0100 Subject: [PATCH 10/19] fix imports Sorry, forgot to add imports correctly, needed to edit the file and make the commit using the github online editor, since I can't access from my current location from git. --- altsrc/yaml_file_loader.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 433023d..3965fe4 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -11,6 +11,8 @@ import ( "net/http" "net/url" "os" + "runtime" + "strings" "gopkg.in/urfave/cli.v1" From f1be59ff3d239f0942b201619030d302bce912cc Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Sat, 19 Nov 2016 22:37:11 +0100 Subject: [PATCH 11/19] added comment to windows filePath check --- altsrc/yaml_file_loader.go | 1 + 1 file changed, 1 insertion(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 3965fe4..dd808d5 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -81,6 +81,7 @@ func loadDataFrom(filePath string) ([]byte, error) { } return ioutil.ReadFile(filePath) } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + // on Windows systems u.Path is always empty, so we need to check the string directly. if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } From f8347a97c68c1b2a11f418da0225953c9b007708 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 8 Jan 2017 18:11:26 -0500 Subject: [PATCH 12/19] Fix altsrc slice inputs so that they correctly parse Was previously attempting to cast directly from []interface{} to []string or []int which Go doesn't support. Instead, we iterate over and cast each value (error'ing if the value is not the correct format). Fixes #580 --- altsrc/flag_test.go | 12 +++---- altsrc/map_input_source.go | 70 +++++++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/altsrc/flag_test.go b/altsrc/flag_test.go index 9e9c96d..cd18294 100644 --- a/altsrc/flag_test.go +++ b/altsrc/flag_test.go @@ -59,7 +59,7 @@ func TestStringSliceApplyInputSourceValue(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, }) expect(t, c.StringSlice("test"), []string{"hello", "world"}) } @@ -68,7 +68,7 @@ func TestStringSliceApplyInputSourceMethodContextSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, ContextValueString: "ohno", }) expect(t, c.StringSlice("test"), []string{"ohno"}) @@ -78,7 +78,7 @@ func TestStringSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test", EnvVar: "TEST"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, EnvVarName: "TEST", EnvVarValue: "oh,no", }) @@ -89,7 +89,7 @@ func TestIntSliceApplyInputSourceValue(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, }) expect(t, c.IntSlice("test"), []int{1, 2}) } @@ -98,7 +98,7 @@ func TestIntSliceApplyInputSourceMethodContextSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, ContextValueString: "3", }) expect(t, c.IntSlice("test"), []int{3}) @@ -108,7 +108,7 @@ func TestIntSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test", EnvVar: "TEST"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, EnvVarName: "TEST", EnvVarValue: "3,4", }) diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go index b720995..b3169e0 100644 --- a/altsrc/map_input_source.go +++ b/altsrc/map_input_source.go @@ -130,45 +130,59 @@ func (fsm *MapInputSource) String(name string) (string, error) { // StringSlice returns an []string from the map if it exists otherwise returns nil func (fsm *MapInputSource) StringSlice(name string) ([]string, error) { otherGenericValue, exists := fsm.valueMap[name] - if exists { - otherValue, isType := otherGenericValue.([]string) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]string", otherGenericValue) + if !exists { + otherGenericValue, exists = nestedVal(name, fsm.valueMap) + if !exists { + return nil, nil } - return otherValue, nil - } - nestedGenericValue, exists := nestedVal(name, fsm.valueMap) - if exists { - otherValue, isType := nestedGenericValue.([]string) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]string", nestedGenericValue) - } - return otherValue, nil } - return nil, nil + otherValue, isType := otherGenericValue.([]interface{}) + if !isType { + return nil, incorrectTypeForFlagError(name, "[]interface{}", otherGenericValue) + } + + var stringSlice = make([]string, 0, len(otherValue)) + for i, v := range otherValue { + stringValue, isType := v.(string) + + if !isType { + return nil, incorrectTypeForFlagError(fmt.Sprintf("%s[%d]", name, i), "string", v) + } + + stringSlice = append(stringSlice, stringValue) + } + + return stringSlice, nil } // IntSlice returns an []int from the map if it exists otherwise returns nil func (fsm *MapInputSource) IntSlice(name string) ([]int, error) { otherGenericValue, exists := fsm.valueMap[name] - if exists { - otherValue, isType := otherGenericValue.([]int) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]int", otherGenericValue) + if !exists { + otherGenericValue, exists = nestedVal(name, fsm.valueMap) + if !exists { + return nil, nil } - return otherValue, nil - } - nestedGenericValue, exists := nestedVal(name, fsm.valueMap) - if exists { - otherValue, isType := nestedGenericValue.([]int) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]int", nestedGenericValue) - } - return otherValue, nil } - return nil, nil + otherValue, isType := otherGenericValue.([]interface{}) + if !isType { + return nil, incorrectTypeForFlagError(name, "[]interface{}", otherGenericValue) + } + + var intSlice = make([]int, 0, len(otherValue)) + for i, v := range otherValue { + intValue, isType := v.(int) + + if !isType { + return nil, incorrectTypeForFlagError(fmt.Sprintf("%s[%d]", name, i), "int", v) + } + + intSlice = append(intSlice, intValue) + } + + return intSlice, nil } // Generic returns an cli.Generic from the map if it exists otherwise returns nil From e87dfbb6bb85250206841a9576e51b1323910356 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 16 Jan 2017 17:34:12 -0800 Subject: [PATCH 13/19] altsrc: Support slices for TOML --- altsrc/toml_file_loader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index 39c124f..37870fc 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -57,8 +57,8 @@ func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { } else { return nil, err } - case reflect.Array: - fallthrough // [todo] - Support array type + case reflect.Array, reflect.Slice: + ret[key] = val.([]interface{}) default: return nil, fmt.Errorf("Unsupported: type = %#v", v.Kind()) } From 4ed366e2011dfb9efa3399c709af0976d5e87db4 Mon Sep 17 00:00:00 2001 From: Robert Bittle Date: Wed, 18 Jan 2017 23:29:26 -0500 Subject: [PATCH 14/19] 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 15/19] 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 16/19] 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 17/19] 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) From 572dc46db5570298570b06ed63ae261086c8c7a4 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 10:39:43 -0700 Subject: [PATCH 18/19] Explicitly fetch `goimports` Fetching the whole tree was failing on some Go versions and we really only need goimports. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 94836d7..4417251 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,7 @@ matrix: before_script: - go get github.com/urfave/gfmrun/... || true -- go get golang.org/x/tools/... || true +- go get golang.org/x/tools/cmd/goimports - if [ ! -f node_modules/.bin/markdown-toc ] ; then npm install markdown-toc ; fi From 1bf0bb96b7b005507c19a2d71f4f3cadcc1202e2 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 10:54:32 -0700 Subject: [PATCH 19/19] Only support supported Go versions As described on https://github.com/golang/go/wiki/Go-Release-Cycle The maintainers do not test compatibility for libraries (e.g. in golang.org/x/tools) on older versions. --- .travis.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4417251..890e185 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,12 +7,9 @@ cache: - node_modules go: -- 1.2.x -- 1.3.x -- 1.4.2 -- 1.5.x - 1.6.x - 1.7.x +- 1.8.x - master matrix: @@ -23,6 +20,8 @@ matrix: os: osx - go: 1.7.x os: osx + - go: 1.8.x + os: osx before_script: - go get github.com/urfave/gfmrun/... || true