diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b0d0ee..f672d59 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,14 +3,48 @@ **ATTN**: This project uses [semantic versioning](http://semver.org/). ## [Unreleased] + + +## [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 +- `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 +- 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 +- `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) + ## [1.18.0] - 2016-06-27 ### Added - `./runtests` test runner with coverage tracking by default @@ -29,6 +63,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 +88,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 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/app.go b/app.go index 9836d31..38d89ae 100644 --- a/app.go +++ b/app.go @@ -178,7 +178,11 @@ func (a *App) Run(arguments []string) (err error) { shellComplete, arguments := checkShellCompleteFlag(a, arguments) // 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) @@ -247,6 +251,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) @@ -289,7 +297,11 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { a.Commands = newCmds // 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 ae87237..fadeb20 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"} @@ -1480,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") @@ -1504,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") @@ -1528,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") @@ -1559,7 +1614,11 @@ 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)) } func TestShellCompletionForIncompleteFlags(t *testing.T) { diff --git a/command.go b/command.go index 04eb0b8..2628fbf 100644 --- a/command.go +++ b/command.go @@ -87,7 +87,10 @@ func (c Command) Run(ctx *Context) (err error) { ) } - 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 { @@ -182,6 +185,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/context.go b/context.go index 4f440ec..cb89e92 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 @@ -98,7 +98,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 } @@ -154,6 +154,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 0cf84d1..a1ab05b 100644 --- a/context_test.go +++ b/context_test.go @@ -184,18 +184,30 @@ 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 - - os.Clearenv() + var ( + timeoutIsSet, tIsSet bool + noEnvVarIsSet, nIsSet bool + passwordIsSet, pIsSet bool + unparsableIsSet, uIsSet bool + ) + + 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: "unparsable, u", EnvVar: "APP_UNPARSABLE"}, 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") + unparsableIsSet = ctx.IsSet("unparsable") + uIsSet = ctx.IsSet("u") noEnvVarIsSet = ctx.IsSet("no-env-var") nIsSet = ctx.IsSet("n") return nil @@ -204,8 +216,15 @@ 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) + + os.Setenv("APP_UNPARSABLE", "foobar") + a.Run([]string{"run"}) + expect(t, unparsableIsSet, false) + expect(t, uIsSet, false) } func TestContext_GlobalIsSet(t *testing.T) { @@ -230,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 - - os.Clearenv() + var ( + timeoutIsSet, tIsSet bool + noEnvVarIsSet, nIsSet bool + passwordIsSet, pIsSet bool + unparsableIsSet, uIsSet bool + ) + + 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{ { @@ -245,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 @@ -252,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/errors.go b/errors.go index ddef369..0206ff4 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 @@ -78,7 +82,11 @@ func HandleExitCoder(err error) { if exitErr, ok := err.(ExitCoder); ok { if err.Error() != "" { - fmt.Fprintln(ErrWriter, err) + if _, ok := exitErr.(ErrorFormatter); ok { + fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { + fmt.Fprintln(ErrWriter, err) + } } OsExiter(exitErr.ExitCode()) return @@ -92,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 04df031..131bd38 100644 --- a/errors_test.go +++ b/errors_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "errors" + "fmt" "testing" ) @@ -104,3 +105,53 @@ 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 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) +} + +func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { + called := false + + OsExiter = func(rc int) { + called = true + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + 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") +} diff --git a/flag.go b/flag.go index 1ff28d3..3e63cf0 100644 --- a/flag.go +++ b/flag.go @@ -3,11 +3,11 @@ package cli import ( "flag" "fmt" - "os" "reflect" "runtime" "strconv" "strings" + "syscall" "time" ) @@ -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 := os.Getenv(envVar); envVal != "" { - val.Set(envVal) + if envVal, ok := syscall.Getenv(envVar); ok { + 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,16 +150,29 @@ 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) - 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) - 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,18 +213,28 @@ 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) - 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) - 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,18 +276,28 @@ 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) - 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) - 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,19 +312,33 @@ 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, ",") { 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 + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) } + + val = envValBool break } } @@ -273,20 +351,35 @@ 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, ",") { envVar = strings.TrimSpace(envVar) - if envVal := os.Getenv(envVar); envVal != "" { - envValBool, err := strconv.ParseBool(envVal) - if err == nil { - val = envValBool + if envVal, ok := syscall.Getenv(envVar); ok { + if envVal == "" { + val = false break } + + envValBool, err := strconv.ParseBool(envVal) + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) + } + + val = envValBool + break } } } @@ -298,14 +391,22 @@ 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) - if envVal := os.Getenv(envVar); envVal != "" { + if envVal, ok := syscall.Getenv(envVar); ok { f.Value = envVal break } @@ -319,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 := 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) - 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 } } } @@ -343,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 := os.Getenv(envVar); envVal != "" { + 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 } } } @@ -367,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 := 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) - 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 } } } @@ -391,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 := 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) - 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 } } } @@ -415,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 := os.Getenv(envVar); envVal != "" { + 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 } } } @@ -439,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 := os.Getenv(envVar); envVal != "" { + 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 } } } @@ -462,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 a7afcc4..0dd8654 100644 --- a/flag_test.go +++ b/flag_test.go @@ -29,6 +29,81 @@ func TestBoolFlagHelpOutput(t *testing.T) { } } +func TestFlagsFromEnv(t *testing.T) { + var flagTests = []struct { + input string + output interface{} + flag Flag + err error + }{ + {"", 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 flagTests { + os.Clearenv() + os.Setenv(reflect.ValueOf(test.flag).FieldByName("EnvVar").String(), test.input) + a := App{ + Flags: []Flag{test.flag}, + Action: func(ctx *Context) error { + 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 + }, + } + + err := a.Run([]string{"run"}) + if !reflect.DeepEqual(test.err, err) { + t.Errorf("expected error %s, got error %s", test.err, err) + } + } +} + var stringFlagTests = []struct { name string usage string @@ -941,6 +1016,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{ @@ -1036,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{ diff --git a/help.go b/help.go index e261bb6..c8c1aee 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}} 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 + } + } + } +}