From 3fc3cd64854e64d2b772631a938083e41269c859 Mon Sep 17 00:00:00 2001 From: Yasuhiro KANDA Date: Fri, 22 Jul 2016 22:45:05 +0900 Subject: [PATCH 01/45] Add TOML config file loader --- altsrc/toml_command_test.go | 310 ++++++++++++++++++++++++++++++++++++ altsrc/toml_file_loader.go | 113 +++++++++++++ 2 files changed, 423 insertions(+) create mode 100644 altsrc/toml_command_test.go create mode 100644 altsrc/toml_file_loader.go diff --git a/altsrc/toml_command_test.go b/altsrc/toml_command_test.go new file mode 100644 index 0000000..c887ab2 --- /dev/null +++ b/altsrc/toml_command_test.go @@ -0,0 +1,310 @@ +// Disabling building of tom support in cases where golang is 1.0 or 1.1 +// as the encoding library is not implemented or supported. + +// +build go1.2 + +package altsrc + +import ( + "flag" + "io/ioutil" + "os" + "testing" + + "github.com/urfave/cli" +) + +func TestCommandTomFileTest(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestGlobalEnvVarWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "10") + defer os.Setenv("THE_TEST", "") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 10) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestGlobalEnvVarWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "10") + defer os.Setenv("THE_TEST", "") + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 10) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestSpecifiedFlagWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml", "--test", "7"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 7) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestSpecifiedFlagWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte(`[top] + test = 15`), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml", "--top.test", "7"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 7) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestDefaultValueFileWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", Value: 7}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileTestDefaultValueFileWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 15) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", Value: 7}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileFlagHasDefaultGlobalEnvTomlSetGlobalEnvWins(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("test = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "11") + defer os.Setenv("THE_TEST", "") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("test") + expect(t, val, 11) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "test", Value: 7, EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} + +func TestCommandTomlFileFlagHasDefaultGlobalEnvTomlSetGlobalEnvWinsNested(t *testing.T) { + app := cli.NewApp() + set := flag.NewFlagSet("test", 0) + ioutil.WriteFile("current.toml", []byte("[top]\ntest = 15"), 0666) + defer os.Remove("current.toml") + + os.Setenv("THE_TEST", "11") + defer os.Setenv("THE_TEST", "") + + test := []string{"test-cmd", "--load", "current.toml"} + set.Parse(test) + + c := cli.NewContext(app, set, nil) + + command := &cli.Command{ + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(c *cli.Context) error { + val := c.Int("top.test") + expect(t, val, 11) + return nil + }, + Flags: []cli.Flag{ + NewIntFlag(cli.IntFlag{Name: "top.test", Value: 7, EnvVar: "THE_TEST"}), + cli.StringFlag{Name: "load"}}, + } + command.Before = InitInputSourceWithContext(command.Flags, NewTomlSourceFromFlagFunc("load")) + err := command.Run(c) + + expect(t, err, nil) +} diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go new file mode 100644 index 0000000..64250a3 --- /dev/null +++ b/altsrc/toml_file_loader.go @@ -0,0 +1,113 @@ +// Disabling building of toml support in cases where golang is 1.0 or 1.1 +// as the encoding library is not implemented or supported. + +// +build go1.2 + +package altsrc + +import ( + "fmt" + "reflect" + + "github.com/BurntSushi/toml" + "github.com/urfave/cli" +) + +type TomlMap struct { + Map map[interface{}]interface{} +} + +func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { + ret = make(map[interface{}]interface{}) + m := i.(map[string]interface{}) + for key, val := range m { + v := reflect.ValueOf(val) + switch v.Kind() { + case reflect.Bool: + ret[key] = val.(bool) + case reflect.String: + ret[key] = val.(string) + case reflect.Int: + ret[key] = int(val.(int)) + case reflect.Int8: + ret[key] = int(val.(int8)) + case reflect.Int16: + ret[key] = int(val.(int16)) + case reflect.Int32: + ret[key] = int(val.(int32)) + case reflect.Int64: + ret[key] = int(val.(int64)) + case reflect.Uint: + ret[key] = int(val.(uint)) + case reflect.Uint8: + ret[key] = int(val.(uint8)) + case reflect.Uint16: + ret[key] = int(val.(uint16)) + case reflect.Uint32: + ret[key] = int(val.(uint32)) + case reflect.Uint64: + ret[key] = int(val.(uint64)) + case reflect.Float32: + ret[key] = float64(val.(float32)) + case reflect.Float64: + ret[key] = float64(val.(float64)) + case reflect.Map: + if tmp, err := unmarshalMap(val); err == nil { + ret[key] = tmp + } else { + return nil, err + } + case reflect.Array: + fallthrough // [todo] - Support array type + default: + return nil, fmt.Errorf("Unsupported: type = %#v", v.Kind()) + } + } + return ret, nil +} + +func (self *TomlMap) UnmarshalTOML(i interface{}) error { + if tmp, err := unmarshalMap(i); err == nil { + self.Map = tmp + } else { + return err + } + return nil +} + +type tomlSourceContext struct { + FilePath string +} + +// NewTomlSourceFromFile creates a new TOML InputSourceContext from a filepath. +func NewTomlSourceFromFile(file string) (InputSourceContext, error) { + tsc := &tomlSourceContext{FilePath: file} + var results TomlMap = TomlMap{} + if err := readCommandToml(tsc.FilePath, &results); err != nil { + return nil, fmt.Errorf("Unable to load TOML file '%s': inner error: \n'%v'", tsc.FilePath, err.Error()) + } + return &MapInputSource{valueMap: results.Map}, nil +} + +// NewTomlSourceFromFlagFunc creates a new TOML InputSourceContext from a provided flag name and source context. +func NewTomlSourceFromFlagFunc(flagFileName string) func(context *cli.Context) (InputSourceContext, error) { + return func(context *cli.Context) (InputSourceContext, error) { + filePath := context.String(flagFileName) + return NewTomlSourceFromFile(filePath) + } +} + +func readCommandToml(filePath string, container interface{}) (err error) { + b, err := loadDataFrom(filePath) + if err != nil { + return err + } + + err = toml.Unmarshal(b, container) + if err != nil { + return err + } + + err = nil + return +} From c59ec842c11a6ceb3dd44aa4e6def4320a9b79c2 Mon Sep 17 00:00:00 2001 From: Yi EungJun Date: Mon, 25 Jul 2016 20:09:41 +0900 Subject: [PATCH 02/45] README: Remove unnecessary 'v' from version numbers 'partay version v19.99.0' sounds 'partay version version 19.99.0' because 'v' stands for 'version'. --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0ffa92f..615e95d 100644 --- a/README.md +++ b/README.md @@ -953,7 +953,7 @@ setting `cli.VersionFlag`, e.g.: ``` go package main @@ -972,7 +972,7 @@ func main() { app := cli.NewApp() app.Name = "partay" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Run(os.Args) } ``` @@ -981,7 +981,7 @@ Alternatively, the version printer at `cli.VersionPrinter` may be overridden, e. ``` go package main @@ -1004,7 +1004,7 @@ func main() { app := cli.NewApp() app.Name = "partay" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Run(os.Args) } ``` @@ -1083,7 +1083,7 @@ func (g *genericType) String() string { func main() { app := cli.NewApp() app.Name = "kənˈtrīv" - app.Version = "v19.99.0" + app.Version = "19.99.0" app.Compiled = time.Now() app.Authors = []cli.Author{ cli.Author{ From 812de9e2507c3ec9ce2750b67e5efe8cc643c0f4 Mon Sep 17 00:00:00 2001 From: kandayasu Date: Tue, 26 Jul 2016 01:37:03 +0900 Subject: [PATCH 03/45] type "TomlMap" to private (rename TomlMap -> tomlMap) --- altsrc/toml_file_loader.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index 64250a3..bc2c11d 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -13,7 +13,7 @@ import ( "github.com/urfave/cli" ) -type TomlMap struct { +type tomlMap struct { Map map[interface{}]interface{} } @@ -66,7 +66,7 @@ func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { return ret, nil } -func (self *TomlMap) UnmarshalTOML(i interface{}) error { +func (self *tomlMap) UnmarshalTOML(i interface{}) error { if tmp, err := unmarshalMap(i); err == nil { self.Map = tmp } else { @@ -82,7 +82,7 @@ type tomlSourceContext struct { // NewTomlSourceFromFile creates a new TOML InputSourceContext from a filepath. func NewTomlSourceFromFile(file string) (InputSourceContext, error) { tsc := &tomlSourceContext{FilePath: file} - var results TomlMap = TomlMap{} + var results tomlMap = tomlMap{} if err := readCommandToml(tsc.FilePath, &results); err != nil { return nil, fmt.Errorf("Unable to load TOML file '%s': inner error: \n'%v'", tsc.FilePath, err.Error()) } From 36b7d89bedc1626a65c03df4f9fad9d4a96a4354 Mon Sep 17 00:00:00 2001 From: kandayasu Date: Tue, 26 Jul 2016 01:37:18 +0900 Subject: [PATCH 04/45] Fix typo --- altsrc/toml_command_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/altsrc/toml_command_test.go b/altsrc/toml_command_test.go index c887ab2..1d91d56 100644 --- a/altsrc/toml_command_test.go +++ b/altsrc/toml_command_test.go @@ -1,4 +1,4 @@ -// Disabling building of tom support in cases where golang is 1.0 or 1.1 +// Disabling building of toml support in cases where golang is 1.0 or 1.1 // as the encoding library is not implemented or supported. // +build go1.2 From b616f6088660d2eaa33739718f0583f8d467a178 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 12:11:52 -0700 Subject: [PATCH 05/45] Note TOML support in README and CHANGELOG --- CHANGELOG.md | 1 + README.md | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26dd564..8b0d0ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Added - Flag type code generation via `go generate` - Write to stderr and exit 1 if action returns non-nil error +- Added support for TOML to the `altsrc` loader ### Changed - Raise minimum tested/supported Go version to 1.2+ diff --git a/README.md b/README.md index 615e95d..31b25b8 100644 --- a/README.md +++ b/README.md @@ -31,7 +31,7 @@ applications in an expressive way. + [Placeholder Values](#placeholder-values) + [Alternate Names](#alternate-names) + [Values from the Environment](#values-from-the-environment) - + [Values from alternate input sources (YAML and others)](#values-from-alternate-input-sources-yaml-and-others) + + [Values from alternate input sources (YAML, TOML, and others)](#values-from-alternate-input-sources-yaml-toml-and-others) * [Subcommands](#subcommands) * [Subcommands categories](#subcommands-categories) * [Exit code](#exit-code) @@ -513,10 +513,14 @@ func main() { } ``` -#### Values from alternate input sources (YAML and others) +#### Values from alternate input sources (YAML, TOML, and others) There is a separate package altsrc that adds support for getting flag values -from other input sources like YAML. +from other file input sources. + +Currently supported input source formats: +* YAML +* TOML In order to get values for a flag from an alternate input source the following code would be added to wrap an existing cli.Flag like below: @@ -538,9 +542,9 @@ the yaml input source for any flags that are defined on that command. As a note the "load" flag used would also have to be defined on the command flags in order for this code snipped to work. -Currently only YAML files are supported but developers can add support for other -input sources by implementing the altsrc.InputSourceContext for their given -sources. +Currently only the aboved specified formats are supported but developers can +add support for other input sources by implementing the +altsrc.InputSourceContext for their given sources. Here is a more complete sample of a command using YAML support: From 6c1f51aa95e7966c49a5211ffabe337bf4d2f3fb Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 14:14:46 -0700 Subject: [PATCH 06/45] Fix context.(Global)IsSet to respect environment variables This appeared to be the least messy approach to hack in support for IsSet also checking environment variables to see if a particular cli.Flag was set without making backwards incompatible changes to the interface. I intend to fix this more properly in v2, probably by adding another method to the cli.Flag interface to push the responsibility down as it occurred to me that it was really the `Flag`s themselves that offer support for configuration via the environment as opposed to the `context` or other supporting structures. This opens the door for the anything implementing the `Flag` interface to have additional sources of input while still supporting `context.IsSet`. --- context.go | 79 ++++++++++++++++++++++++++++++++++++++----------- context_test.go | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 18 deletions(-) diff --git a/context.go b/context.go index 14ad3f7..5ea4f8f 100644 --- a/context.go +++ b/context.go @@ -3,6 +3,8 @@ package cli import ( "errors" "flag" + "os" + "reflect" "strings" ) @@ -11,12 +13,11 @@ import ( // can be used to retrieve context-specific Args and // parsed command-line options. type Context struct { - App *App - Command Command - flagSet *flag.FlagSet - setFlags map[string]bool - globalSetFlags map[string]bool - parentContext *Context + App *App + Command Command + flagSet *flag.FlagSet + setFlags map[string]bool + parentContext *Context } // NewContext creates a new context. For use in when invoking an App or Command action. @@ -43,28 +44,70 @@ func (c *Context) GlobalSet(name, value string) error { func (c *Context) IsSet(name string) bool { if c.setFlags == nil { c.setFlags = make(map[string]bool) + c.flagSet.Visit(func(f *flag.Flag) { c.setFlags[f.Name] = true }) + + c.flagSet.VisitAll(func(f *flag.Flag) { + if _, ok := c.setFlags[f.Name]; ok { + return + } + c.setFlags[f.Name] = false + }) + + // XXX hack to support IsSet for flags with EnvVar + // + // There isn't an easy way to do this with the current implementation since + // whether a flag was set via an environment variable is very difficult to + // determine here. Instead, we intend to introduce a backwards incompatible + // change in version 2 to add `IsSet` to the Flag interface to push the + // responsibility closer to where the information required to determine + // whether a flag is set by non-standard means such as environment + // variables is avaliable. + // + // See https://github.com/urfave/cli/issues/294 for additional discussion + flags := c.Command.Flags + if c.Command.Name == "" { // cannot == Command{} since it contains slice types + if c.App != nil { + flags = c.App.Flags + } + } + for _, f := range flags { + eachName(f.GetName(), func(name string) { + if isSet, ok := c.setFlags[name]; isSet || !ok { + return + } + + envVars := reflect.ValueOf(f).FieldByName("EnvVar").String() + + eachName(envVars, func(envVar string) { + envVar = strings.TrimSpace(envVar) + if envVal := os.Getenv(envVar); envVal != "" { + c.setFlags[name] = true + return + } + }) + }) + } } - return c.setFlags[name] == true + + return c.setFlags[name] } // GlobalIsSet determines if the global flag was actually set func (c *Context) GlobalIsSet(name string) bool { - if c.globalSetFlags == nil { - c.globalSetFlags = make(map[string]bool) - ctx := c - if ctx.parentContext != nil { - ctx = ctx.parentContext - } - for ; ctx != nil && c.globalSetFlags[name] == false; ctx = ctx.parentContext { - ctx.flagSet.Visit(func(f *flag.Flag) { - c.globalSetFlags[f.Name] = true - }) + ctx := c + if ctx.parentContext != nil { + ctx = ctx.parentContext + } + + for ; ctx != nil; ctx = ctx.parentContext { + if ctx.IsSet(name) { + return true } } - return c.globalSetFlags[name] + return false } // FlagNames returns a slice of flag names used in this context. diff --git a/context_test.go b/context_test.go index 5c68fdd..0cf84d1 100644 --- a/context_test.go +++ b/context_test.go @@ -2,6 +2,7 @@ package cli import ( "flag" + "os" "testing" "time" ) @@ -180,6 +181,33 @@ func TestContext_IsSet(t *testing.T) { expect(t, c.IsSet("myflagGlobal"), false) } +// XXX Corresponds to hack in context.IsSet for flags with EnvVar field +// Should be moved to `flag_test` in v2 +func TestContext_IsSet_fromEnv(t *testing.T) { + var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + + os.Clearenv() + os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + a := App{ + Flags: []Flag{ + Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + Float64Flag{Name: "no-env-var, n"}, + }, + Action: func(ctx *Context) error { + timeoutIsSet = ctx.IsSet("timeout") + tIsSet = ctx.IsSet("t") + noEnvVarIsSet = ctx.IsSet("no-env-var") + nIsSet = ctx.IsSet("n") + return nil + }, + } + a.Run([]string{"run"}) + expect(t, timeoutIsSet, true) + expect(t, tIsSet, true) + expect(t, noEnvVarIsSet, false) + expect(t, nIsSet, false) +} + func TestContext_GlobalIsSet(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") @@ -199,6 +227,38 @@ func TestContext_GlobalIsSet(t *testing.T) { expect(t, c.GlobalIsSet("bogusGlobal"), false) } +// XXX Corresponds to hack in context.IsSet for flags with EnvVar field +// Should be moved to `flag_test` in v2 +func TestContext_GlobalIsSet_fromEnv(t *testing.T) { + var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + + os.Clearenv() + os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + a := App{ + Flags: []Flag{ + Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + Float64Flag{Name: "no-env-var, n"}, + }, + Commands: []Command{ + { + Name: "hello", + Action: func(ctx *Context) error { + timeoutIsSet = ctx.GlobalIsSet("timeout") + tIsSet = ctx.GlobalIsSet("t") + noEnvVarIsSet = ctx.GlobalIsSet("no-env-var") + nIsSet = ctx.GlobalIsSet("n") + return nil + }, + }, + }, + } + a.Run([]string{"run", "hello"}) + expect(t, timeoutIsSet, true) + expect(t, tIsSet, true) + expect(t, noEnvVarIsSet, false) + expect(t, nIsSet, false) +} + func TestContext_NumFlags(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") From 168c95418e66e019fe17b8f4f5c45aa62ff80e23 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 31 Jul 2016 20:09:57 -0700 Subject: [PATCH 07/45] Ensure that EnvVar struct field exists before interrogating it Otherwise you end up with `` which, in practice, would probably work, but this is cleaner. --- context.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5ea4f8f..15570c5 100644 --- a/context.go +++ b/context.go @@ -79,9 +79,12 @@ func (c *Context) IsSet(name string) bool { return } - envVars := reflect.ValueOf(f).FieldByName("EnvVar").String() + envVarValue := reflect.ValueOf(f).FieldByName("EnvVar") + if !envVarValue.IsValid() { + return + } - eachName(envVars, func(envVar string) { + eachName(envVarValue.String(), func(envVar string) { envVar = strings.TrimSpace(envVar) if envVal := os.Getenv(envVar); envVal != "" { c.setFlags[name] = true From 8e6aa34a1284bc29e84866ed015bca0f3dd2d88e Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 21 Aug 2016 13:51:55 -0700 Subject: [PATCH 08/45] remove the possiblity of end-user's seeing deprecation warnings Instead use deprecation pattern described in https://blog.golang.org/godoc-documenting-go-code. Fixes #507 --- app.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app.go b/app.go index 0755bb6..3b54518 100644 --- a/app.go +++ b/app.go @@ -62,10 +62,11 @@ type App struct { // An action to execute after any subcommands are run, but after the subcommand has finished // It is run even if Action() panics After AfterFunc + // The action to execute when no subcommands are specified + // Expects a `cli.ActionFunc` but will accept the *deprecated* signature of `func(*cli.Context) {}` + // *Note*: support for the deprecated `Action` signature will be removed in a future version Action interface{} - // TODO: replace `Action: interface{}` with `Action: ActionFunc` once some kind - // of deprecation period has passed, maybe? // Execute this function if the proper command cannot be found CommandNotFound CommandNotFoundFunc @@ -247,11 +248,12 @@ func (a *App) Run(arguments []string) (err error) { return err } -// DEPRECATED: Another entry point to the cli app, takes care of passing arguments and error handling +// RunAndExitOnError calls .Run() and exits non-zero if an error was returned +// +// Deprecated: instead you should return an error that fulfills cli.ExitCoder +// to cli.App.Run. This will cause the application to exit with the given eror +// code in the cli.ExitCoder func (a *App) RunAndExitOnError() { - fmt.Fprintf(a.errWriter(), - "DEPRECATED cli.App.RunAndExitOnError. %s See %s\n", - contactSysadmin, runAndExitOnErrorDeprecationURL) if err := a.Run(os.Args); err != nil { fmt.Fprintln(a.errWriter(), err) OsExiter(1) @@ -471,7 +473,7 @@ func HandleAction(action interface{}, context *Context) (err error) { // swallowing all panics that may happen when calling an Action func. s := fmt.Sprintf("%v", r) if strings.HasPrefix(s, "reflect: ") && strings.Contains(s, "too many input arguments") { - err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v. See %s", r, appActionDeprecationURL), 2) + err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v."), 2) } else { panic(r) } @@ -485,9 +487,6 @@ func HandleAction(action interface{}, context *Context) (err error) { vals := reflect.ValueOf(action).Call([]reflect.Value{reflect.ValueOf(context)}) if len(vals) == 0 { - fmt.Fprintf(ErrWriter, - "DEPRECATED Action signature. Must be `cli.ActionFunc`. %s See %s\n", - contactSysadmin, appActionDeprecationURL) return nil } From a5ca09a9344c444c8dfcd5c1718d06b75b7aac1e Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 21 Aug 2016 14:06:59 -0700 Subject: [PATCH 09/45] fixup! remove the possiblity of end-user's seeing deprecation warnings --- app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.go b/app.go index 3b54518..b9adf46 100644 --- a/app.go +++ b/app.go @@ -473,7 +473,7 @@ func HandleAction(action interface{}, context *Context) (err error) { // swallowing all panics that may happen when calling an Action func. s := fmt.Sprintf("%v", r) if strings.HasPrefix(s, "reflect: ") && strings.Contains(s, "too many input arguments") { - err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v."), 2) + err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v.", r), 2) } else { panic(r) } From c0cf41eb54ec35f7c325e9b42aaec3c33a36194f Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 22 Aug 2016 15:26:33 -0400 Subject: [PATCH 10/45] Skip gfmrun installation and tests below go1.3 --- .travis.yml | 2 +- runtests | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d1d820d..a6a1386 100644 --- a/.travis.yml +++ b/.travis.yml @@ -22,7 +22,7 @@ matrix: os: osx before_script: -- go get github.com/urfave/gfmrun/... +- go get github.com/urfave/gfmrun/... || true - go get golang.org/x/tools/... || true - if [ ! -f node_modules/.bin/markdown-toc ] ; then npm install markdown-toc ; diff --git a/runtests b/runtests index e13faf7..ee22bde 100755 --- a/runtests +++ b/runtests @@ -57,6 +57,10 @@ def _test(): def _gfmrun(): + go_version = check_output('go version'.split()).split()[2] + if go_version < 'go1.3': + print('runtests: skip on {}'.format(go_version), file=sys.stderr) + return _run(['gfmrun', '-c', str(_gfmrun_count()), '-s', 'README.md']) From c75c862386f960a352e2e48611de3544d585bac4 Mon Sep 17 00:00:00 2001 From: Jake Champlin Date: Sat, 27 Aug 2016 19:09:14 -0400 Subject: [PATCH 11/45] Fix import paths in altsrc Uses gopkg.in as the import path for the `altsrc` package. Fixes: #473 --- altsrc/flag.go | 2 +- altsrc/flag_generated.go | 2 +- altsrc/flag_test.go | 2 +- altsrc/input_source_context.go | 2 +- altsrc/map_input_source.go | 2 +- altsrc/toml_command_test.go | 2 +- altsrc/toml_file_loader.go | 2 +- altsrc/yaml_command_test.go | 2 +- altsrc/yaml_file_loader.go | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) diff --git a/altsrc/flag.go b/altsrc/flag.go index 8add2fb..ec14e40 100644 --- a/altsrc/flag.go +++ b/altsrc/flag.go @@ -6,7 +6,7 @@ import ( "strconv" "strings" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) // FlagInputSourceExtension is an extension interface of cli.Flag that diff --git a/altsrc/flag_generated.go b/altsrc/flag_generated.go index fa76724..b6b96a1 100644 --- a/altsrc/flag_generated.go +++ b/altsrc/flag_generated.go @@ -3,7 +3,7 @@ package altsrc import ( "flag" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) // WARNING: This file is generated! diff --git a/altsrc/flag_test.go b/altsrc/flag_test.go index 218e9b8..9e9c96d 100644 --- a/altsrc/flag_test.go +++ b/altsrc/flag_test.go @@ -8,7 +8,7 @@ import ( "testing" "time" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) type testApplyInputSource struct { diff --git a/altsrc/input_source_context.go b/altsrc/input_source_context.go index 8ea7e92..276dcda 100644 --- a/altsrc/input_source_context.go +++ b/altsrc/input_source_context.go @@ -3,7 +3,7 @@ package altsrc import ( "time" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) // InputSourceContext is an interface used to allow diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go index b1c8e4f..b720995 100644 --- a/altsrc/map_input_source.go +++ b/altsrc/map_input_source.go @@ -6,7 +6,7 @@ import ( "strings" "time" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) // MapInputSource implements InputSourceContext to return diff --git a/altsrc/toml_command_test.go b/altsrc/toml_command_test.go index 1d91d56..a5053d4 100644 --- a/altsrc/toml_command_test.go +++ b/altsrc/toml_command_test.go @@ -11,7 +11,7 @@ import ( "os" "testing" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) func TestCommandTomFileTest(t *testing.T) { diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index bc2c11d..39c124f 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -10,7 +10,7 @@ import ( "reflect" "github.com/BurntSushi/toml" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) type tomlMap struct { diff --git a/altsrc/yaml_command_test.go b/altsrc/yaml_command_test.go index 31f78ce..9d3f431 100644 --- a/altsrc/yaml_command_test.go +++ b/altsrc/yaml_command_test.go @@ -11,7 +11,7 @@ import ( "os" "testing" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" ) func TestCommandYamlFileTest(t *testing.T) { diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index b4e3365..335356f 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -12,7 +12,7 @@ import ( "net/url" "os" - "github.com/urfave/cli" + "gopkg.in/urfave/cli.v1" "gopkg.in/yaml.v2" ) From df95e0708f6416a3f148c984464cfd417e9eccfe Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 3 Sep 2016 14:22:45 -0700 Subject: [PATCH 12/45] Manually set import in altsrc flag generation Otherwise `goimports` switches it to `github.com/urfave/cli` --- generate-flag-types | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/generate-flag-types b/generate-flag-types index 8567f5d..47a168b 100755 --- a/generate-flag-types +++ b/generate-flag-types @@ -202,6 +202,10 @@ def _write_altsrc_flag_types(outfile, types): _fwrite(outfile, """\ package altsrc + import ( + "gopkg.in/urfave/cli.v1" + ) + // WARNING: This file is generated! """) From 4b62cb6b3397f6046f1e001f1be53daa7b4d970d Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 3 Sep 2016 13:18:48 -0700 Subject: [PATCH 13/45] Update wording around gopkg.in pinning to be more accurate Since we have `v1.X` tags, gopkg.in/urfave/cli.v1 will pull the latest tagged release rather than the `v1` branch. Since there are no tagged `v2` releases, `gopkg.in` will pull the latest commit of that branch. Fixes #513 --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 31b25b8..2c863ae 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ applications in an expressive way. - [Installation](#installation) * [Supported platforms](#supported-platforms) * [Using the `v2` branch](#using-the-v2-branch) - * [Pinning to the `v1` branch](#pinning-to-the-v1-branch) + * [Pinning to the `v1` releases](#pinning-to-the-v1-releases) - [Getting Started](#getting-started) - [Examples](#examples) * [Arguments](#arguments) @@ -104,11 +104,11 @@ import ( ... ``` -### Pinning to the `v1` branch +### Pinning to the `v1` releases Similarly to the section above describing use of the `v2` branch, if one wants to avoid any unexpected compatibility pains once `v2` becomes `master`, then -pinning to the `v1` branch is an acceptable option, e.g.: +pinning to `v1` is an acceptable option, e.g.: ``` $ go get gopkg.in/urfave/cli.v1 @@ -122,6 +122,8 @@ import ( ... ``` +This will pull the latest tagged `v1` release (e.g. `v1.18.1` at the time of writing). + ## Getting Started One of the philosophies behind cli is that an API should be playful and full of From 76164d6e36d466a0b42d9079d2f4789ace074184 Mon Sep 17 00:00:00 2001 From: Nathan Bullock Date: Mon, 5 Sep 2016 07:16:05 -0400 Subject: [PATCH 14/45] Fix typo in README cosumized -> customized --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 31b25b8..2971e26 100644 --- a/README.md +++ b/README.md @@ -847,7 +847,7 @@ func main() { ### Generated Help Text -The default help flag (`-h/--help`) is defined as `cli.HelpFlag` and is checked +The default help flag (`-h/--help`) is defined as `cli.HelpFlag` and is checked by the cli internals in order to print generated help text for the app, command, or subcommand, and break execution. @@ -952,7 +952,7 @@ is checked by the cli internals in order to print the `App.Version` via #### Customization -The default flag may be cusomized to something other than `-v/--version` by +The default flag may be customized to something other than `-v/--version` by setting `cli.VersionFlag`, e.g.: +``` go +package main + +import ( + "os" + "sort" + + "github.com/urfave/cli" +) + +func main() { + app := cli.NewApp() + + app.Flags = []cli.Flag { + cli.StringFlag{ + Name: "lang, l", + Value: "english", + Usage: "Language for the greeting", + }, + cli.StringFlag{ + Name: "config, c", + Usage: "Load configuration from `FILE`", + }, + } + + sort.Sort(cli.FlagsByName(app.Flags)) + + app.Run(os.Args) +} +``` + +Will result in help output like: + +``` +--config FILE, -c FILE Load configuration from FILE +--lang value, -l value Language for the greeting (default: "english") +``` + #### Values from the Environment You can also have the default value set from the environment via `EnvVar`. e.g. diff --git a/flag.go b/flag.go index e748c02..1ff28d3 100644 --- a/flag.go +++ b/flag.go @@ -37,6 +37,21 @@ var HelpFlag = BoolFlag{ // to display a flag. var FlagStringer FlagStringFunc = stringifyFlag +// FlagsByName is a slice of Flag. +type FlagsByName []Flag + +func (f FlagsByName) Len() int { + return len(f) +} + +func (f FlagsByName) Less(i, j int) bool { + return f[i].GetName() < f[j].GetName() +} + +func (f FlagsByName) Swap(i, j int) { + f[i], f[j] = f[j], f[i] +} + // Flag is a common interface related to parsing flags in cli. // For more advanced flag parsing techniques, it is recommended that // this interface be implemented. From d913b71c72fbd5857a4e5219d53023d14f0eb346 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Fri, 21 Oct 2016 10:42:30 +0200 Subject: [PATCH 23/45] .travis.yml: add go 1.7.x Signed-off-by: Antonio Murdaca --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index a6a1386..94836d7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ go: - 1.4.2 - 1.5.x - 1.6.x +- 1.7.x - master matrix: @@ -20,6 +21,8 @@ matrix: include: - go: 1.6.x os: osx + - go: 1.7.x + os: osx before_script: - go get github.com/urfave/gfmrun/... || true From 0c143a2a268ff019a99698fe2e6d9e7bfed088c7 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 22 Oct 2016 15:58:07 -0700 Subject: [PATCH 24/45] app: Fix trailing space for Author.String() This code initially landed with lots of space: '{name} <{email}> ' or: '{name} ' in 3d718330 (app, help: add support for multiple authors, 2015-01-31). The doubled space between the name and email was removed in c6592bb4 (app, help: add backwards compatibility for Authors, 2015-02-21), but a trailing space remained in both the email and email-less cases. This commit removes that trailing space. --- app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index e5c2839..77eb141 100644 --- a/app.go +++ b/app.go @@ -458,10 +458,10 @@ type Author struct { func (a Author) String() string { e := "" if a.Email != "" { - e = "<" + a.Email + "> " + e = " <" + a.Email + ">" } - return fmt.Sprintf("%v %v", a.Name, e) + return fmt.Sprintf("%v%v", a.Name, e) } // HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an From 3c2bce5807d92a07ae1d3adf503a9811525e3e4b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 22 Oct 2016 16:02:21 -0700 Subject: [PATCH 25/45] help: Cleanup AppHelpTemplate trailing whitespace Most of the changes here remove trailing whitespace, but I also add code to select "AUTHOR" or "AUTHORS" as appropriate instead of the previous "AUTHOR(S)". The template for listing with an entry per line is: {{range $index, $entry := pipeline}}{{if $index}} {{end}}{{$entry}}{{end}} That range syntax is discussed in [1]. Also add a unit test, which tests both these whitespace changes and also the earlier App.Description addition. [1]: https://golang.org/pkg/text/template/#hdr-Variables --- app_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++- help.go | 31 +++++++++++++++-------------- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/app_test.go b/app_test.go index 23c8aa6..2b541f8 100644 --- a/app_test.go +++ b/app_test.go @@ -90,7 +90,62 @@ func ExampleApp_Run_subcommand() { // Hello, Jeremy } -func ExampleApp_Run_help() { +func ExampleApp_Run_appHelp() { + // set args for examples sake + os.Args = []string{"greet", "help"} + + app := NewApp() + app.Name = "greet" + app.Version = "0.1.0" + app.Description = "This is how we describe greet the app" + app.Authors = []Author{ + {Name: "Harrison", Email: "harrison@lolwut.com"}, + {Name: "Oliver Allen", Email: "oliver@toyshop.com"}, + } + app.Flags = []Flag{ + StringFlag{Name: "name", Value: "bob", Usage: "a name to say"}, + } + app.Commands = []Command{ + { + Name: "describeit", + Aliases: []string{"d"}, + Usage: "use it to see a description", + Description: "This is how we describe describeit the function", + Action: func(c *Context) error { + fmt.Printf("i like to describe things") + return nil + }, + }, + } + app.Run(os.Args) + // Output: + // NAME: + // greet - A new cli application + // + // USAGE: + // greet [global options] command [command options] [arguments...] + // + // VERSION: + // 0.1.0 + // + // DESCRIPTION: + // This is how we describe greet the app + // + // AUTHORS: + // Harrison + // Oliver Allen + // + // COMMANDS: + // describeit, d use it to see a description + // help, h Shows a list of commands or help for one command + // + // GLOBAL OPTIONS: + // --name value a name to say (default: "bob") + // --help, -h show help + // --version, -v print the version +} + +func ExampleApp_Run_commandHelp() { // set args for examples sake os.Args = []string{"greet", "h", "describeit"} diff --git a/help.go b/help.go index 529e5ea..515f744 100644 --- a/help.go +++ b/help.go @@ -16,27 +16,28 @@ var AppHelpTemplate = `NAME: {{.Name}} - {{.Usage}} USAGE: - {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}} - {{if .Version}}{{if not .HideVersion}} + {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Version}}{{if not .HideVersion}} + VERSION: - {{.Version}} - {{end}}{{end}}{{if .Description}} + {{.Version}}{{end}}{{end}}{{if .Description}} + DESCRIPTION: - {{.Description}} -{{end}}{{if len .Authors}} -AUTHOR(S): - {{range .Authors}}{{.}}{{end}} - {{end}}{{if .VisibleCommands}} + {{.Description}}{{end}}{{if len .Authors}} + +AUTHOR{{with $length := len .Authors}}{{if ne 1 $length}}S{{end}}{{end}}: + {{range $index, $author := .Authors}}{{if $index}} + {{end}}{{$author}}{{end}}{{end}}{{if .VisibleCommands}} + COMMANDS:{{range .VisibleCategories}}{{if .Name}} {{.Name}}:{{end}}{{range .VisibleCommands}} - {{join .Names ", "}}{{"\t"}}{{.Usage}}{{end}} -{{end}}{{end}}{{if .VisibleFlags}} + {{join .Names ", "}}{{"\t"}}{{.Usage}}{{end}}{{end}}{{end}}{{if .VisibleFlags}} + GLOBAL OPTIONS: - {{range .VisibleFlags}}{{.}} - {{end}}{{end}}{{if .Copyright}} + {{range $index, $option := .VisibleFlags}}{{if $index}} + {{end}}{{$option}}{{end}}{{end}}{{if .Copyright}} + COPYRIGHT: - {{.Copyright}} - {{end}} + {{.Copyright}}{{end}} ` // CommandHelpTemplate is the text template for the command help topic. From b377b5d9e93d2359a7ffe16fc1bb902b3df291b6 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 1 Nov 2016 20:33:12 -0700 Subject: [PATCH 26/45] Use type assertions rather than reflection to determine how to call the `Action` This has some benefits, but results in possibly less informative error messaging; however, given that there are only two accepted types, I think the error messaging is sufficient. --- app.go | 52 +++++++++++----------------------------------------- app_test.go | 6 +++--- 2 files changed, 14 insertions(+), 44 deletions(-) diff --git a/app.go b/app.go index dd2d2df..26cf09a 100644 --- a/app.go +++ b/app.go @@ -6,9 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" - "reflect" "sort" - "strings" "time" ) @@ -19,11 +17,8 @@ var ( contactSysadmin = "This is an error in the application. Please contact the distributor of this application if this is not you." - errNonFuncAction = NewExitError("ERROR invalid Action type. "+ - fmt.Sprintf("Must be a func of type `cli.ActionFunc`. %s", contactSysadmin)+ - fmt.Sprintf("See %s", appActionDeprecationURL), 2) - errInvalidActionSignature = NewExitError("ERROR invalid Action signature. "+ - fmt.Sprintf("Must be `cli.ActionFunc`. %s", contactSysadmin)+ + errInvalidActionType = NewExitError("ERROR invalid Action type. "+ + fmt.Sprintf("Must be `func(*Context`)` or `func(*Context) error). %s", contactSysadmin)+ fmt.Sprintf("See %s", appActionDeprecationURL), 2) ) @@ -468,41 +463,16 @@ func (a Author) String() string { return fmt.Sprintf("%v%v", a.Name, e) } -// HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an -// ActionFunc, a func with the legacy signature for Action, or some other -// invalid thing. If it's an ActionFunc or a func with the legacy signature for -// Action, the func is run! +// HandleAction attempts to figure out which Action signature was used. If +// it's an ActionFunc or a func with the legacy signature for Action, the func +// is run! func HandleAction(action interface{}, context *Context) (err error) { - defer func() { - if r := recover(); r != nil { - // Try to detect a known reflection error from *this scope*, rather than - // swallowing all panics that may happen when calling an Action func. - s := fmt.Sprintf("%v", r) - if strings.HasPrefix(s, "reflect: ") && strings.Contains(s, "too many input arguments") { - err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v.", r), 2) - } else { - panic(r) - } - } - }() - - if reflect.TypeOf(action).Kind() != reflect.Func { - return errNonFuncAction - } - - vals := reflect.ValueOf(action).Call([]reflect.Value{reflect.ValueOf(context)}) - - if len(vals) == 0 { + if a, ok := action.(func(*Context) error); ok { + return a(context) + } else if a, ok := action.(func(*Context)); ok { // deprecated function signature + a(context) return nil + } else { + return errInvalidActionType } - - if len(vals) > 1 { - return errInvalidActionSignature - } - - if retErr, ok := vals[0].Interface().(error); vals[0].IsValid() && ok { - return retErr - } - - return err } diff --git a/app_test.go b/app_test.go index e36a8c2..40a598d 100644 --- a/app_test.go +++ b/app_test.go @@ -1492,7 +1492,7 @@ func TestHandleAction_WithNonFuncAction(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type.") { t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error()) } @@ -1516,7 +1516,7 @@ func TestHandleAction_WithInvalidFuncSignature(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR unknown Action error") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error()) } @@ -1540,7 +1540,7 @@ func TestHandleAction_WithInvalidFuncReturnSignature(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action signature") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { t.Fatalf("expected an invalid Action signature error, but got: %v", exitErr.Error()) } From ea3df26e645524f2b56f2c6c4d81826ab43a3f45 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Fri, 4 Nov 2016 14:56:28 -0600 Subject: [PATCH 27/45] make shell autocomplete more robust --- app.go | 18 +++++++++--------- command.go | 29 ++++++++++++----------------- context.go | 9 ++++++++- help.go | 39 +++++++++++++++++++++++++++++++-------- 4 files changed, 60 insertions(+), 35 deletions(-) diff --git a/app.go b/app.go index 26cf09a..50edf9a 100644 --- a/app.go +++ b/app.go @@ -145,10 +145,6 @@ func (a *App) Setup() { } } - if a.EnableBashCompletion { - a.appendFlag(BashCompletionFlag) - } - if !a.HideVersion { a.appendFlag(VersionFlag) } @@ -173,6 +169,14 @@ func (a *App) Setup() { func (a *App) Run(arguments []string) (err error) { a.Setup() + // handle the completion flag separately from the flagset since + // completion could be attempted after a flag, but before its value was put + // on the command line. this causes the flagset to interpret the completion + // flag name as the value of the flag before it which is undesirable + // note that we can only do this because the shell autocomplete function + // always appends the completion flag at the end of the command + complete, arguments := checkCompleteFlag(a, arguments) + // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) @@ -184,6 +188,7 @@ func (a *App) Run(arguments []string) (err error) { ShowAppHelp(context) return nerr } + context.complete = complete if checkCompletions(context) { return nil @@ -283,11 +288,6 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { } a.Commands = newCmds - // append flags - if a.EnableBashCompletion { - a.appendFlag(BashCompletionFlag) - } - // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) diff --git a/command.go b/command.go index d955249..04eb0b8 100644 --- a/command.go +++ b/command.go @@ -87,10 +87,6 @@ func (c Command) Run(ctx *Context) (err error) { ) } - if ctx.App.EnableBashCompletion { - c.Flags = append(c.Flags, BashCompletionFlag) - } - set := flagSet(c.Name, c.Flags) set.SetOutput(ioutil.Discard) @@ -132,18 +128,6 @@ func (c Command) Run(ctx *Context) (err error) { err = set.Parse(ctx.Args().Tail()) } - if err != nil { - if c.OnUsageError != nil { - err := c.OnUsageError(ctx, err, false) - HandleExitCoder(err) - return err - } - fmt.Fprintln(ctx.App.Writer, "Incorrect Usage:", err.Error()) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) - return err - } - nerr := normalizeFlags(c.Flags, set) if nerr != nil { fmt.Fprintln(ctx.App.Writer, nerr) @@ -153,11 +137,22 @@ func (c Command) Run(ctx *Context) (err error) { } context := NewContext(ctx.App, set, ctx) - if checkCommandCompletions(context, c.Name) { return nil } + if err != nil { + if c.OnUsageError != nil { + err := c.OnUsageError(ctx, err, false) + HandleExitCoder(err) + return err + } + fmt.Fprintln(ctx.App.Writer, "Incorrect Usage:", err.Error()) + fmt.Fprintln(ctx.App.Writer) + ShowCommandHelp(ctx, c.Name) + return err + } + if checkCommandHelp(context, c.Name) { return nil } diff --git a/context.go b/context.go index 492a742..81404c4 100644 --- a/context.go +++ b/context.go @@ -15,6 +15,7 @@ import ( type Context struct { App *App Command Command + complete bool flagSet *flag.FlagSet setFlags map[string]bool parentContext *Context @@ -22,7 +23,13 @@ type Context struct { // NewContext creates a new context. For use in when invoking an App or Command action. func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { - return &Context{App: app, flagSet: set, parentContext: parentCtx} + c := &Context{App: app, flagSet: set, parentContext: parentCtx} + + if parentCtx != nil { + c.complete = parentCtx.complete + } + + return c } // NumFlags returns the number of flags set diff --git a/help.go b/help.go index 515f744..e453e72 100644 --- a/help.go +++ b/help.go @@ -252,20 +252,43 @@ func checkSubcommandHelp(c *Context) bool { return false } +func checkCompleteFlag(a *App, arguments []string) (bool, []string) { + if !a.EnableBashCompletion { + return false, arguments + } + + pos := len(arguments) - 1 + lastArg := arguments[pos] + + if lastArg != "--"+BashCompletionFlag.Name { + return false, arguments + } + + return true, arguments[:pos] +} + func checkCompletions(c *Context) bool { - if (c.GlobalBool(BashCompletionFlag.Name) || c.Bool(BashCompletionFlag.Name)) && c.App.EnableBashCompletion { - ShowCompletions(c) - return true + if !c.complete { + return false } - return false + if args := c.Args(); args.Present() { + name := args.First() + if cmd := c.App.Command(name); cmd != nil { + // let the command handle the completion + return false + } + } + + ShowCompletions(c) + return true } func checkCommandCompletions(c *Context, name string) bool { - if c.Bool(BashCompletionFlag.Name) && c.App.EnableBashCompletion { - ShowCommandCompletions(c, name) - return true + if !c.complete { + return false } - return false + ShowCommandCompletions(c, name) + return true } From 79591889a97af09f177601e3e9e61f9c7e359359 Mon Sep 17 00:00:00 2001 From: mh-cbon Date: Sat, 5 Nov 2016 10:27:46 +0100 Subject: [PATCH 28/45] Close #558: detect FormattedError and print their stack trace --- errors.go | 14 ++++++++++---- errors_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/errors.go b/errors.go index ddef369..e0f295e 100644 --- a/errors.go +++ b/errors.go @@ -34,6 +34,10 @@ func (m MultiError) Error() string { return strings.Join(errs, "\n") } +type ErrorFormatter interface { + Format(s fmt.State, verb rune) +} + // ExitCoder is the interface checked by `App` and `Command` for a custom exit // code type ExitCoder interface { @@ -44,11 +48,11 @@ type ExitCoder interface { // ExitError fulfills both the builtin `error` interface and `ExitCoder` type ExitError struct { exitCode int - message string + message interface{} } // NewExitError makes a new *ExitError -func NewExitError(message string, exitCode int) *ExitError { +func NewExitError(message interface{}, exitCode int) *ExitError { return &ExitError{ exitCode: exitCode, message: message, @@ -58,7 +62,7 @@ func NewExitError(message string, exitCode int) *ExitError { // Error returns the string message, fulfilling the interface required by // `error` func (ee *ExitError) Error() string { - return ee.message + return fmt.Sprintf("%v", ee.message) } // ExitCode returns the exit code, fulfilling the interface required by @@ -77,7 +81,9 @@ func HandleExitCoder(err error) { } if exitErr, ok := err.(ExitCoder); ok { - if err.Error() != "" { + if _, ok := exitErr.(ErrorFormatter); ok { + fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { fmt.Fprintln(ErrWriter, err) } OsExiter(exitErr.ExitCode()) diff --git a/errors_test.go b/errors_test.go index 04df031..e6dfc16 100644 --- a/errors_test.go +++ b/errors_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "errors" + "fmt" "testing" ) @@ -104,3 +105,32 @@ func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { expect(t, called, true) expect(t, ErrWriter.(*bytes.Buffer).String(), "") } + +// make a stub to not import pkg/errors +type ErrorWithFormat struct { + error +} + +func (f *ErrorWithFormat) Format(s fmt.State, verb rune) { + fmt.Fprintf(s, "This the format: %v", f.error) +} + +func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { + called := false + + OsExiter = func(rc int) { + called = true + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + err := &ErrorWithFormat{error: errors.New("I am formatted")} + HandleExitCoder(err) + + expect(t, called, true) + expect(t, ErrWriter.(*bytes.Buffer).String(), "This the format: I am formatted\n") +} From 0113f56d1094e8565f6d1ad10526865fd61d57aa Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 12 Nov 2016 13:37:07 -0800 Subject: [PATCH 29/45] If no action is specified on the command or app, print the help documentation Rather than panic'ing or displaying an opaque error message about the signature which is more confusing to the end user. Fixes #562 --- app.go | 4 ++++ app_test.go | 43 +++++++++++++++++++++++++++++++++++++++++++ command.go | 4 ++++ help.go | 2 +- 4 files changed, 52 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index 26cf09a..a90a4df 100644 --- a/app.go +++ b/app.go @@ -242,6 +242,10 @@ func (a *App) Run(arguments []string) (err error) { } } + if a.Action == nil { + a.Action = helpCommand.Action + } + // Run default Action err = HandleAction(a.Action, context) diff --git a/app_test.go b/app_test.go index 40a598d..711de47 100644 --- a/app_test.go +++ b/app_test.go @@ -178,6 +178,49 @@ func ExampleApp_Run_commandHelp() { // This is how we describe describeit the function } +func ExampleApp_Run_noAction() { + app := App{} + app.Name = "greet" + app.Run([]string{"greet"}) + // Output: + // NAME: + // greet + // + // USAGE: + // [global options] command [command options] [arguments...] + // + // COMMANDS: + // help, h Shows a list of commands or help for one command + // + // GLOBAL OPTIONS: + // --help, -h show help + // --version, -v print the version +} + +func ExampleApp_Run_subcommandNoAction() { + app := App{} + app.Name = "greet" + app.Commands = []Command{ + { + Name: "describeit", + Aliases: []string{"d"}, + Usage: "use it to see a description", + Description: "This is how we describe describeit the function", + }, + } + app.Run([]string{"greet", "describeit"}) + // Output: + // NAME: + // describeit - use it to see a description + // + // USAGE: + // describeit [arguments...] + // + // DESCRIPTION: + // This is how we describe describeit the function + +} + func ExampleApp_Run_bashComplete() { // set args for examples sake os.Args = []string{"greet", "--generate-bash-completion"} diff --git a/command.go b/command.go index d955249..8f1a215 100644 --- a/command.go +++ b/command.go @@ -187,6 +187,10 @@ func (c Command) Run(ctx *Context) (err error) { } } + if c.Action == nil { + c.Action = helpSubcommand.Action + } + context.Command = c err = HandleAction(c.Action, context) diff --git a/help.go b/help.go index 515f744..5ee11a6 100644 --- a/help.go +++ b/help.go @@ -13,7 +13,7 @@ import ( // cli.go uses text/template to render templates. You can // render custom help text by setting this variable. var AppHelpTemplate = `NAME: - {{.Name}} - {{.Usage}} + {{.Name}}{{if .Usage}} - {{.Usage}}{{end}} USAGE: {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Version}}{{if not .HideVersion}} From b0a8f25773c10b7445d0d740d380a56e3dcd2166 Mon Sep 17 00:00:00 2001 From: mh-cbon Date: Sun, 13 Nov 2016 22:20:13 +0100 Subject: [PATCH 30/45] 558: handle multi formatter errors --- errors.go | 16 +++++++++++----- errors_test.go | 23 ++++++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/errors.go b/errors.go index e0f295e..0206ff4 100644 --- a/errors.go +++ b/errors.go @@ -81,10 +81,12 @@ func HandleExitCoder(err error) { } if exitErr, ok := err.(ExitCoder); ok { - if _, ok := exitErr.(ErrorFormatter); ok { - fmt.Fprintf(ErrWriter, "%+v\n", err) - } else { - fmt.Fprintln(ErrWriter, err) + if err.Error() != "" { + if _, ok := exitErr.(ErrorFormatter); ok { + fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { + fmt.Fprintln(ErrWriter, err) + } } OsExiter(exitErr.ExitCode()) return @@ -98,7 +100,11 @@ func HandleExitCoder(err error) { } if err.Error() != "" { - fmt.Fprintln(ErrWriter, err) + if _, ok := err.(ErrorFormatter); ok { + fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { + fmt.Fprintln(ErrWriter, err) + } } OsExiter(1) } diff --git a/errors_test.go b/errors_test.go index e6dfc16..131bd38 100644 --- a/errors_test.go +++ b/errors_test.go @@ -111,6 +111,10 @@ type ErrorWithFormat struct { error } +func NewErrorWithFormat(m string) *ErrorWithFormat { + return &ErrorWithFormat{error: errors.New(m)} +} + func (f *ErrorWithFormat) Format(s fmt.State, verb rune) { fmt.Fprintf(s, "This the format: %v", f.error) } @@ -128,9 +132,26 @@ func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { ErrWriter = fakeErrWriter }() - err := &ErrorWithFormat{error: errors.New("I am formatted")} + err := NewErrorWithFormat("I am formatted") HandleExitCoder(err) expect(t, called, true) expect(t, ErrWriter.(*bytes.Buffer).String(), "This the format: I am formatted\n") } + +func TestHandleExitCoder_MultiErrorWithFormat(t *testing.T) { + called := false + + OsExiter = func(rc int) { + called = true + } + ErrWriter = &bytes.Buffer{} + + defer func() { OsExiter = fakeOsExiter }() + + err := NewMultiError(NewErrorWithFormat("err1"), NewErrorWithFormat("err2")) + HandleExitCoder(err) + + expect(t, called, true) + expect(t, ErrWriter.(*bytes.Buffer).String(), "This the format: err1\nThis the format: err2\n") +} From a00c3f5872b3129785af69ee16a2912cd58079d3 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 11 Sep 2016 20:32:43 -0700 Subject: [PATCH 31/45] Consider empty environment variables as set When assigning values to flags (also when interogatting via `context.(Global)IsSet`. For boolean flags, consider empty as `false`. Using `syscall.Getenv` rather than `os.LookupEnv` in order to support older Golang versions. --- altsrc/flag.go | 8 +++---- context.go | 4 ++-- context_test.go | 8 ++++++- flag.go | 38 ++++++++++++++++++----------- flag_test.go | 64 +++++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 100 insertions(+), 22 deletions(-) diff --git a/altsrc/flag.go b/altsrc/flag.go index ec14e40..84ef009 100644 --- a/altsrc/flag.go +++ b/altsrc/flag.go @@ -2,9 +2,9 @@ package altsrc import ( "fmt" - "os" "strconv" "strings" + "syscall" "gopkg.in/urfave/cli.v1" ) @@ -237,13 +237,11 @@ func (f *Float64Flag) ApplyInputSourceValue(context *cli.Context, isc InputSourc func isEnvVarSet(envVars string) bool { for _, envVar := range strings.Split(envVars, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if _, ok := syscall.Getenv(envVar); ok { // TODO: Can't use this for bools as // set means that it was true or false based on // Bool flag type, should work for other types - if len(envVal) > 0 { - return true - } + return true } } diff --git a/context.go b/context.go index 492a742..3893a50 100644 --- a/context.go +++ b/context.go @@ -3,9 +3,9 @@ package cli import ( "errors" "flag" - "os" "reflect" "strings" + "syscall" ) // Context is a type that is passed through to @@ -91,7 +91,7 @@ func (c *Context) IsSet(name string) bool { eachName(envVarValue.String(), func(envVar string) { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if _, ok := syscall.Getenv(envVar); ok { c.setFlags[name] = true return } diff --git a/context_test.go b/context_test.go index 0cf84d1..b5bc993 100644 --- a/context_test.go +++ b/context_test.go @@ -184,18 +184,22 @@ func TestContext_IsSet(t *testing.T) { // XXX Corresponds to hack in context.IsSet for flags with EnvVar field // Should be moved to `flag_test` in v2 func TestContext_IsSet_fromEnv(t *testing.T) { - var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet, passwordIsSet, pIsSet bool os.Clearenv() os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + os.Setenv("APP_PASSWORD", "") a := App{ Flags: []Flag{ Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + Float64Flag{Name: "password, p", EnvVar: "APP_PASSWORD"}, Float64Flag{Name: "no-env-var, n"}, }, Action: func(ctx *Context) error { timeoutIsSet = ctx.IsSet("timeout") tIsSet = ctx.IsSet("t") + passwordIsSet = ctx.IsSet("password") + pIsSet = ctx.IsSet("p") noEnvVarIsSet = ctx.IsSet("no-env-var") nIsSet = ctx.IsSet("n") return nil @@ -204,6 +208,8 @@ func TestContext_IsSet_fromEnv(t *testing.T) { a.Run([]string{"run"}) expect(t, timeoutIsSet, true) expect(t, tIsSet, true) + expect(t, passwordIsSet, true) + expect(t, pIsSet, true) expect(t, noEnvVarIsSet, false) expect(t, nIsSet, false) } diff --git a/flag.go b/flag.go index 1ff28d3..5e565fb 100644 --- a/flag.go +++ b/flag.go @@ -3,11 +3,11 @@ package cli import ( "flag" "fmt" - "os" "reflect" "runtime" "strconv" "strings" + "syscall" "time" ) @@ -92,7 +92,7 @@ func (f GenericFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { val.Set(envVal) break } @@ -128,7 +128,7 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { newVal := &StringSlice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) @@ -176,7 +176,7 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { newVal := &IntSlice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) @@ -227,7 +227,7 @@ func (f Int64SliceFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { newVal := &Int64Slice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) @@ -256,7 +256,12 @@ func (f BoolFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { + if envVal == "" { + val = false + break + } + envValBool, err := strconv.ParseBool(envVal) if err == nil { val = envValBool @@ -281,7 +286,12 @@ func (f BoolTFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { + if envVal == "" { + val = false + break + } + envValBool, err := strconv.ParseBool(envVal) if err == nil { val = envValBool @@ -305,7 +315,7 @@ func (f StringFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { f.Value = envVal break } @@ -326,7 +336,7 @@ func (f IntFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseInt(envVal, 0, 64) if err == nil { f.Value = int(envValInt) @@ -350,7 +360,7 @@ func (f Int64Flag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseInt(envVal, 0, 64) if err == nil { f.Value = envValInt @@ -374,7 +384,7 @@ func (f UintFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseUint(envVal, 0, 64) if err == nil { f.Value = uint(envValInt) @@ -398,7 +408,7 @@ func (f Uint64Flag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseUint(envVal, 0, 64) if err == nil { f.Value = uint64(envValInt) @@ -422,7 +432,7 @@ func (f DurationFlag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValDuration, err := time.ParseDuration(envVal) if err == nil { f.Value = envValDuration @@ -446,7 +456,7 @@ func (f Float64Flag) Apply(set *flag.FlagSet) { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { envValFloat, err := strconv.ParseFloat(envVal, 10) if err == nil { f.Value = float64(envValFloat) diff --git a/flag_test.go b/flag_test.go index a7afcc4..58ada36 100644 --- a/flag_test.go +++ b/flag_test.go @@ -29,6 +29,38 @@ func TestBoolFlagHelpOutput(t *testing.T) { } } +func TestParseBoolFromEnv(t *testing.T) { + var boolFlagTests = []struct { + input string + output bool + }{ + {"", false}, + {"1", true}, + {"false", false}, + {"true", true}, + } + + for _, test := range boolFlagTests { + os.Clearenv() + os.Setenv("DEBUG", test.input) + a := App{ + Flags: []Flag{ + BoolFlag{Name: "debug, d", EnvVar: "DEBUG"}, + }, + Action: func(ctx *Context) error { + if ctx.Bool("debug") != test.output { + t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("debug")) + } + if ctx.Bool("d") != test.output { + t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("d")) + } + return nil + }, + } + a.Run([]string{"run"}) + } +} + var stringFlagTests = []struct { name string usage string @@ -941,6 +973,38 @@ func TestParseMultiBoolFromEnvCascade(t *testing.T) { a.Run([]string{"run"}) } +func TestParseBoolTFromEnv(t *testing.T) { + var boolTFlagTests = []struct { + input string + output bool + }{ + {"", false}, + {"1", true}, + {"false", false}, + {"true", true}, + } + + for _, test := range boolTFlagTests { + os.Clearenv() + os.Setenv("DEBUG", test.input) + a := App{ + Flags: []Flag{ + BoolTFlag{Name: "debug, d", EnvVar: "DEBUG"}, + }, + Action: func(ctx *Context) error { + if ctx.Bool("debug") != test.output { + t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("debug")) + } + if ctx.Bool("d") != test.output { + t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("d")) + } + return nil + }, + } + a.Run([]string{"run"}) + } +} + func TestParseMultiBoolT(t *testing.T) { a := App{ Flags: []Flag{ From e367fafa3d3ce89a09e75d1650a9a73057a9bf36 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 17 Sep 2016 16:54:29 -0700 Subject: [PATCH 32/45] Return an error when parsing environment variables for values fails Currently cli silently (aside from IntSlice and Int64Slice which oddly printed directly to the error stream) ignores failures that occur when parsing environment variables for their value for flags that define environment variables. Instead, we should propogate up the error to the user. This is accomplished in a backwards compatible manner by adding a new, internal, interface which defines an applyWithError function that all flags here define. In v2, when we can modify the interface, we can drop this secondary interface and modify `Apply` to return an error. --- app.go | 12 ++- app_test.go | 24 +++++- command.go | 5 +- context.go | 5 ++ context_test.go | 44 +++++++++- flag.go | 225 ++++++++++++++++++++++++++++++++++++++++-------- flag_test.go | 83 ++++++++++++++---- 7 files changed, 333 insertions(+), 65 deletions(-) diff --git a/app.go b/app.go index a90a4df..95aa5ed 100644 --- a/app.go +++ b/app.go @@ -174,7 +174,11 @@ func (a *App) Run(arguments []string) (err error) { a.Setup() // parse flags - set := flagSet(a.Name, a.Flags) + set, err := flagSet(a.Name, a.Flags) + if err != nil { + return err + } + set.SetOutput(ioutil.Discard) err = set.Parse(arguments[1:]) nerr := normalizeFlags(a.Flags, set) @@ -293,7 +297,11 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { } // parse flags - set := flagSet(a.Name, a.Flags) + set, err := flagSet(a.Name, a.Flags) + if err != nil { + return err + } + set.SetOutput(ioutil.Discard) err = set.Parse(ctx.Args().Tail()) nerr := normalizeFlags(a.Flags, set) diff --git a/app_test.go b/app_test.go index 711de47..83d096f 100644 --- a/app_test.go +++ b/app_test.go @@ -1523,7 +1523,11 @@ func TestApp_OnUsageError_WithWrongFlagValue_ForSubcommand(t *testing.T) { func TestHandleAction_WithNonFuncAction(t *testing.T) { app := NewApp() app.Action = 42 - err := HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + err = HandleAction(app.Action, NewContext(app, fs, nil)) if err == nil { t.Fatalf("expected to receive error from Run, got none") @@ -1547,7 +1551,11 @@ func TestHandleAction_WithNonFuncAction(t *testing.T) { func TestHandleAction_WithInvalidFuncSignature(t *testing.T) { app := NewApp() app.Action = func() string { return "" } - err := HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + err = HandleAction(app.Action, NewContext(app, fs, nil)) if err == nil { t.Fatalf("expected to receive error from Run, got none") @@ -1571,7 +1579,11 @@ func TestHandleAction_WithInvalidFuncSignature(t *testing.T) { func TestHandleAction_WithInvalidFuncReturnSignature(t *testing.T) { app := NewApp() app.Action = func(_ *Context) (int, error) { return 0, nil } - err := HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + err = HandleAction(app.Action, NewContext(app, fs, nil)) if err == nil { t.Fatalf("expected to receive error from Run, got none") @@ -1602,5 +1614,9 @@ func TestHandleAction_WithUnknownPanic(t *testing.T) { fn(ctx) return nil } - HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + HandleAction(app.Action, NewContext(app, fs, nil)) } diff --git a/command.go b/command.go index 8f1a215..3cc643d 100644 --- a/command.go +++ b/command.go @@ -91,7 +91,10 @@ func (c Command) Run(ctx *Context) (err error) { c.Flags = append(c.Flags, BashCompletionFlag) } - set := flagSet(c.Name, c.Flags) + set, err := flagSet(c.Name, c.Flags) + if err != nil { + return err + } set.SetOutput(ioutil.Discard) if c.SkipFlagParsing { diff --git a/context.go b/context.go index 3893a50..19de0d8 100644 --- a/context.go +++ b/context.go @@ -147,6 +147,11 @@ func (c *Context) Parent() *Context { return c.parentContext } +// value returns the value of the flag coressponding to `name` +func (c *Context) value(name string) interface{} { + return c.flagSet.Lookup(name).Value.(flag.Getter).Get() +} + // Args contains apps console arguments type Args []string diff --git a/context_test.go b/context_test.go index b5bc993..7a6eebf 100644 --- a/context_test.go +++ b/context_test.go @@ -184,7 +184,12 @@ func TestContext_IsSet(t *testing.T) { // XXX Corresponds to hack in context.IsSet for flags with EnvVar field // Should be moved to `flag_test` in v2 func TestContext_IsSet_fromEnv(t *testing.T) { - var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet, passwordIsSet, pIsSet bool + var ( + timeoutIsSet, tIsSet bool + noEnvVarIsSet, nIsSet bool + passwordIsSet, pIsSet bool + unparsableIsSet, uIsSet bool + ) os.Clearenv() os.Setenv("APP_TIMEOUT_SECONDS", "15.5") @@ -192,7 +197,8 @@ func TestContext_IsSet_fromEnv(t *testing.T) { a := App{ Flags: []Flag{ Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, - Float64Flag{Name: "password, p", EnvVar: "APP_PASSWORD"}, + StringFlag{Name: "password, p", EnvVar: "APP_PASSWORD"}, + Float64Flag{Name: "unparsable, u", EnvVar: "APP_UNPARSABLE"}, Float64Flag{Name: "no-env-var, n"}, }, Action: func(ctx *Context) error { @@ -200,6 +206,8 @@ func TestContext_IsSet_fromEnv(t *testing.T) { tIsSet = ctx.IsSet("t") passwordIsSet = ctx.IsSet("password") pIsSet = ctx.IsSet("p") + unparsableIsSet = ctx.IsSet("unparsable") + uIsSet = ctx.IsSet("u") noEnvVarIsSet = ctx.IsSet("no-env-var") nIsSet = ctx.IsSet("n") return nil @@ -212,6 +220,11 @@ func TestContext_IsSet_fromEnv(t *testing.T) { expect(t, pIsSet, true) expect(t, noEnvVarIsSet, false) expect(t, nIsSet, false) + + os.Setenv("APP_UNPARSABLE", "foobar") + a.Run([]string{"run"}) + expect(t, unparsableIsSet, false) + expect(t, uIsSet, false) } func TestContext_GlobalIsSet(t *testing.T) { @@ -236,14 +249,22 @@ func TestContext_GlobalIsSet(t *testing.T) { // XXX Corresponds to hack in context.IsSet for flags with EnvVar field // Should be moved to `flag_test` in v2 func TestContext_GlobalIsSet_fromEnv(t *testing.T) { - var timeoutIsSet, tIsSet, noEnvVarIsSet, nIsSet bool + var ( + timeoutIsSet, tIsSet bool + noEnvVarIsSet, nIsSet bool + passwordIsSet, pIsSet bool + unparsableIsSet, uIsSet bool + ) os.Clearenv() os.Setenv("APP_TIMEOUT_SECONDS", "15.5") + os.Setenv("APP_PASSWORD", "") a := App{ Flags: []Flag{ Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, + StringFlag{Name: "password, p", EnvVar: "APP_PASSWORD"}, Float64Flag{Name: "no-env-var, n"}, + Float64Flag{Name: "unparsable, u", EnvVar: "APP_UNPARSABLE"}, }, Commands: []Command{ { @@ -251,6 +272,10 @@ func TestContext_GlobalIsSet_fromEnv(t *testing.T) { Action: func(ctx *Context) error { timeoutIsSet = ctx.GlobalIsSet("timeout") tIsSet = ctx.GlobalIsSet("t") + passwordIsSet = ctx.GlobalIsSet("password") + pIsSet = ctx.GlobalIsSet("p") + unparsableIsSet = ctx.GlobalIsSet("unparsable") + uIsSet = ctx.GlobalIsSet("u") noEnvVarIsSet = ctx.GlobalIsSet("no-env-var") nIsSet = ctx.GlobalIsSet("n") return nil @@ -258,11 +283,22 @@ func TestContext_GlobalIsSet_fromEnv(t *testing.T) { }, }, } - a.Run([]string{"run", "hello"}) + if err := a.Run([]string{"run", "hello"}); err != nil { + t.Logf("error running Run(): %+v", err) + } expect(t, timeoutIsSet, true) expect(t, tIsSet, true) + expect(t, passwordIsSet, true) + expect(t, pIsSet, true) expect(t, noEnvVarIsSet, false) expect(t, nIsSet, false) + + os.Setenv("APP_UNPARSABLE", "foobar") + if err := a.Run([]string{"run"}); err != nil { + t.Logf("error running Run(): %+v", err) + } + expect(t, unparsableIsSet, false) + expect(t, uIsSet, false) } func TestContext_NumFlags(t *testing.T) { diff --git a/flag.go b/flag.go index 5e565fb..3e63cf0 100644 --- a/flag.go +++ b/flag.go @@ -62,13 +62,29 @@ type Flag interface { GetName() string } -func flagSet(name string, flags []Flag) *flag.FlagSet { +// errorableFlag is an interface that allows us to return errors during apply +// it allows flags defined in this library to return errors in a fashion backwards compatible +// TODO remove in v2 and modify the existing Flag interface to return errors +type errorableFlag interface { + Flag + + applyWithError(*flag.FlagSet) error +} + +func flagSet(name string, flags []Flag) (*flag.FlagSet, error) { set := flag.NewFlagSet(name, flag.ContinueOnError) for _, f := range flags { - f.Apply(set) + //TODO remove in v2 when errorableFlag is removed + if f, ok := f.(errorableFlag); ok { + if err := f.applyWithError(set); err != nil { + return nil, err + } + } else { + f.Apply(set) + } } - return set + return set, nil } func eachName(longName string, fn func(string)) { @@ -87,13 +103,22 @@ type Generic interface { // Apply takes the flagset and calls Set on the generic flag with the value // provided by the user for parsing by the flag +// Ignores parsing errors func (f GenericFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError takes the flagset and calls Set on the generic flag with the value +// provided by the user for parsing by the flag +func (f GenericFlag) applyWithError(set *flag.FlagSet) error { val := f.Value if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { - val.Set(envVal) + if err := val.Set(envVal); err != nil { + return fmt.Errorf("could not parse %s as value for flag %s: %s", envVal, f.Name, err) + } break } } @@ -102,9 +127,11 @@ func (f GenericFlag) Apply(set *flag.FlagSet) { eachName(f.Name, func(name string) { set.Var(f.Value, name, f.Usage) }) + + return nil } -// StringSlice is an opaque type for []string to satisfy flag.Value +// StringSlice is an opaque type for []string to satisfy flag.Value and flag.Getter type StringSlice []string // Set appends the string value to the list of values @@ -123,8 +150,19 @@ func (f *StringSlice) Value() []string { return *f } +// Get returns the slice of strings set by this flag +func (f *StringSlice) Get() interface{} { + return *f +} + // Apply populates the flag given the flag set and environment +// Ignores errors func (f StringSliceFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f StringSliceFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -132,7 +170,9 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { newVal := &StringSlice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) - newVal.Set(s) + if err := newVal.Set(s); err != nil { + return fmt.Errorf("could not parse %s as string value for flag %s: %s", envVal, f.Name, err) + } } f.Value = newVal break @@ -146,9 +186,11 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { } set.Var(f.Value, name, f.Usage) }) + + return nil } -// IntSlice is an opaque type for []int to satisfy flag.Value +// IntSlice is an opaque type for []int to satisfy flag.Value and flag.Getter type IntSlice []int // Set parses the value into an integer and appends it to the list of values @@ -171,8 +213,19 @@ func (f *IntSlice) Value() []int { return *f } +// Get returns the slice of ints set by this flag +func (f *IntSlice) Get() interface{} { + return *f +} + // Apply populates the flag given the flag set and environment +// Ignores errors func (f IntSliceFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f IntSliceFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -180,9 +233,8 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { newVal := &IntSlice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) - err := newVal.Set(s) - if err != nil { - fmt.Fprintf(ErrWriter, err.Error()) + if err := newVal.Set(s); err != nil { + return fmt.Errorf("could not parse %s as int slice value for flag %s: %s", envVal, f.Name, err) } } f.Value = newVal @@ -197,9 +249,11 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { } set.Var(f.Value, name, f.Usage) }) + + return nil } -// Int64Slice is an opaque type for []int to satisfy flag.Value +// Int64Slice is an opaque type for []int to satisfy flag.Value and flag.Getter type Int64Slice []int64 // Set parses the value into an integer and appends it to the list of values @@ -222,8 +276,19 @@ func (f *Int64Slice) Value() []int64 { return *f } +// Get returns the slice of ints set by this flag +func (f *Int64Slice) Get() interface{} { + return *f +} + // Apply populates the flag given the flag set and environment +// Ignores errors func (f Int64SliceFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f Int64SliceFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -231,9 +296,8 @@ func (f Int64SliceFlag) Apply(set *flag.FlagSet) { newVal := &Int64Slice{} for _, s := range strings.Split(envVal, ",") { s = strings.TrimSpace(s) - err := newVal.Set(s) - if err != nil { - fmt.Fprintf(ErrWriter, err.Error()) + if err := newVal.Set(s); err != nil { + return fmt.Errorf("could not parse %s as int64 slice value for flag %s: %s", envVal, f.Name, err) } } f.Value = newVal @@ -248,10 +312,17 @@ func (f Int64SliceFlag) Apply(set *flag.FlagSet) { } set.Var(f.Value, name, f.Usage) }) + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f BoolFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f BoolFlag) applyWithError(set *flag.FlagSet) error { val := false if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { @@ -263,9 +334,11 @@ func (f BoolFlag) Apply(set *flag.FlagSet) { } envValBool, err := strconv.ParseBool(envVal) - if err == nil { - val = envValBool + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) } + + val = envValBool break } } @@ -278,10 +351,18 @@ func (f BoolFlag) Apply(set *flag.FlagSet) { } set.Bool(name, val, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f BoolTFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f BoolTFlag) applyWithError(set *flag.FlagSet) error { val := true if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { @@ -293,10 +374,12 @@ func (f BoolTFlag) Apply(set *flag.FlagSet) { } envValBool, err := strconv.ParseBool(envVal) - if err == nil { - val = envValBool - break + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) } + + val = envValBool + break } } } @@ -308,10 +391,18 @@ func (f BoolTFlag) Apply(set *flag.FlagSet) { } set.Bool(name, val, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f StringFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f StringFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -329,19 +420,28 @@ func (f StringFlag) Apply(set *flag.FlagSet) { } set.String(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f IntFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f IntFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseInt(envVal, 0, 64) - if err == nil { - f.Value = int(envValInt) - break + if err != nil { + return fmt.Errorf("could not parse %s as int value for flag %s: %s", envVal, f.Name, err) } + f.Value = int(envValInt) + break } } } @@ -353,19 +453,29 @@ func (f IntFlag) Apply(set *flag.FlagSet) { } set.Int(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f Int64Flag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f Int64Flag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseInt(envVal, 0, 64) - if err == nil { - f.Value = envValInt - break + if err != nil { + return fmt.Errorf("could not parse %s as int value for flag %s: %s", envVal, f.Name, err) } + + f.Value = envValInt + break } } } @@ -377,19 +487,29 @@ func (f Int64Flag) Apply(set *flag.FlagSet) { } set.Int64(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f UintFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f UintFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseUint(envVal, 0, 64) - if err == nil { - f.Value = uint(envValInt) - break + if err != nil { + return fmt.Errorf("could not parse %s as uint value for flag %s: %s", envVal, f.Name, err) } + + f.Value = uint(envValInt) + break } } } @@ -401,19 +521,29 @@ func (f UintFlag) Apply(set *flag.FlagSet) { } set.Uint(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f Uint64Flag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f Uint64Flag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValInt, err := strconv.ParseUint(envVal, 0, 64) - if err == nil { - f.Value = uint64(envValInt) - break + if err != nil { + return fmt.Errorf("could not parse %s as uint64 value for flag %s: %s", envVal, f.Name, err) } + + f.Value = uint64(envValInt) + break } } } @@ -425,19 +555,29 @@ func (f Uint64Flag) Apply(set *flag.FlagSet) { } set.Uint64(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f DurationFlag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f DurationFlag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValDuration, err := time.ParseDuration(envVal) - if err == nil { - f.Value = envValDuration - break + if err != nil { + return fmt.Errorf("could not parse %s as duration for flag %s: %s", envVal, f.Name, err) } + + f.Value = envValDuration + break } } } @@ -449,18 +589,29 @@ func (f DurationFlag) Apply(set *flag.FlagSet) { } set.Duration(name, f.Value, f.Usage) }) + + return nil } // Apply populates the flag given the flag set and environment +// Ignores errors func (f Float64Flag) Apply(set *flag.FlagSet) { + f.applyWithError(set) +} + +// applyWithError populates the flag given the flag set and environment +func (f Float64Flag) applyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { envValFloat, err := strconv.ParseFloat(envVal, 10) - if err == nil { - f.Value = float64(envValFloat) + if err != nil { + return fmt.Errorf("could not parse %s as float64 value for flag %s: %s", envVal, f.Name, err) } + + f.Value = float64(envValFloat) + break } } } @@ -472,6 +623,8 @@ func (f Float64Flag) Apply(set *flag.FlagSet) { } set.Float64(name, f.Value, f.Usage) }) + + return nil } func visibleFlags(fl []Flag) []Flag { diff --git a/flag_test.go b/flag_test.go index 58ada36..0dd8654 100644 --- a/flag_test.go +++ b/flag_test.go @@ -29,35 +29,78 @@ func TestBoolFlagHelpOutput(t *testing.T) { } } -func TestParseBoolFromEnv(t *testing.T) { - var boolFlagTests = []struct { +func TestFlagsFromEnv(t *testing.T) { + var flagTests = []struct { input string - output bool + output interface{} + flag Flag + err error }{ - {"", false}, - {"1", true}, - {"false", false}, - {"true", true}, + {"", 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, 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`)}, + + {"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`)}, + + {"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", 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", 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,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", 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`)}, + + {"foo", "foo", StringFlag{Name: "name", EnvVar: "NAME"}, nil}, + + {"foo,bar", StringSlice{"foo", "bar"}, StringSliceFlag{Name: "names", EnvVar: "NAMES"}, nil}, + + {"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", 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`)}, + + {"foo,bar", &Parser{"foo", "bar"}, GenericFlag{Name: "names", Value: &Parser{}, EnvVar: "NAMES"}, nil}, } - for _, test := range boolFlagTests { + for _, test := range flagTests { os.Clearenv() - os.Setenv("DEBUG", test.input) + os.Setenv(reflect.ValueOf(test.flag).FieldByName("EnvVar").String(), test.input) a := App{ - Flags: []Flag{ - BoolFlag{Name: "debug, d", EnvVar: "DEBUG"}, - }, + Flags: []Flag{test.flag}, Action: func(ctx *Context) error { - if ctx.Bool("debug") != test.output { - t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("debug")) - } - if ctx.Bool("d") != test.output { - t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.Bool("d")) + if !reflect.DeepEqual(ctx.value(test.flag.GetName()), test.output) { + t.Errorf("expected %+v to be parsed as %+v, instead was %+v", test.input, test.output, ctx.value(test.flag.GetName())) } return nil }, } - a.Run([]string{"run"}) + + err := a.Run([]string{"run"}) + if !reflect.DeepEqual(test.err, err) { + t.Errorf("expected error %s, got error %s", test.err, err) + } } } @@ -1100,6 +1143,10 @@ func (p *Parser) String() string { return fmt.Sprintf("%s,%s", p[0], p[1]) } +func (p *Parser) Get() interface{} { + return p +} + func TestParseGeneric(t *testing.T) { a := App{ Flags: []Flag{ From b4a64dc08db6f198cd94c009885a6a83c0ad14c5 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 12 Nov 2016 20:01:44 -0800 Subject: [PATCH 33/45] Add windows implementation of Clearenv for tests Apparently `Clearenv` in Windows just sets the variables to "" --- context_test.go | 4 ++-- helpers_unix_test.go | 9 +++++++++ helpers_windows_test.go | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 helpers_unix_test.go create mode 100644 helpers_windows_test.go diff --git a/context_test.go b/context_test.go index 7a6eebf..a1ab05b 100644 --- a/context_test.go +++ b/context_test.go @@ -191,7 +191,7 @@ func TestContext_IsSet_fromEnv(t *testing.T) { unparsableIsSet, uIsSet bool ) - os.Clearenv() + clearenv() os.Setenv("APP_TIMEOUT_SECONDS", "15.5") os.Setenv("APP_PASSWORD", "") a := App{ @@ -256,7 +256,7 @@ func TestContext_GlobalIsSet_fromEnv(t *testing.T) { unparsableIsSet, uIsSet bool ) - os.Clearenv() + clearenv() os.Setenv("APP_TIMEOUT_SECONDS", "15.5") os.Setenv("APP_PASSWORD", "") a := App{ diff --git a/helpers_unix_test.go b/helpers_unix_test.go new file mode 100644 index 0000000..ae27fc5 --- /dev/null +++ b/helpers_unix_test.go @@ -0,0 +1,9 @@ +// +build darwin dragonfly freebsd linux netbsd openbsd solaris + +package cli + +import "os" + +func clearenv() { + os.Clearenv() +} diff --git a/helpers_windows_test.go b/helpers_windows_test.go new file mode 100644 index 0000000..4eb84f9 --- /dev/null +++ b/helpers_windows_test.go @@ -0,0 +1,20 @@ +package cli + +import ( + "os" + "syscall" +) + +// os.Clearenv() doesn't actually unset variables on Windows +// See: https://github.com/golang/go/issues/17902 +func clearenv() { + for _, s := range os.Environ() { + for j := 1; j < len(s); j++ { + if s[j] == '=' { + keyp, _ := syscall.UTF16PtrFromString(s[0:j]) + syscall.SetEnvironmentVariable(keyp, nil) + break + } + } + } +} From de551c44504d08ca37fb9b1fcfeebcd9799f1656 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 28 Aug 2016 17:43:50 -0700 Subject: [PATCH 34/45] Backport removal of deprecation warnings #508 --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b0d0ee..5cbbe5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ ### Changed - Raise minimum tested/supported Go version to 1.2+ +## [1.18.1] - 2016-08-28 +### Fixed +- Removed deprecation warnings to STDERR to avoid them leaking to the end-user (backported) + ## [1.18.0] - 2016-06-27 ### Added - `./runtests` test runner with coverage tracking by default @@ -29,6 +33,10 @@ - No longer swallows `panic`s that occur within the `Action`s themselves when detecting the signature of the `Action` field +## [1.17.1] - 2016-08-28 +### Fixed +- Removed deprecation warnings to STDERR to avoid them leaking to the end-user + ## [1.17.0] - 2016-05-09 ### Added - Pluggable flag-level help text rendering via `cli.DefaultFlagStringFunc` @@ -50,6 +58,10 @@ - cleanups based on [Go Report Card feedback](https://goreportcard.com/report/github.com/urfave/cli) +## [1.16.1] - 2016-08-28 +### Fixed +- Removed deprecation warnings to STDERR to avoid them leaking to the end-user + ## [1.16.0] - 2016-05-02 ### Added - `Hidden` field on all flag struct types to omit from generated help text From 4c2360f9755dd0d69902a9f7bdc64aef4dd30d24 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 11 Sep 2016 20:17:58 -0700 Subject: [PATCH 35/45] Prepare CHANGELOG for v1.19.0 --- CHANGELOG.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cbbe5b..c5661f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,14 +3,34 @@ **ATTN**: This project uses [semantic versioning](http://semver.org/). ## [Unreleased] + + +## [1.19.0] - 2016-09-11 ### Added - Flag type code generation via `go generate` - Write to stderr and exit 1 if action returns non-nil error - Added support for TOML to the `altsrc` loader +- `SkipArgReorder` was added to allow users to skip the argument reordering. + This is useful if you want to consider all "flags" after an argument as + arguments rather than flags (the default behavior of the stdlib `flag` + library). This is backported functionality from the [removal of the flag + reordering](https://github.com/urfave/cli/pull/398) in the unreleased version + 2 ### Changed - Raise minimum tested/supported Go version to 1.2+ +### Fixed +- `App.Metadata` is initialized automatically now (previously was `nil` unless initialized) +- Correctly show help message if `-h` is provided to a subcommand +- `context.(Global)IsSet` now respects environment variables. Previously it + would return `false` if a flag was specified in the environment rather than + as an argument +- Removed deprecation warnings to STDERR to avoid them leaking to the end-user +- `altsrc`s import paths were updated to use `gopkg.in/urfave/cli.v1`. This + fixes issues that occurred when `gopkg.in/urfave/cli.v1` was imported as well + as `altsrc` where Go would complain that the types didn't match + ## [1.18.1] - 2016-08-28 ### Fixed - Removed deprecation warnings to STDERR to avoid them leaking to the end-user (backported) From fa7ccb1447a78d77447007b830b81cbb0cec7c89 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 12 Nov 2016 13:53:26 -0800 Subject: [PATCH 36/45] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5661f7..b030967 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ - Raise minimum tested/supported Go version to 1.2+ ### Fixed +- If no action is specified on a command or app, the help is now printed instead of `panic`ing - `App.Metadata` is initialized automatically now (previously was `nil` unless initialized) - Correctly show help message if `-h` is provided to a subcommand - `context.(Global)IsSet` now respects environment variables. Previously it From dcdea39be2d678ff387fa40a6cf63f430dc3d805 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 12 Nov 2016 14:01:13 -0800 Subject: [PATCH 37/45] Add additional entries to changelog for v.19.0 release --- CHANGELOG.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b030967..f672d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,11 @@ ## [Unreleased] -## [1.19.0] - 2016-09-11 +## [1.19.0] - 2016-11-12 ### Added +- `FlagsByName` was added to make it easy to sort flags (e.g. `sort.Sort(cli.FlagsByName(app.Flags))`) +- A `Description` field was added to `App` for a more detailed description of + the application (similar to the existing `Description` field on `Command`) - Flag type code generation via `go generate` - Write to stderr and exit 1 if action returns non-nil error - Added support for TOML to the `altsrc` loader @@ -21,6 +24,12 @@ - Raise minimum tested/supported Go version to 1.2+ ### Fixed +- Consider empty environment variables as set (previously environment variables + with the equivalent of `""` would be skipped rather than their value used). +- Return an error if the value in a given environment variable cannot be parsed + as the flag type. Previously these errors were silently swallowed. +- Print full error when an invalid flag is specified (which includes the invalid flag) +- `App.Writer` defaults to `stdout` when `nil` - If no action is specified on a command or app, the help is now printed instead of `panic`ing - `App.Metadata` is initialized automatically now (previously was `nil` unless initialized) - Correctly show help message if `-h` is provided to a subcommand From 8dd1962f7b0654225424d5d1bc0189b0113fcafc Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Mon, 14 Nov 2016 09:35:22 -0700 Subject: [PATCH 38/45] change "complete" to "shellComplete" --- app.go | 4 ++-- context.go | 4 ++-- help.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app.go b/app.go index 50edf9a..9836d31 100644 --- a/app.go +++ b/app.go @@ -175,7 +175,7 @@ func (a *App) Run(arguments []string) (err error) { // flag name as the value of the flag before it which is undesirable // note that we can only do this because the shell autocomplete function // always appends the completion flag at the end of the command - complete, arguments := checkCompleteFlag(a, arguments) + shellComplete, arguments := checkShellCompleteFlag(a, arguments) // parse flags set := flagSet(a.Name, a.Flags) @@ -188,7 +188,7 @@ func (a *App) Run(arguments []string) (err error) { ShowAppHelp(context) return nerr } - context.complete = complete + context.shellComplete = shellComplete if checkCompletions(context) { return nil diff --git a/context.go b/context.go index 81404c4..4f440ec 100644 --- a/context.go +++ b/context.go @@ -15,7 +15,7 @@ import ( type Context struct { App *App Command Command - complete bool + shellComplete bool flagSet *flag.FlagSet setFlags map[string]bool parentContext *Context @@ -26,7 +26,7 @@ func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { c := &Context{App: app, flagSet: set, parentContext: parentCtx} if parentCtx != nil { - c.complete = parentCtx.complete + c.shellComplete = parentCtx.shellComplete } return c diff --git a/help.go b/help.go index e453e72..e261bb6 100644 --- a/help.go +++ b/help.go @@ -252,7 +252,7 @@ func checkSubcommandHelp(c *Context) bool { return false } -func checkCompleteFlag(a *App, arguments []string) (bool, []string) { +func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) { if !a.EnableBashCompletion { return false, arguments } @@ -268,7 +268,7 @@ func checkCompleteFlag(a *App, arguments []string) (bool, []string) { } func checkCompletions(c *Context) bool { - if !c.complete { + if !c.shellComplete { return false } @@ -285,7 +285,7 @@ func checkCompletions(c *Context) bool { } func checkCommandCompletions(c *Context, name string) bool { - if !c.complete { + if !c.shellComplete { return false } From 3272baf434b471d349ddd0f51d07ab11a8cb9228 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Mon, 14 Nov 2016 10:10:51 -0700 Subject: [PATCH 39/45] add a test for shell completion using incomplete flags --- app_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/app_test.go b/app_test.go index 40a598d..ae87237 100644 --- a/app_test.go +++ b/app_test.go @@ -1561,3 +1561,47 @@ func TestHandleAction_WithUnknownPanic(t *testing.T) { } HandleAction(app.Action, NewContext(app, flagSet(app.Name, app.Flags), nil)) } + +func TestShellCompletionForIncompleteFlags(t *testing.T) { + app := NewApp() + app.Flags = []Flag{ + IntFlag{ + Name: "test-completion", + }, + } + app.EnableBashCompletion = true + app.BashComplete = func(ctx *Context) { + for _, command := range ctx.App.Commands { + if command.Hidden { + continue + } + + for _, name := range command.Names() { + fmt.Fprintln(ctx.App.Writer, name) + } + } + + for _, flag := range ctx.App.Flags { + for _, name := range strings.Split(flag.GetName(), ",") { + if name == BashCompletionFlag.Name { + continue + } + + switch name = strings.TrimSpace(name); len(name) { + case 0: + case 1: + fmt.Fprintln(ctx.App.Writer, "-"+name) + default: + fmt.Fprintln(ctx.App.Writer, "--"+name) + } + } + } + } + app.Action = func(ctx *Context) error { + return fmt.Errorf("should not get here") + } + err := app.Run([]string{"", "--test-completion", "--" + BashCompletionFlag.Name}) + if err != nil { + t.Errorf("app should not return an error: %s", err) + } +} From 4661a59b20d724d0476a479a1b2041bf1eccd82b Mon Sep 17 00:00:00 2001 From: drekar Date: Thu, 17 Nov 2016 09:48:03 -1000 Subject: [PATCH 40/45] errorableFlag: scope result of type assertion. --- flag.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flag.go b/flag.go index 3e63cf0..799c6f7 100644 --- a/flag.go +++ b/flag.go @@ -76,8 +76,8 @@ func flagSet(name string, flags []Flag) (*flag.FlagSet, error) { for _, f := range flags { //TODO remove in v2 when errorableFlag is removed - if f, ok := f.(errorableFlag); ok { - if err := f.applyWithError(set); err != nil { + if ef, ok := f.(errorableFlag); ok { + if err := ef.applyWithError(set); err != nil { return nil, err } } else { From d71794de198717467a8f053036b5620ccb0d613c Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 13 Nov 2016 16:15:05 -0800 Subject: [PATCH 41/45] Make ApplyWithError a public method on errorableFlag Add to altsrc flags. Otherwise, flagSet() was bypassing altsrc's attempt at shadowing. --- altsrc/flag_generated.go | 91 ++++++++++++++++++++++++++++++++++++++++ flag.go | 82 ++++++++++++++++++------------------ generate-flag-types | 7 ++++ 3 files changed, 139 insertions(+), 41 deletions(-) diff --git a/altsrc/flag_generated.go b/altsrc/flag_generated.go index b6b96a1..0aeb0b0 100644 --- a/altsrc/flag_generated.go +++ b/altsrc/flag_generated.go @@ -27,6 +27,13 @@ func (f *BoolFlag) Apply(set *flag.FlagSet) { f.BoolFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped BoolFlag.ApplyWithError +func (f *BoolFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.BoolFlag.ApplyWithError(set) +} + // BoolTFlag is the flag type that wraps cli.BoolTFlag to allow // for other values to be specified type BoolTFlag struct { @@ -46,6 +53,13 @@ func (f *BoolTFlag) Apply(set *flag.FlagSet) { f.BoolTFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped BoolTFlag.ApplyWithError +func (f *BoolTFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.BoolTFlag.ApplyWithError(set) +} + // DurationFlag is the flag type that wraps cli.DurationFlag to allow // for other values to be specified type DurationFlag struct { @@ -65,6 +79,13 @@ func (f *DurationFlag) Apply(set *flag.FlagSet) { f.DurationFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped DurationFlag.ApplyWithError +func (f *DurationFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.DurationFlag.ApplyWithError(set) +} + // Float64Flag is the flag type that wraps cli.Float64Flag to allow // for other values to be specified type Float64Flag struct { @@ -84,6 +105,13 @@ func (f *Float64Flag) Apply(set *flag.FlagSet) { f.Float64Flag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped Float64Flag.ApplyWithError +func (f *Float64Flag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.Float64Flag.ApplyWithError(set) +} + // GenericFlag is the flag type that wraps cli.GenericFlag to allow // for other values to be specified type GenericFlag struct { @@ -103,6 +131,13 @@ func (f *GenericFlag) Apply(set *flag.FlagSet) { f.GenericFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped GenericFlag.ApplyWithError +func (f *GenericFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.GenericFlag.ApplyWithError(set) +} + // Int64Flag is the flag type that wraps cli.Int64Flag to allow // for other values to be specified type Int64Flag struct { @@ -122,6 +157,13 @@ func (f *Int64Flag) Apply(set *flag.FlagSet) { f.Int64Flag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped Int64Flag.ApplyWithError +func (f *Int64Flag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.Int64Flag.ApplyWithError(set) +} + // IntFlag is the flag type that wraps cli.IntFlag to allow // for other values to be specified type IntFlag struct { @@ -141,6 +183,13 @@ func (f *IntFlag) Apply(set *flag.FlagSet) { f.IntFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped IntFlag.ApplyWithError +func (f *IntFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.IntFlag.ApplyWithError(set) +} + // IntSliceFlag is the flag type that wraps cli.IntSliceFlag to allow // for other values to be specified type IntSliceFlag struct { @@ -160,6 +209,13 @@ func (f *IntSliceFlag) Apply(set *flag.FlagSet) { f.IntSliceFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped IntSliceFlag.ApplyWithError +func (f *IntSliceFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.IntSliceFlag.ApplyWithError(set) +} + // Int64SliceFlag is the flag type that wraps cli.Int64SliceFlag to allow // for other values to be specified type Int64SliceFlag struct { @@ -179,6 +235,13 @@ func (f *Int64SliceFlag) Apply(set *flag.FlagSet) { f.Int64SliceFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped Int64SliceFlag.ApplyWithError +func (f *Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.Int64SliceFlag.ApplyWithError(set) +} + // StringFlag is the flag type that wraps cli.StringFlag to allow // for other values to be specified type StringFlag struct { @@ -198,6 +261,13 @@ func (f *StringFlag) Apply(set *flag.FlagSet) { f.StringFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped StringFlag.ApplyWithError +func (f *StringFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.StringFlag.ApplyWithError(set) +} + // StringSliceFlag is the flag type that wraps cli.StringSliceFlag to allow // for other values to be specified type StringSliceFlag struct { @@ -217,6 +287,13 @@ func (f *StringSliceFlag) Apply(set *flag.FlagSet) { f.StringSliceFlag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped StringSliceFlag.ApplyWithError +func (f *StringSliceFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.StringSliceFlag.ApplyWithError(set) +} + // Uint64Flag is the flag type that wraps cli.Uint64Flag to allow // for other values to be specified type Uint64Flag struct { @@ -236,6 +313,13 @@ func (f *Uint64Flag) Apply(set *flag.FlagSet) { f.Uint64Flag.Apply(set) } +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped Uint64Flag.ApplyWithError +func (f *Uint64Flag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.Uint64Flag.ApplyWithError(set) +} + // UintFlag is the flag type that wraps cli.UintFlag to allow // for other values to be specified type UintFlag struct { @@ -254,3 +338,10 @@ func (f *UintFlag) Apply(set *flag.FlagSet) { f.set = set f.UintFlag.Apply(set) } + +// ApplyWithError saves the flagSet for later usage calls, then calls the +// wrapped UintFlag.ApplyWithError +func (f *UintFlag) ApplyWithError(set *flag.FlagSet) error { + f.set = set + return f.UintFlag.ApplyWithError(set) +} diff --git a/flag.go b/flag.go index 799c6f7..7dd8a2c 100644 --- a/flag.go +++ b/flag.go @@ -68,7 +68,7 @@ type Flag interface { type errorableFlag interface { Flag - applyWithError(*flag.FlagSet) error + ApplyWithError(*flag.FlagSet) error } func flagSet(name string, flags []Flag) (*flag.FlagSet, error) { @@ -77,7 +77,7 @@ func flagSet(name string, flags []Flag) (*flag.FlagSet, error) { for _, f := range flags { //TODO remove in v2 when errorableFlag is removed if ef, ok := f.(errorableFlag); ok { - if err := ef.applyWithError(set); err != nil { + if err := ef.ApplyWithError(set); err != nil { return nil, err } } else { @@ -105,12 +105,12 @@ type Generic interface { // provided by the user for parsing by the flag // Ignores parsing errors func (f GenericFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError takes the flagset and calls Set on the generic flag with the value +// ApplyWithError takes the flagset and calls Set on the generic flag with the value // provided by the user for parsing by the flag -func (f GenericFlag) applyWithError(set *flag.FlagSet) error { +func (f GenericFlag) ApplyWithError(set *flag.FlagSet) error { val := f.Value if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { @@ -158,11 +158,11 @@ func (f *StringSlice) Get() interface{} { // Apply populates the flag given the flag set and environment // Ignores errors func (f StringSliceFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f StringSliceFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f StringSliceFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -221,11 +221,11 @@ func (f *IntSlice) Get() interface{} { // Apply populates the flag given the flag set and environment // Ignores errors func (f IntSliceFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f IntSliceFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f IntSliceFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -284,11 +284,11 @@ func (f *Int64Slice) Get() interface{} { // Apply populates the flag given the flag set and environment // Ignores errors func (f Int64SliceFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f Int64SliceFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -318,11 +318,11 @@ func (f Int64SliceFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f BoolFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f BoolFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f BoolFlag) ApplyWithError(set *flag.FlagSet) error { val := false if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { @@ -358,11 +358,11 @@ func (f BoolFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f BoolTFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f BoolTFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f BoolTFlag) ApplyWithError(set *flag.FlagSet) error { val := true if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { @@ -398,11 +398,11 @@ func (f BoolTFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f StringFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f StringFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f StringFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -427,11 +427,11 @@ func (f StringFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f IntFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f IntFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f IntFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -460,11 +460,11 @@ func (f IntFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f Int64Flag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f Int64Flag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f Int64Flag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -494,11 +494,11 @@ func (f Int64Flag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f UintFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f UintFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f UintFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -528,11 +528,11 @@ func (f UintFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f Uint64Flag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f Uint64Flag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f Uint64Flag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -562,11 +562,11 @@ func (f Uint64Flag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f DurationFlag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f DurationFlag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f DurationFlag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) @@ -596,11 +596,11 @@ func (f DurationFlag) applyWithError(set *flag.FlagSet) error { // Apply populates the flag given the flag set and environment // Ignores errors func (f Float64Flag) Apply(set *flag.FlagSet) { - f.applyWithError(set) + f.ApplyWithError(set) } -// applyWithError populates the flag given the flag set and environment -func (f Float64Flag) applyWithError(set *flag.FlagSet) error { +// ApplyWithError populates the flag given the flag set and environment +func (f Float64Flag) ApplyWithError(set *flag.FlagSet) error { if f.EnvVar != "" { for _, envVar := range strings.Split(f.EnvVar, ",") { envVar = strings.TrimSpace(envVar) diff --git a/generate-flag-types b/generate-flag-types index 47a168b..7147381 100755 --- a/generate-flag-types +++ b/generate-flag-types @@ -232,6 +232,13 @@ def _write_altsrc_flag_types(outfile, types): f.set = set f.{name}Flag.Apply(set) }} + + // ApplyWithError saves the flagSet for later usage calls, then calls the + // wrapped {name}Flag.ApplyWithError + func (f *{name}Flag) ApplyWithError(set *flag.FlagSet) error {{ + f.set = set + return f.{name}Flag.ApplyWithError(set) + }} """.format(**typedef)) From a8fc36b690712e61212d8cb9450eabf2be359fca Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 19 Nov 2016 10:54:17 -0800 Subject: [PATCH 42/45] Update CHANGELOG --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f672d59..9a6b36e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ library). This is backported functionality from the [removal of the flag reordering](https://github.com/urfave/cli/pull/398) in the unreleased version 2 +- For formatted errors (those implementing `ErrorFormatter`), the errors will + be formatted during output. Compatible with `pkg/errors`. ### Changed - Raise minimum tested/supported Go version to 1.2+ From 7fbcf2396a29f95b33946a716a2c4e959c92e154 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 19 Nov 2016 11:04:57 -0800 Subject: [PATCH 43/45] Update date of 1.19.0 release in CHANGELOG [ci skip] --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a6b36e..edd24a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ## [Unreleased] -## [1.19.0] - 2016-11-12 +## [1.19.0] - 2016-11-19 ### Added - `FlagsByName` was added to make it easy to sort flags (e.g. `sort.Sort(cli.FlagsByName(app.Flags))`) - A `Description` field was added to `App` for a more detailed description of From 8fa549846ee27c7f7542c70de604c6f8807fdf07 Mon Sep 17 00:00:00 2001 From: Kasey Klipsch Date: Mon, 21 Nov 2016 09:47:23 -0600 Subject: [PATCH 44/45] #556 broke the api for users who were using the ActionFunc --- app.go | 4 +++- app_test.go | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index 38d89ae..95ffc0b 100644 --- a/app.go +++ b/app.go @@ -479,7 +479,9 @@ func (a Author) String() string { // it's an ActionFunc or a func with the legacy signature for Action, the func // is run! func HandleAction(action interface{}, context *Context) (err error) { - if a, ok := action.(func(*Context) error); ok { + if a, ok := action.(ActionFunc); ok { + return a(context) + } else if a, ok := action.(func(*Context) error); ok { return a(context) } else if a, ok := action.(func(*Context)); ok { // deprecated function signature a(context) diff --git a/app_test.go b/app_test.go index fadeb20..10f1562 100644 --- a/app_test.go +++ b/app_test.go @@ -1664,3 +1664,22 @@ func TestShellCompletionForIncompleteFlags(t *testing.T) { t.Errorf("app should not return an error: %s", err) } } + +func TestHandleActionActuallyWorksWithActions(t *testing.T) { + var f ActionFunc + called := false + f = func(c *Context) error { + called = true + return nil + } + + err := HandleAction(f, nil) + + if err != nil { + t.Errorf("Should not have errored: %v", err) + } + + if !called { + t.Errorf("Function was not called") + } +} From 0ef658215409bbac685b2219743292620e5687fa Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 21 Nov 2016 20:26:35 -0800 Subject: [PATCH 45/45] Prep 1.19.1 --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index edd24a0..07f7546 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,18 @@ ## [Unreleased] +## [1.19.1] - 2016-11-21 + +### Fixed + +- Fixes regression introduced in 1.19.0 where using an `ActionFunc` as + the `Action` for a command would cause it to error rather than calling the + function. Should not have a affected declarative cases using `func(c + *cli.Context) err)`. +- Shell completion now handles the case where the user specifies + `--generate-bash-completion` immediately after a flag that takes an argument. + Previously it call the application with `--generate-bash-completion` as the + flag value. ## [1.19.0] - 2016-11-19 ### Added