From ce428377024188ae8ce8aae411881b7e993f9553 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 30 Sep 2016 19:23:44 +0900 Subject: [PATCH 01/57] Call HandleExitCoder for all members of MultiError.Errors --- errors.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/errors.go b/errors.go index c7d8c2f..fd67b96 100644 --- a/errors.go +++ b/errors.go @@ -86,8 +86,9 @@ func HandleExitCoder(err error) { if multiErr, ok := err.(MultiError); ok { for _, merr := range multiErr.Errors { - HandleExitCoder(merr) + fmt.Fprintln(ErrWriter, merr) } + OsExiter(1) return } From 508a23430b94565dc28414c9587d31626b3ce596 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 18 Oct 2016 20:56:31 -0700 Subject: [PATCH 02/57] Default app.Writer to os.Stdout As NewApp() does. Fixes #545 --- app.go | 4 ++++ app_test.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/app.go b/app.go index b9adf46..c7e2594 100644 --- a/app.go +++ b/app.go @@ -165,6 +165,10 @@ func (a *App) Setup() { if a.Metadata == nil { a.Metadata = make(map[string]interface{}) } + + if a.Writer == nil { + a.Writer = os.Stdout + } } // Run is the entry point to the cli app. Parses the arguments slice and routes diff --git a/app_test.go b/app_test.go index 23c8aa6..59fea69 100644 --- a/app_test.go +++ b/app_test.go @@ -202,6 +202,12 @@ func TestApp_Command(t *testing.T) { } } +func TestApp_Setup_defaultsWriter(t *testing.T) { + app := &App{} + app.Setup() + expect(t, app.Writer, os.Stdout) +} + func TestApp_CommandWithArgBeforeFlags(t *testing.T) { var parsedOption, firstArg string From b377b5d9e93d2359a7ffe16fc1bb902b3df291b6 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 1 Nov 2016 20:33:12 -0700 Subject: [PATCH 03/57] 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 04/57] make shell autocomplete more robust --- app.go | 18 +++++++++--------- command.go | 31 +++++++++++++------------------ context.go | 9 ++++++++- help.go | 41 ++++++++++++++++++++++++++++++++--------- 4 files changed, 62 insertions(+), 37 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,6 +128,19 @@ func (c Command) Run(ctx *Context) (err error) { err = set.Parse(ctx.Args().Tail()) } + nerr := normalizeFlags(c.Flags, set) + if nerr != nil { + fmt.Fprintln(ctx.App.Writer, nerr) + fmt.Fprintln(ctx.App.Writer) + ShowCommandHelp(ctx, c.Name) + return nerr + } + + 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) @@ -144,20 +153,6 @@ func (c Command) Run(ctx *Context) (err error) { return err } - nerr := normalizeFlags(c.Flags, set) - if nerr != nil { - fmt.Fprintln(ctx.App.Writer, nerr) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) - return nerr - } - - context := NewContext(ctx.App, set, ctx) - - if checkCommandCompletions(context, c.Name) { - return nil - } - 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 checkCompletions(c *Context) bool { - if (c.GlobalBool(BashCompletionFlag.Name) || c.Bool(BashCompletionFlag.Name)) && c.App.EnableBashCompletion { - ShowCompletions(c) - return true +func checkCompleteFlag(a *App, arguments []string) (bool, []string) { + if !a.EnableBashCompletion { + return false, arguments } - return false + 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.complete { + 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 05/57] 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 6c50b15a273d29dc3820b5e4d50d78eeb113d335 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 11 Nov 2016 13:11:50 +0900 Subject: [PATCH 06/57] Exit with the code of ExitCoder if exists --- errors.go | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/errors.go b/errors.go index fd67b96..583f89f 100644 --- a/errors.go +++ b/errors.go @@ -85,10 +85,8 @@ func HandleExitCoder(err error) { } if multiErr, ok := err.(MultiError); ok { - for _, merr := range multiErr.Errors { - fmt.Fprintln(ErrWriter, merr) - } - OsExiter(1) + code := handleMultiError(multiErr) + OsExiter(code) return } @@ -97,3 +95,18 @@ func HandleExitCoder(err error) { } OsExiter(1) } + +func handleMultiError(multiErr MultiError) int { + code := 1 + for _, merr := range multiErr.Errors { + if multiErr2, ok := merr.(MultiError); ok { + code = handleMultiError(multiErr2) + } else { + fmt.Fprintln(ErrWriter, merr) + if exitErr, ok := merr.(ExitCoder); ok { + code = exitErr.ExitCode() + } + } + } + return code +} From 0113f56d1094e8565f6d1ad10526865fd61d57aa Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 12 Nov 2016 13:37:07 -0800 Subject: [PATCH 07/57] 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 08/57] 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 09/57] 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 10/57] 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 11/57] 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 12/57] 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 13/57] 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 14/57] 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 15/57] 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 16/57] 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 17/57] 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 b5d06bd2a9cddb942cf282cad11c97c251b6f6c7 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 16:58:46 +0100 Subject: [PATCH 18/57] allow to load YAML configuration files on Windows The funtion `loadDataFrom` does not take care of Windows users since any of the conditions met and it returns an error. The change looks for the runtime where it's running and then if the filePath contains a `\` --- altsrc/yaml_file_loader.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 335356f..433023d 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -78,6 +78,11 @@ func loadDataFrom(filePath string) ([]byte, error) { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } return ioutil.ReadFile(filePath) + } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { + return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) + } + return ioutil.ReadFile(filePath) } else { return nil, fmt.Errorf("unable to determine how to load from path %s", filePath) } From 9f357f76252fcd4446dbdd981dabd52b40c87481 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 17:08:01 +0100 Subject: [PATCH 19/57] fix imports Sorry, forgot to add imports correctly, needed to edit the file and make the commit using the github online editor, since I can't access from my current location from git. --- altsrc/yaml_file_loader.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 433023d..3965fe4 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -11,6 +11,8 @@ import ( "net/http" "net/url" "os" + "runtime" + "strings" "gopkg.in/urfave/cli.v1" From 4661a59b20d724d0476a479a1b2041bf1eccd82b Mon Sep 17 00:00:00 2001 From: drekar Date: Thu, 17 Nov 2016 09:48:03 -1000 Subject: [PATCH 20/57] 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 21/57] 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 22/57] 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 23/57] 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 6503447ae720ddabdb8902f7aebef525191da02f Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Sat, 19 Nov 2016 22:37:11 +0100 Subject: [PATCH 24/57] added comment to windows filePath check --- altsrc/yaml_file_loader.go | 1 + 1 file changed, 1 insertion(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 3965fe4..dd808d5 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -81,6 +81,7 @@ func loadDataFrom(filePath string) ([]byte, error) { } return ioutil.ReadFile(filePath) } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + // on Windows systems u.Path is always empty, so we need to check the string directly. if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } From 8fa549846ee27c7f7542c70de604c6f8807fdf07 Mon Sep 17 00:00:00 2001 From: Kasey Klipsch Date: Mon, 21 Nov 2016 09:47:23 -0600 Subject: [PATCH 25/57] #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 26/57] 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 From 4267cd827cbfb066cf031bcb01ddbecdf7d0c49e Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 4 Jan 2017 10:28:58 +0800 Subject: [PATCH 27/57] [ci skip] Fix template syntax error --- README.md | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index bb5f61e..6ba60c4 100644 --- a/README.md +++ b/README.md @@ -940,16 +940,13 @@ SUPPORT: support@awesometown.example.com cli.AppHelpTemplate = `NAME: {{.Name}} - {{.Usage}} USAGE: - {{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command -[command options]{{end}} {{if -.ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} + {{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} {{if len .Authors}} -AUTHOR(S): +AUTHOR: {{range .Authors}}{{ . }}{{end}} {{end}}{{if .Commands}} COMMANDS: -{{range .Commands}}{{if not .HideHelp}} {{join .Names ", "}}{{ "\t" -}}{{.Usage}}{{ "\n" }}{{end}}{{end}}{{end}}{{if .VisibleFlags}} +{{range .Commands}}{{if not .HideHelp}} {{join .Names ", "}}{{ "\t"}}{{.Usage}}{{ "\n" }}{{end}}{{end}}{{end}}{{if .VisibleFlags}} GLOBAL OPTIONS: {{range .VisibleFlags}}{{.}} {{end}}{{end}}{{if .Copyright }} From ac772237b96509d5150567da31ba6c6c3897d452 Mon Sep 17 00:00:00 2001 From: Maximilian Meister Date: Sun, 18 Dec 2016 17:43:48 +0100 Subject: [PATCH 28/57] command: enable ordering commands by name --- README.md | 24 ++++++++++++++++++++++-- command.go | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 6ba60c4..2bbbd8e 100644 --- a/README.md +++ b/README.md @@ -455,13 +455,13 @@ error. Flags for the application and commands are shown in the order they are defined. However, it's possible to sort them from outside this library by using `FlagsByName` -with `sort`. +or `CommandsByName` with `sort`. For example this: ``` go package main @@ -488,7 +488,27 @@ func main() { }, } + app.Commands = []cli.Command{ + { + Name: "complete", + Aliases: []string{"c"}, + Usage: "complete a task on the list", + Action: func(c *cli.Context) error { + return nil + }, + }, + { + Name: "add", + Aliases: []string{"a"}, + Usage: "add a task to the list", + Action: func(c *cli.Context) error { + return nil + }, + }, + } + sort.Sort(cli.FlagsByName(app.Flags)) + sort.Sort(cli.CommandsByName(app.Commands)) app.Run(os.Args) } diff --git a/command.go b/command.go index 2628fbf..d297eb9 100644 --- a/command.go +++ b/command.go @@ -61,6 +61,20 @@ type Command struct { commandNamePath []string } +type CommandsByName []Command + +func (c CommandsByName) Len() int { + return len(c) +} + +func (c CommandsByName) Less(i, j int) bool { + return c[i].Name < c[j].Name +} + +func (c CommandsByName) Swap(i, j int) { + c[i], c[j] = c[j], c[i] +} + // FullName returns the full name of the command. // For subcommands this ensures that parent commands are part of the command path func (c Command) FullName() string { From 0083ae8732e4b5d6911729b5e60c99a19d6179ac Mon Sep 17 00:00:00 2001 From: Joe Richey Date: Mon, 9 Jan 2017 15:57:49 -0800 Subject: [PATCH 29/57] Usage/Description/ArgsUsage correctly copied when using subcommand --- command.go | 8 +++----- help.go | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/command.go b/command.go index 2628fbf..68c760e 100644 --- a/command.go +++ b/command.go @@ -230,11 +230,9 @@ func (c Command) startApp(ctx *Context) error { app.HelpName = app.Name } - if c.Description != "" { - app.Usage = c.Description - } else { - app.Usage = c.Usage - } + app.Usage = c.Usage + app.Description = c.Description + app.ArgsUsage = c.ArgsUsage // set CommandNotFound app.CommandNotFound = ctx.App.CommandNotFound diff --git a/help.go b/help.go index c8c1aee..d00e4da 100644 --- a/help.go +++ b/help.go @@ -64,7 +64,7 @@ OPTIONS: // cli.go uses text/template to render templates. You can // render custom help text by setting this variable. var SubcommandHelpTemplate = `NAME: - {{.HelpName}} - {{.Usage}} + {{.HelpName}} - {{if .Description}}{{.Description}}{{else}}{{.Usage}}{{end}} USAGE: {{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} From acc622e5fb6f273c26946ded483aa813bebdcee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Wed, 11 Jan 2017 14:38:54 +0200 Subject: [PATCH 30/57] Invalidate context.setFlags cache on modification. --- context.go | 2 ++ context_test.go | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/context.go b/context.go index cb89e92..db94191 100644 --- a/context.go +++ b/context.go @@ -39,11 +39,13 @@ func (c *Context) NumFlags() int { // Set sets a context flag to a value. func (c *Context) Set(name, value string) error { + c.setFlags = nil return c.flagSet.Set(name, value) } // GlobalSet sets a context flag to a value on the global flagset func (c *Context) GlobalSet(name, value string) error { + globalContext(c).setFlags = nil return globalContext(c).flagSet.Set(name, value) } diff --git a/context_test.go b/context_test.go index a1ab05b..7acca10 100644 --- a/context_test.go +++ b/context_test.go @@ -375,8 +375,10 @@ func TestContext_Set(t *testing.T) { set.Int("int", 5, "an int") c := NewContext(nil, set, nil) + expect(t, c.IsSet("int"), false) c.Set("int", "1") expect(t, c.Int("int"), 1) + expect(t, c.IsSet("int"), true) } func TestContext_GlobalSet(t *testing.T) { @@ -393,7 +395,9 @@ func TestContext_GlobalSet(t *testing.T) { expect(t, c.Int("int"), 1) expect(t, c.GlobalInt("int"), 5) + expect(t, c.GlobalIsSet("int"), false) c.GlobalSet("int", "1") expect(t, c.Int("int"), 1) expect(t, c.GlobalInt("int"), 1) + expect(t, c.GlobalIsSet("int"), true) } From 71ced406af64ee9961533d518dab28d455a66666 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Thu, 12 Jan 2017 18:59:38 +0900 Subject: [PATCH 31/57] Treat `rc` first called as exit code Because default OsExiter is os.Exit. --- errors_test.go | 38 ++++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-) diff --git a/errors_test.go b/errors_test.go index 131bd38..d3764b7 100644 --- a/errors_test.go +++ b/errors_test.go @@ -12,8 +12,10 @@ func TestHandleExitCoder_nil(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -29,8 +31,10 @@ func TestHandleExitCoder_ExitCoder(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -46,8 +50,10 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } defer func() { OsExiter = fakeOsExiter }() @@ -65,8 +71,10 @@ func TestHandleExitCoder_ErrorWithMessage(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } ErrWriter = &bytes.Buffer{} @@ -88,8 +96,10 @@ func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { called := false OsExiter = func(rc int) { - exitCode = rc - called = true + if !called { + exitCode = rc + called = true + } } ErrWriter = &bytes.Buffer{} @@ -123,7 +133,9 @@ func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { called := false OsExiter = func(rc int) { - called = true + if !called { + called = true + } } ErrWriter = &bytes.Buffer{} @@ -143,7 +155,9 @@ func TestHandleExitCoder_MultiErrorWithFormat(t *testing.T) { called := false OsExiter = func(rc int) { - called = true + if !called { + called = true + } } ErrWriter = &bytes.Buffer{} From 286b4b56d988e0ac6da075615f1da496db94da87 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Thu, 12 Jan 2017 19:12:17 +0900 Subject: [PATCH 32/57] Document on exit code in case of MultiError is given --- errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors.go b/errors.go index 8aa8d9e..f9d648e 100644 --- a/errors.go +++ b/errors.go @@ -74,7 +74,7 @@ func (ee *ExitError) ExitCode() int { // HandleExitCoder checks if the error fulfills the ExitCoder interface, and if // so prints the error to stderr (if it is non-empty) and calls OsExiter with the // given exit code. If the given error is a MultiError, then this func is -// called on all members of the Errors slice. +// called on all members of the Errors slice and calls OsExiter with the last exit code. func HandleExitCoder(err error) { if err == nil { return From adec15acf537c7b696c299223a16c60cb3eeeed8 Mon Sep 17 00:00:00 2001 From: HIROSE Masaaki Date: Fri, 13 Jan 2017 15:37:09 +0900 Subject: [PATCH 33/57] Add another ExitCoder to assert that it uses last one --- errors_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/errors_test.go b/errors_test.go index d3764b7..d9e0da6 100644 --- a/errors_test.go +++ b/errors_test.go @@ -59,10 +59,11 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { defer func() { OsExiter = fakeOsExiter }() exitErr := NewExitError("galactic perimeter breach", 9) - err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr) + exitErr2 := NewExitError("last ExitCoder", 11) + err := NewMultiError(errors.New("wowsa"), errors.New("egad"), exitErr, exitErr2) HandleExitCoder(err) - expect(t, exitCode, 9) + expect(t, exitCode, 11) expect(t, called, true) } From 7b912d9f8f8a78f192061ed5b0f40918dfda5a07 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 16:58:46 +0100 Subject: [PATCH 34/57] allow to load YAML configuration files on Windows The funtion `loadDataFrom` does not take care of Windows users since any of the conditions met and it returns an error. The change looks for the runtime where it's running and then if the filePath contains a `\` --- altsrc/yaml_file_loader.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 335356f..433023d 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -78,6 +78,11 @@ func loadDataFrom(filePath string) ([]byte, error) { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } return ioutil.ReadFile(filePath) + } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { + return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) + } + return ioutil.ReadFile(filePath) } else { return nil, fmt.Errorf("unable to determine how to load from path %s", filePath) } From 341ca93b01a20fe6fc361e005b45d968fd559aa5 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 17:08:01 +0100 Subject: [PATCH 35/57] fix imports Sorry, forgot to add imports correctly, needed to edit the file and make the commit using the github online editor, since I can't access from my current location from git. --- altsrc/yaml_file_loader.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 433023d..3965fe4 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -11,6 +11,8 @@ import ( "net/http" "net/url" "os" + "runtime" + "strings" "gopkg.in/urfave/cli.v1" From f1be59ff3d239f0942b201619030d302bce912cc Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Sat, 19 Nov 2016 22:37:11 +0100 Subject: [PATCH 36/57] added comment to windows filePath check --- altsrc/yaml_file_loader.go | 1 + 1 file changed, 1 insertion(+) diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index 3965fe4..dd808d5 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -81,6 +81,7 @@ func loadDataFrom(filePath string) ([]byte, error) { } return ioutil.ReadFile(filePath) } else if runtime.GOOS == "windows" && strings.Contains(u.String(), "\\") { + // on Windows systems u.Path is always empty, so we need to check the string directly. if _, notFoundFileErr := os.Stat(filePath); notFoundFileErr != nil { return nil, fmt.Errorf("Cannot read from file: '%s' because it does not exist.", filePath) } From f8347a97c68c1b2a11f418da0225953c9b007708 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 8 Jan 2017 18:11:26 -0500 Subject: [PATCH 37/57] Fix altsrc slice inputs so that they correctly parse Was previously attempting to cast directly from []interface{} to []string or []int which Go doesn't support. Instead, we iterate over and cast each value (error'ing if the value is not the correct format). Fixes #580 --- altsrc/flag_test.go | 12 +++---- altsrc/map_input_source.go | 70 +++++++++++++++++++++++--------------- 2 files changed, 48 insertions(+), 34 deletions(-) diff --git a/altsrc/flag_test.go b/altsrc/flag_test.go index 9e9c96d..cd18294 100644 --- a/altsrc/flag_test.go +++ b/altsrc/flag_test.go @@ -59,7 +59,7 @@ func TestStringSliceApplyInputSourceValue(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, }) expect(t, c.StringSlice("test"), []string{"hello", "world"}) } @@ -68,7 +68,7 @@ func TestStringSliceApplyInputSourceMethodContextSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, ContextValueString: "ohno", }) expect(t, c.StringSlice("test"), []string{"ohno"}) @@ -78,7 +78,7 @@ func TestStringSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewStringSliceFlag(cli.StringSliceFlag{Name: "test", EnvVar: "TEST"}), FlagName: "test", - MapValue: []string{"hello", "world"}, + MapValue: []interface{}{"hello", "world"}, EnvVarName: "TEST", EnvVarValue: "oh,no", }) @@ -89,7 +89,7 @@ func TestIntSliceApplyInputSourceValue(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, }) expect(t, c.IntSlice("test"), []int{1, 2}) } @@ -98,7 +98,7 @@ func TestIntSliceApplyInputSourceMethodContextSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, ContextValueString: "3", }) expect(t, c.IntSlice("test"), []int{3}) @@ -108,7 +108,7 @@ func TestIntSliceApplyInputSourceMethodEnvVarSet(t *testing.T) { c := runTest(t, testApplyInputSource{ Flag: NewIntSliceFlag(cli.IntSliceFlag{Name: "test", EnvVar: "TEST"}), FlagName: "test", - MapValue: []int{1, 2}, + MapValue: []interface{}{1, 2}, EnvVarName: "TEST", EnvVarValue: "3,4", }) diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go index b720995..b3169e0 100644 --- a/altsrc/map_input_source.go +++ b/altsrc/map_input_source.go @@ -130,45 +130,59 @@ func (fsm *MapInputSource) String(name string) (string, error) { // StringSlice returns an []string from the map if it exists otherwise returns nil func (fsm *MapInputSource) StringSlice(name string) ([]string, error) { otherGenericValue, exists := fsm.valueMap[name] - if exists { - otherValue, isType := otherGenericValue.([]string) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]string", otherGenericValue) + if !exists { + otherGenericValue, exists = nestedVal(name, fsm.valueMap) + if !exists { + return nil, nil } - return otherValue, nil - } - nestedGenericValue, exists := nestedVal(name, fsm.valueMap) - if exists { - otherValue, isType := nestedGenericValue.([]string) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]string", nestedGenericValue) - } - return otherValue, nil } - return nil, nil + otherValue, isType := otherGenericValue.([]interface{}) + if !isType { + return nil, incorrectTypeForFlagError(name, "[]interface{}", otherGenericValue) + } + + var stringSlice = make([]string, 0, len(otherValue)) + for i, v := range otherValue { + stringValue, isType := v.(string) + + if !isType { + return nil, incorrectTypeForFlagError(fmt.Sprintf("%s[%d]", name, i), "string", v) + } + + stringSlice = append(stringSlice, stringValue) + } + + return stringSlice, nil } // IntSlice returns an []int from the map if it exists otherwise returns nil func (fsm *MapInputSource) IntSlice(name string) ([]int, error) { otherGenericValue, exists := fsm.valueMap[name] - if exists { - otherValue, isType := otherGenericValue.([]int) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]int", otherGenericValue) + if !exists { + otherGenericValue, exists = nestedVal(name, fsm.valueMap) + if !exists { + return nil, nil } - return otherValue, nil - } - nestedGenericValue, exists := nestedVal(name, fsm.valueMap) - if exists { - otherValue, isType := nestedGenericValue.([]int) - if !isType { - return nil, incorrectTypeForFlagError(name, "[]int", nestedGenericValue) - } - return otherValue, nil } - return nil, nil + otherValue, isType := otherGenericValue.([]interface{}) + if !isType { + return nil, incorrectTypeForFlagError(name, "[]interface{}", otherGenericValue) + } + + var intSlice = make([]int, 0, len(otherValue)) + for i, v := range otherValue { + intValue, isType := v.(int) + + if !isType { + return nil, incorrectTypeForFlagError(fmt.Sprintf("%s[%d]", name, i), "int", v) + } + + intSlice = append(intSlice, intValue) + } + + return intSlice, nil } // Generic returns an cli.Generic from the map if it exists otherwise returns nil From e87dfbb6bb85250206841a9576e51b1323910356 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 16 Jan 2017 17:34:12 -0800 Subject: [PATCH 38/57] altsrc: Support slices for TOML --- altsrc/toml_file_loader.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index 39c124f..37870fc 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -57,8 +57,8 @@ func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { } else { return nil, err } - case reflect.Array: - fallthrough // [todo] - Support array type + case reflect.Array, reflect.Slice: + ret[key] = val.([]interface{}) default: return nil, fmt.Errorf("Unsupported: type = %#v", v.Kind()) } From 4ed366e2011dfb9efa3399c709af0976d5e87db4 Mon Sep 17 00:00:00 2001 From: Robert Bittle Date: Wed, 18 Jan 2017 23:29:26 -0500 Subject: [PATCH 39/57] Pass the ErrWriter on the root app to subcommands --- command.go | 1 + command_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/command.go b/command.go index d297eb9..4a409d4 100644 --- a/command.go +++ b/command.go @@ -264,6 +264,7 @@ func (c Command) startApp(ctx *Context) error { app.Author = ctx.App.Author app.Email = ctx.App.Email app.Writer = ctx.App.Writer + app.ErrWriter = ctx.App.ErrWriter app.categories = CommandCategories{} for _, command := range c.Subcommands { diff --git a/command_test.go b/command_test.go index 5e0e8de..5710184 100644 --- a/command_test.go +++ b/command_test.go @@ -153,3 +153,32 @@ func TestCommand_OnUsageError_WithWrongFlagValue(t *testing.T) { t.Errorf("Expect an intercepted error, but got \"%v\"", err) } } + +func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) { + app := NewApp() + app.ErrWriter = ioutil.Discard + app.Commands = []Command{ + { + Name: "bar", + Usage: "this is for testing", + Subcommands: []Command{ + { + Name: "baz", + Usage: "this is for testing", + Action: func(c *Context) error { + if c.App.ErrWriter != ioutil.Discard { + return fmt.Errorf("ErrWriter not passed") + } + + return nil + }, + }, + }, + }, + } + + err := app.Run([]string{"foo", "bar", "baz"}) + if err != nil { + t.Fatal(err) + } +} From 2526b57c56f30b50466c96c4133b1a4ad0f0191f Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 14 Feb 2017 21:17:05 -0800 Subject: [PATCH 40/57] Allow slightly longer lines in Python scripts --- .flake8 | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .flake8 diff --git a/.flake8 b/.flake8 new file mode 100644 index 0000000..6deafc2 --- /dev/null +++ b/.flake8 @@ -0,0 +1,2 @@ +[flake8] +max-line-length = 120 From 8d8f927bcb0447918aaa09c5b87160ddf2e5842b Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 4 Mar 2017 14:28:24 -0800 Subject: [PATCH 41/57] Change flag test error checking to use regexp rather than strings As the error messages changed in 1.8 --- flag_test.go | 92 +++++++++++++++++++++++++++++----------------------- 1 file changed, 52 insertions(+), 40 deletions(-) diff --git a/flag_test.go b/flag_test.go index 0dd8654..1ccb639 100644 --- a/flag_test.go +++ b/flag_test.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "reflect" + "regexp" "runtime" "strings" "testing" @@ -31,57 +32,57 @@ func TestBoolFlagHelpOutput(t *testing.T) { func TestFlagsFromEnv(t *testing.T) { var flagTests = []struct { - input string - output interface{} - flag Flag - err error + input string + output interface{} + flag Flag + errRegexp string }{ - {"", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"1", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"false", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"foobar", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Errorf(`could not parse foobar as bool value for flag debug: strconv.ParseBool: parsing "foobar": invalid syntax`)}, + {"", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"1", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"false", false, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"foobar", true, BoolFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Sprintf(`could not parse foobar as bool value for flag debug: .*`)}, - {"", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"1", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"false", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, nil}, - {"foobar", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Errorf(`could not parse foobar as bool value for flag debug: strconv.ParseBool: parsing "foobar": invalid syntax`)}, + {"", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"1", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"false", false, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, ""}, + {"foobar", true, BoolTFlag{Name: "debug", EnvVar: "DEBUG"}, fmt.Sprintf(`could not parse foobar as bool value for flag debug: .*`)}, - {"1s", 1 * time.Second, DurationFlag{Name: "time", EnvVar: "TIME"}, nil}, - {"foobar", false, DurationFlag{Name: "time", EnvVar: "TIME"}, fmt.Errorf(`could not parse foobar as duration for flag time: time: invalid duration foobar`)}, + {"1s", 1 * time.Second, DurationFlag{Name: "time", EnvVar: "TIME"}, ""}, + {"foobar", false, DurationFlag{Name: "time", EnvVar: "TIME"}, fmt.Sprintf(`could not parse foobar as duration for flag time: .*`)}, - {"1.2", 1.2, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1", 1.0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"foobar", 0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as float64 value for flag seconds: strconv.ParseFloat: parsing "foobar": invalid syntax`)}, + {"1.2", 1.2, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1", 1.0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"foobar", 0, Float64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as float64 value for flag seconds: .*`)}, - {"1", int64(1), Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as int value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1", int64(1), Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as int value for flag seconds: .*`)}, + {"foobar", 0, Int64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int value for flag seconds: .*`)}, - {"1", 1, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as int value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1", 1, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as int value for flag seconds: .*`)}, + {"foobar", 0, IntFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int value for flag seconds: .*`)}, - {"1,2", IntSlice{1, 2}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2,2", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2,2 as int slice value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int slice value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1,2", IntSlice{1, 2}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2,2", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2,2 as int slice value for flag seconds: .*`)}, + {"foobar", IntSlice{}, IntSliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int slice value for flag seconds: .*`)}, - {"1,2", Int64Slice{1, 2}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2,2", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2,2 as int64 slice value for flag seconds: strconv.ParseInt: parsing "1.2": invalid syntax`)}, - {"foobar", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as int64 slice value for flag seconds: strconv.ParseInt: parsing "foobar": invalid syntax`)}, + {"1,2", Int64Slice{1, 2}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2,2", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2,2 as int64 slice value for flag seconds: .*`)}, + {"foobar", Int64Slice{}, Int64SliceFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as int64 slice value for flag seconds: .*`)}, - {"foo", "foo", StringFlag{Name: "name", EnvVar: "NAME"}, nil}, + {"foo", "foo", StringFlag{Name: "name", EnvVar: "NAME"}, ""}, - {"foo,bar", StringSlice{"foo", "bar"}, StringSliceFlag{Name: "names", EnvVar: "NAMES"}, nil}, + {"foo,bar", StringSlice{"foo", "bar"}, StringSliceFlag{Name: "names", EnvVar: "NAMES"}, ""}, - {"1", uint(1), UintFlag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as uint value for flag seconds: strconv.ParseUint: parsing "1.2": invalid syntax`)}, - {"foobar", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as uint value for flag seconds: strconv.ParseUint: parsing "foobar": invalid syntax`)}, + {"1", uint(1), UintFlag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as uint value for flag seconds: .*`)}, + {"foobar", 0, UintFlag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as uint value for flag seconds: .*`)}, - {"1", uint64(1), Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, nil}, - {"1.2", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse 1.2 as uint64 value for flag seconds: strconv.ParseUint: parsing "1.2": invalid syntax`)}, - {"foobar", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Errorf(`could not parse foobar as uint64 value for flag seconds: strconv.ParseUint: parsing "foobar": invalid syntax`)}, + {"1", uint64(1), Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, ""}, + {"1.2", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse 1.2 as uint64 value for flag seconds: .*`)}, + {"foobar", 0, Uint64Flag{Name: "seconds", EnvVar: "SECONDS"}, fmt.Sprintf(`could not parse foobar as uint64 value for flag seconds: .*`)}, - {"foo,bar", &Parser{"foo", "bar"}, GenericFlag{Name: "names", Value: &Parser{}, EnvVar: "NAMES"}, nil}, + {"foo,bar", &Parser{"foo", "bar"}, GenericFlag{Name: "names", Value: &Parser{}, EnvVar: "NAMES"}, ""}, } for _, test := range flagTests { @@ -98,8 +99,19 @@ func TestFlagsFromEnv(t *testing.T) { } err := a.Run([]string{"run"}) - if !reflect.DeepEqual(test.err, err) { - t.Errorf("expected error %s, got error %s", test.err, err) + + if test.errRegexp != "" { + if err == nil { + t.Errorf("expected error to match %s, got none", test.errRegexp) + } else { + if matched, _ := regexp.MatchString(test.errRegexp, err.Error()); !matched { + t.Errorf("expected error to match %s, got error %s", test.errRegexp, err) + } + } + } else { + if err != nil && test.errRegexp == "" { + t.Errorf("expected no error got %s", err) + } } } } From 9e5b04886c4bfee2ceba1465b8121057355c4e53 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sat, 4 Mar 2017 14:33:36 -0800 Subject: [PATCH 42/57] Remove logic that exited even if the error was not an OsExiter This was introduced in #496, but was discovered to be a regression in the behavior of the library. --- errors.go | 9 --------- errors_test.go | 52 +------------------------------------------------- 2 files changed, 1 insertion(+), 60 deletions(-) diff --git a/errors.go b/errors.go index f9d648e..562b295 100644 --- a/errors.go +++ b/errors.go @@ -97,15 +97,6 @@ func HandleExitCoder(err error) { OsExiter(code) return } - - if err.Error() != "" { - if _, ok := err.(ErrorFormatter); ok { - fmt.Fprintf(ErrWriter, "%+v\n", err) - } else { - fmt.Fprintln(ErrWriter, err) - } - } - OsExiter(1) } func handleMultiError(multiErr MultiError) int { diff --git a/errors_test.go b/errors_test.go index d9e0da6..9b609c5 100644 --- a/errors_test.go +++ b/errors_test.go @@ -67,56 +67,6 @@ func TestHandleExitCoder_MultiErrorWithExitCoder(t *testing.T) { expect(t, called, true) } -func TestHandleExitCoder_ErrorWithMessage(t *testing.T) { - exitCode := 0 - called := false - - OsExiter = func(rc int) { - if !called { - exitCode = rc - called = true - } - } - ErrWriter = &bytes.Buffer{} - - defer func() { - OsExiter = fakeOsExiter - ErrWriter = fakeErrWriter - }() - - err := errors.New("gourd havens") - HandleExitCoder(err) - - expect(t, exitCode, 1) - expect(t, called, true) - expect(t, ErrWriter.(*bytes.Buffer).String(), "gourd havens\n") -} - -func TestHandleExitCoder_ErrorWithoutMessage(t *testing.T) { - exitCode := 0 - called := false - - OsExiter = func(rc int) { - if !called { - exitCode = rc - called = true - } - } - ErrWriter = &bytes.Buffer{} - - defer func() { - OsExiter = fakeOsExiter - ErrWriter = fakeErrWriter - }() - - err := errors.New("") - HandleExitCoder(err) - - expect(t, exitCode, 1) - expect(t, called, true) - expect(t, ErrWriter.(*bytes.Buffer).String(), "") -} - // make a stub to not import pkg/errors type ErrorWithFormat struct { error @@ -145,7 +95,7 @@ func TestHandleExitCoder_ErrorWithFormat(t *testing.T) { ErrWriter = fakeErrWriter }() - err := NewErrorWithFormat("I am formatted") + err := NewExitError(NewErrorWithFormat("I am formatted"), 1) HandleExitCoder(err) expect(t, called, true) From b9c535392027a122ff9776531002d450821426e5 Mon Sep 17 00:00:00 2001 From: Tony Date: Thu, 6 Apr 2017 15:11:23 +0200 Subject: [PATCH 43/57] Unset PROG variable to fix issue when mulitple apps run autocomplete from etc/bash_completion.d directory --- autocomplete/bash_autocomplete | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) mode change 100644 => 100755 autocomplete/bash_autocomplete diff --git a/autocomplete/bash_autocomplete b/autocomplete/bash_autocomplete old mode 100644 new mode 100755 index 21a232f..37d9c14 --- a/autocomplete/bash_autocomplete +++ b/autocomplete/bash_autocomplete @@ -3,12 +3,14 @@ : ${PROG:=$(basename ${BASH_SOURCE})} _cli_bash_autocomplete() { - local cur opts base - COMPREPLY=() - cur="${COMP_WORDS[COMP_CWORD]}" - opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion ) - COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) - return 0 - } - - complete -F _cli_bash_autocomplete $PROG + local cur opts base + COMPREPLY=() + cur="${COMP_WORDS[COMP_CWORD]}" + opts=$( ${COMP_WORDS[@]:0:$COMP_CWORD} --generate-bash-completion ) + COMPREPLY=( $(compgen -W "${opts}" -- ${cur}) ) + return 0 +} + +complete -F _cli_bash_autocomplete $PROG + +unset PROG From 572dc46db5570298570b06ed63ae261086c8c7a4 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 10:39:43 -0700 Subject: [PATCH 44/57] Explicitly fetch `goimports` Fetching the whole tree was failing on some Go versions and we really only need goimports. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 94836d7..4417251 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,7 @@ matrix: before_script: - go get github.com/urfave/gfmrun/... || true -- go get golang.org/x/tools/... || true +- go get golang.org/x/tools/cmd/goimports - if [ ! -f node_modules/.bin/markdown-toc ] ; then npm install markdown-toc ; fi From 1bf0bb96b7b005507c19a2d71f4f3cadcc1202e2 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 10:54:32 -0700 Subject: [PATCH 45/57] Only support supported Go versions As described on https://github.com/golang/go/wiki/Go-Release-Cycle The maintainers do not test compatibility for libraries (e.g. in golang.org/x/tools) on older versions. --- .travis.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 4417251..890e185 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,12 +7,9 @@ cache: - node_modules go: -- 1.2.x -- 1.3.x -- 1.4.2 -- 1.5.x - 1.6.x - 1.7.x +- 1.8.x - master matrix: @@ -23,6 +20,8 @@ matrix: os: osx - go: 1.7.x os: osx + - go: 1.8.x + os: osx before_script: - go get github.com/urfave/gfmrun/... || true From 87fe13079e3f452a366ae8d7f991261f9324d630 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 15:19:34 -0700 Subject: [PATCH 46/57] Rely on Command context in Run() Was previously relying on the parent context which caused things like `.Command` to not be available to OnUsageError(). Fixes #609 --- command.go | 16 ++++++++-------- command_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/command.go b/command.go index 40ebdb6..63f183a 100644 --- a/command.go +++ b/command.go @@ -154,19 +154,20 @@ func (c Command) Run(ctx *Context) (err error) { } context := NewContext(ctx.App, set, ctx) + context.Command = c if checkCommandCompletions(context, c.Name) { return nil } if err != nil { if c.OnUsageError != nil { - err := c.OnUsageError(ctx, err, false) + err := c.OnUsageError(context, err, false) HandleExitCoder(err) return err } - fmt.Fprintln(ctx.App.Writer, "Incorrect Usage:", err.Error()) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) + fmt.Fprintln(context.App.Writer, "Incorrect Usage:", err.Error()) + fmt.Fprintln(context.App.Writer) + ShowCommandHelp(context, c.Name) return err } @@ -191,9 +192,9 @@ func (c Command) Run(ctx *Context) (err error) { if c.Before != nil { err = c.Before(context) if err != nil { - fmt.Fprintln(ctx.App.Writer, err) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) + fmt.Fprintln(context.App.Writer, err) + fmt.Fprintln(context.App.Writer) + ShowCommandHelp(context, c.Name) HandleExitCoder(err) return err } @@ -203,7 +204,6 @@ func (c Command) Run(ctx *Context) (err error) { c.Action = helpSubcommand.Action } - context.Command = c err = HandleAction(c.Action, context) if err != nil { diff --git a/command_test.go b/command_test.go index 5710184..10fff2d 100644 --- a/command_test.go +++ b/command_test.go @@ -127,6 +127,30 @@ func TestCommand_Run_BeforeSavesMetadata(t *testing.T) { } } +func TestCommand_OnUsageError_hasCommandContext(t *testing.T) { + app := NewApp() + app.Commands = []Command{ + { + Name: "bar", + Flags: []Flag{ + IntFlag{Name: "flag"}, + }, + OnUsageError: func(c *Context, err error, _ bool) error { + return fmt.Errorf("intercepted in %s: %s", c.Command.Name, err.Error()) + }, + }, + } + + err := app.Run([]string{"foo", "bar", "--flag=wrong"}) + if err == nil { + t.Fatalf("expected to receive error from Run, got none") + } + + if !strings.HasPrefix(err.Error(), "intercepted in bar") { + t.Errorf("Expect an intercepted error, but got \"%v\"", err) + } +} + func TestCommand_OnUsageError_WithWrongFlagValue(t *testing.T) { app := NewApp() app.Commands = []Command{ From 1794792adfc0f1cea5a4e56eefc36fd849018d05 Mon Sep 17 00:00:00 2001 From: "Joe Richey joerichey@google.com" Date: Fri, 5 May 2017 20:07:18 -0700 Subject: [PATCH 47/57] Add ability to use custom Flag types Users can now use custom flags types (conforming to the Flag interface) in their applications. They can also use custom flags for the three global flags (Help, Version, bash completion). --- app_test.go | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++-- flag.go | 12 +++++------ help.go | 10 ++++----- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/app_test.go b/app_test.go index 10f1562..e14ddaf 100644 --- a/app_test.go +++ b/app_test.go @@ -1520,6 +1520,63 @@ func TestApp_OnUsageError_WithWrongFlagValue_ForSubcommand(t *testing.T) { } } +// A custom flag that conforms to the relevant interfaces, but has none of the +// fields that the other flag types do. +type customBoolFlag struct { + Nombre string +} + +// Don't use the normal FlagStringer +func (c *customBoolFlag) String() string { + return "***" + c.Nombre + "***" +} + +func (c *customBoolFlag) GetName() string { + return c.Nombre +} + +func (c *customBoolFlag) Apply(set *flag.FlagSet) { + set.String(c.Nombre, c.Nombre, "") +} + +func TestCustomFlagsUnused(t *testing.T) { + app := NewApp() + app.Flags = []Flag{&customBoolFlag{"custom"}} + + err := app.Run([]string{"foo"}) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } +} + +func TestCustomFlagsUsed(t *testing.T) { + app := NewApp() + app.Flags = []Flag{&customBoolFlag{"custom"}} + + err := app.Run([]string{"foo", "--custom=bar"}) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } +} + +func TestCustomHelpVersionFlags(t *testing.T) { + app := NewApp() + + // Be sure to reset the global flags + defer func(helpFlag Flag, versionFlag Flag) { + HelpFlag = helpFlag + VersionFlag = versionFlag + }(HelpFlag, VersionFlag) + + HelpFlag = &customBoolFlag{"help-custom"} + VersionFlag = &customBoolFlag{"version-custom"} + + err := app.Run([]string{"foo", "--help-custom=bar"}) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } +} + func TestHandleAction_WithNonFuncAction(t *testing.T) { app := NewApp() app.Action = 42 @@ -1642,7 +1699,7 @@ func TestShellCompletionForIncompleteFlags(t *testing.T) { for _, flag := range ctx.App.Flags { for _, name := range strings.Split(flag.GetName(), ",") { - if name == BashCompletionFlag.Name { + if name == BashCompletionFlag.GetName() { continue } @@ -1659,7 +1716,7 @@ func TestShellCompletionForIncompleteFlags(t *testing.T) { app.Action = func(ctx *Context) error { return fmt.Errorf("should not get here") } - err := app.Run([]string{"", "--test-completion", "--" + BashCompletionFlag.Name}) + err := app.Run([]string{"", "--test-completion", "--" + BashCompletionFlag.GetName()}) if err != nil { t.Errorf("app should not return an error: %s", err) } diff --git a/flag.go b/flag.go index 7dd8a2c..877ff35 100644 --- a/flag.go +++ b/flag.go @@ -14,13 +14,13 @@ import ( const defaultPlaceholder = "value" // BashCompletionFlag enables bash-completion for all commands and subcommands -var BashCompletionFlag = BoolFlag{ +var BashCompletionFlag Flag = BoolFlag{ Name: "generate-bash-completion", Hidden: true, } // VersionFlag prints the version for the application -var VersionFlag = BoolFlag{ +var VersionFlag Flag = BoolFlag{ Name: "version, v", Usage: "print the version", } @@ -28,7 +28,7 @@ var VersionFlag = BoolFlag{ // HelpFlag prints the help for all commands and subcommands // Set to the zero value (BoolFlag{}) to disable flag -- keeps subcommand // unless HideHelp is set to true) -var HelpFlag = BoolFlag{ +var HelpFlag Flag = BoolFlag{ Name: "help, h", Usage: "show help", } @@ -630,7 +630,8 @@ func (f Float64Flag) ApplyWithError(set *flag.FlagSet) error { func visibleFlags(fl []Flag) []Flag { visible := []Flag{} for _, flag := range fl { - if !flagValue(flag).FieldByName("Hidden").Bool() { + field := flagValue(flag).FieldByName("Hidden") + if !field.IsValid() || !field.Bool() { visible = append(visible, flag) } } @@ -723,9 +724,8 @@ func stringifyFlag(f Flag) string { needsPlaceholder := false defaultValueString := "" - val := fv.FieldByName("Value") - if val.IsValid() { + if val := fv.FieldByName("Value"); val.IsValid() { needsPlaceholder = true defaultValueString = fmt.Sprintf(" (default: %v)", val.Interface()) diff --git a/help.go b/help.go index d00e4da..df4cb56 100644 --- a/help.go +++ b/help.go @@ -212,8 +212,8 @@ func printHelp(out io.Writer, templ string, data interface{}) { func checkVersion(c *Context) bool { found := false - if VersionFlag.Name != "" { - eachName(VersionFlag.Name, func(name string) { + if VersionFlag.GetName() != "" { + eachName(VersionFlag.GetName(), func(name string) { if c.GlobalBool(name) || c.Bool(name) { found = true } @@ -224,8 +224,8 @@ func checkVersion(c *Context) bool { func checkHelp(c *Context) bool { found := false - if HelpFlag.Name != "" { - eachName(HelpFlag.Name, func(name string) { + if HelpFlag.GetName() != "" { + eachName(HelpFlag.GetName(), func(name string) { if c.GlobalBool(name) || c.Bool(name) { found = true } @@ -260,7 +260,7 @@ func checkShellCompleteFlag(a *App, arguments []string) (bool, []string) { pos := len(arguments) - 1 lastArg := arguments[pos] - if lastArg != "--"+BashCompletionFlag.Name { + if lastArg != "--"+BashCompletionFlag.GetName() { return false, arguments } From f7d6a07f2d060ba198dc9c00a48fa6dbe03fb7b5 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 25 Nov 2016 00:16:48 -0800 Subject: [PATCH 48/57] Add support for custom help templates. --- app.go | 4 ++++ command.go | 6 ++++++ context.go | 16 +++++++++++++++- help.go | 24 ++++++++++++++++++++++-- 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/app.go b/app.go index 95ffc0b..8f84cb3 100644 --- a/app.go +++ b/app.go @@ -85,6 +85,10 @@ type App struct { ErrWriter io.Writer // Other custom info Metadata map[string]interface{} + // CustomAppHelpTemplate the text template for app help topic. + // cli.go uses text/template to render templates. You can + // render custom help text by setting this variable. + CustomAppHelpTemplate string didSetup bool } diff --git a/command.go b/command.go index 63f183a..a83495e 100644 --- a/command.go +++ b/command.go @@ -59,6 +59,11 @@ type Command struct { // Full name of command for help, defaults to full command name, including parent commands. HelpName string commandNamePath []string + + // CustomHelpTemplate the text template for the command help topic. + // cli.go uses text/template to render templates. You can + // render custom help text by setting this variable. + CustomHelpTemplate string } type CommandsByName []Command @@ -250,6 +255,7 @@ func (c Command) startApp(ctx *Context) error { // set CommandNotFound app.CommandNotFound = ctx.App.CommandNotFound + app.CustomAppHelpTemplate = c.CustomHelpTemplate // set the flags and commands app.Commands = c.Subcommands diff --git a/context.go b/context.go index cb89e92..021e5e5 100644 --- a/context.go +++ b/context.go @@ -186,9 +186,23 @@ func (a Args) First() string { return a.Get(0) } +// Last - Return the last argument, or else a blank string +func (a Args) Last() string { + return a.Get(len(a) - 1) +} + +// Head - Return the rest of the arguments (not the last one) +// or else an empty string slice +func (a Args) Head() Args { + if len(a) == 1 { + return a + } + return []string(a)[:len(a)-1] +} + // Tail returns the rest of the arguments (not the first one) // or else an empty string slice -func (a Args) Tail() []string { +func (a Args) Tail() Args { if len(a) >= 2 { return []string(a)[1:] } diff --git a/help.go b/help.go index d00e4da..78f84f5 100644 --- a/help.go +++ b/help.go @@ -120,9 +120,19 @@ var HelpPrinter helpPrinter = printHelp // VersionPrinter prints the version for the App var VersionPrinter = printVersion +// ShowAppHelpAndExit - Prints the list of subcommands for the app and exits with exit code. +func ShowAppHelpAndExit(c *Context, exitCode int) { + ShowAppHelp(c) + os.Exit(exitCode) +} + // ShowAppHelp is an action that displays the help. func ShowAppHelp(c *Context) error { - HelpPrinter(c.App.Writer, AppHelpTemplate, c.App) + if c.App.CustomAppHelpTemplate != "" { + HelpPrinter(c.App.Writer, c.App.CustomAppHelpTemplate, c.App) + } else { + HelpPrinter(c.App.Writer, AppHelpTemplate, c.App) + } return nil } @@ -138,6 +148,12 @@ func DefaultAppComplete(c *Context) { } } +// ShowCommandHelpAndExit - exits with code after showing help +func ShowCommandHelpAndExit(c *Context, command string, code int) { + ShowCommandHelp(c, command) + os.Exit(code) +} + // ShowCommandHelp prints help for the given command func ShowCommandHelp(ctx *Context, command string) error { // show the subcommand help for a command with subcommands @@ -148,7 +164,11 @@ func ShowCommandHelp(ctx *Context, command string) error { for _, c := range ctx.App.Commands { if c.HasName(command) { - HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c) + if c.CustomHelpTemplate != "" { + HelpPrinter(ctx.App.Writer, c.CustomHelpTemplate, c) + } else { + HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c) + } return nil } } From baa33cb888078362b0b955d6f8715445ad2cf662 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Fri, 25 Nov 2016 01:07:42 -0800 Subject: [PATCH 49/57] Add support for ExtraInfo. --- app.go | 2 ++ help.go | 26 ++++++++++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index 8f84cb3..26a4d4d 100644 --- a/app.go +++ b/app.go @@ -85,6 +85,8 @@ type App struct { ErrWriter io.Writer // Other custom info Metadata map[string]interface{} + // Carries a function which returns app specific info. + ExtraInfo func() map[string]string // CustomAppHelpTemplate the text template for app help topic. // cli.go uses text/template to render templates. You can // render custom help text by setting this variable. diff --git a/help.go b/help.go index 78f84f5..b8ffee0 100644 --- a/help.go +++ b/help.go @@ -112,11 +112,18 @@ var helpSubcommand = Command{ // Prints help for the App or Command type helpPrinter func(w io.Writer, templ string, data interface{}) +// Prints help for the App or Command with custom template function. +type helpPrinterCustom func(w io.Writer, templ string, data interface{}, customFunc map[string]interface{}) + // HelpPrinter is a function that writes the help output. If not set a default // is used. The function signature is: // func(w io.Writer, templ string, data interface{}) var HelpPrinter helpPrinter = printHelp +// HelPrinterCustom is same as HelpPrinter but +// takes a custom function for template function map. +var HelpPrinterCustom helpPrinterCustom = printHelpCustom + // VersionPrinter prints the version for the App var VersionPrinter = printVersion @@ -129,7 +136,13 @@ func ShowAppHelpAndExit(c *Context, exitCode int) { // ShowAppHelp is an action that displays the help. func ShowAppHelp(c *Context) error { if c.App.CustomAppHelpTemplate != "" { - HelpPrinter(c.App.Writer, c.App.CustomAppHelpTemplate, c.App) + if c.App.ExtraInfo != nil { + HelpPrinterCustom(c.App.Writer, c.App.CustomAppHelpTemplate, c.App, map[string]interface{}{ + "ExtraInfo": c.App.ExtraInfo, + }) + } else { + HelpPrinter(c.App.Writer, c.App.CustomAppHelpTemplate, c.App) + } } else { HelpPrinter(c.App.Writer, AppHelpTemplate, c.App) } @@ -211,10 +224,15 @@ func ShowCommandCompletions(ctx *Context, command string) { } } -func printHelp(out io.Writer, templ string, data interface{}) { +func printHelpCustom(out io.Writer, templ string, data interface{}, customFunc map[string]interface{}) { funcMap := template.FuncMap{ "join": strings.Join, } + if customFunc != nil { + for key, value := range customFunc { + funcMap[key] = value + } + } w := tabwriter.NewWriter(out, 1, 8, 2, ' ', 0) t := template.Must(template.New("help").Funcs(funcMap).Parse(templ)) @@ -230,6 +248,10 @@ func printHelp(out io.Writer, templ string, data interface{}) { w.Flush() } +func printHelp(out io.Writer, templ string, data interface{}) { + printHelpCustom(out, templ, data, nil) +} + func checkVersion(c *Context) bool { found := false if VersionFlag.Name != "" { From dd3849a7e602d4506eace87bed5202d9d416f44f Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 15 Feb 2017 01:44:04 -0800 Subject: [PATCH 50/57] Add tests as requested. --- context.go | 16 +------ help.go | 26 +++++------ help_test.go | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 27 deletions(-) diff --git a/context.go b/context.go index 021e5e5..cb89e92 100644 --- a/context.go +++ b/context.go @@ -186,23 +186,9 @@ func (a Args) First() string { return a.Get(0) } -// Last - Return the last argument, or else a blank string -func (a Args) Last() string { - return a.Get(len(a) - 1) -} - -// Head - Return the rest of the arguments (not the last one) -// or else an empty string slice -func (a Args) Head() Args { - if len(a) == 1 { - return a - } - return []string(a)[:len(a)-1] -} - // Tail returns the rest of the arguments (not the first one) // or else an empty string slice -func (a Args) Tail() Args { +func (a Args) Tail() []string { if len(a) >= 2 { return []string(a)[1:] } diff --git a/help.go b/help.go index b8ffee0..e5602d8 100644 --- a/help.go +++ b/help.go @@ -120,7 +120,7 @@ type helpPrinterCustom func(w io.Writer, templ string, data interface{}, customF // func(w io.Writer, templ string, data interface{}) var HelpPrinter helpPrinter = printHelp -// HelPrinterCustom is same as HelpPrinter but +// HelpPrinterCustom is same as HelpPrinter but // takes a custom function for template function map. var HelpPrinterCustom helpPrinterCustom = printHelpCustom @@ -134,18 +134,20 @@ func ShowAppHelpAndExit(c *Context, exitCode int) { } // ShowAppHelp is an action that displays the help. -func ShowAppHelp(c *Context) error { - if c.App.CustomAppHelpTemplate != "" { - if c.App.ExtraInfo != nil { - HelpPrinterCustom(c.App.Writer, c.App.CustomAppHelpTemplate, c.App, map[string]interface{}{ - "ExtraInfo": c.App.ExtraInfo, - }) - } else { - HelpPrinter(c.App.Writer, c.App.CustomAppHelpTemplate, c.App) - } - } else { +func ShowAppHelp(c *Context) (err error) { + if c.App.CustomAppHelpTemplate == "" { HelpPrinter(c.App.Writer, AppHelpTemplate, c.App) + return } + customAppData := func() map[string]interface{} { + if c.App.ExtraInfo == nil { + return nil + } + return map[string]interface{}{ + "ExtraInfo": c.App.ExtraInfo, + } + } + HelpPrinterCustom(c.App.Writer, c.App.CustomAppHelpTemplate, c.App, customAppData()) return nil } @@ -178,7 +180,7 @@ func ShowCommandHelp(ctx *Context, command string) error { for _, c := range ctx.App.Commands { if c.HasName(command) { if c.CustomHelpTemplate != "" { - HelpPrinter(ctx.App.Writer, c.CustomHelpTemplate, c) + HelpPrinterCustom(ctx.App.Writer, c.CustomHelpTemplate, c, nil) } else { HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c) } diff --git a/help_test.go b/help_test.go index 7c15400..78d8973 100644 --- a/help_test.go +++ b/help_test.go @@ -3,6 +3,8 @@ package cli import ( "bytes" "flag" + "fmt" + "runtime" "strings" "testing" ) @@ -256,6 +258,49 @@ func TestShowSubcommandHelp_CommandAliases(t *testing.T) { } } +func TestShowCommandHelp_Customtemplate(t *testing.T) { + app := &App{ + Commands: []Command{ + { + Name: "frobbly", + Action: func(ctx *Context) error { + return nil + }, + HelpName: "foo frobbly", + CustomHelpTemplate: `NAME: + {{.HelpName}} - {{.Usage}} + +USAGE: + {{.HelpName}} [FLAGS] TARGET [TARGET ...] + +FLAGS: + {{range .VisibleFlags}}{{.}} + {{end}} +EXAMPLES: + 1. Frobbly runs with this param locally. + $ {{.HelpName}} wobbly +`, + }, + }, + } + + output := &bytes.Buffer{} + app.Writer = output + app.Run([]string{"foo", "help", "frobbly"}) + + if strings.Contains(output.String(), "2. Frobbly runs without this param locally.") { + t.Errorf("expected output to exclude \"2. Frobbly runs without this param locally.\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "1. Frobbly runs with this param locally.") { + t.Errorf("expected output to include \"1. Frobbly runs with this param locally.\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "$ foo frobbly wobbly") { + t.Errorf("expected output to include \"$ foo frobbly wobbly\"; got: %q", output.String()) + } +} + func TestShowAppHelp_HiddenCommand(t *testing.T) { app := &App{ Commands: []Command{ @@ -287,3 +332,78 @@ func TestShowAppHelp_HiddenCommand(t *testing.T) { t.Errorf("expected output to include \"frobbly\"; got: %q", output.String()) } } + +func TestShowAppHelp_CustomAppTemplate(t *testing.T) { + app := &App{ + Commands: []Command{ + { + Name: "frobbly", + Action: func(ctx *Context) error { + return nil + }, + }, + { + Name: "secretfrob", + Hidden: true, + Action: func(ctx *Context) error { + return nil + }, + }, + }, + ExtraInfo: func() map[string]string { + platform := fmt.Sprintf("OS: %s | Arch: %s", runtime.GOOS, runtime.GOARCH) + goruntime := fmt.Sprintf("Version: %s | CPUs: %d", runtime.Version(), runtime.NumCPU()) + return map[string]string{ + "PLATFORM": platform, + "RUNTIME": goruntime, + } + }, + CustomAppHelpTemplate: `NAME: + {{.Name}} - {{.Usage}} + +USAGE: + {{.Name}} {{if .VisibleFlags}}[FLAGS] {{end}}COMMAND{{if .VisibleFlags}} [COMMAND FLAGS | -h]{{end}} [ARGUMENTS...] + +COMMANDS: + {{range .VisibleCommands}}{{join .Names ", "}}{{ "\t" }}{{.Usage}} + {{end}}{{if .VisibleFlags}} +GLOBAL FLAGS: + {{range .VisibleFlags}}{{.}} + {{end}}{{end}} +VERSION: + 2.0.0 +{{"\n"}}{{range $key, $value := ExtraInfo}} +{{$key}}: + {{$value}} +{{end}}`, + } + + output := &bytes.Buffer{} + app.Writer = output + app.Run([]string{"app", "--help"}) + + if strings.Contains(output.String(), "secretfrob") { + t.Errorf("expected output to exclude \"secretfrob\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "frobbly") { + t.Errorf("expected output to include \"frobbly\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "PLATFORM:") || + !strings.Contains(output.String(), "OS:") || + !strings.Contains(output.String(), "Arch:") { + t.Errorf("expected output to include \"PLATFORM:, OS: and Arch:\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "RUNTIME:") || + !strings.Contains(output.String(), "Version:") || + !strings.Contains(output.String(), "CPUs:") { + t.Errorf("expected output to include \"RUNTIME:, Version: and CPUs:\"; got: %q", output.String()) + } + + if !strings.Contains(output.String(), "VERSION:") || + !strings.Contains(output.String(), "2.0.0") { + t.Errorf("expected output to include \"VERSION:, 2.0.0\"; got: %q", output.String()) + } +} From 291122b8f0ad73134ddee3cdc27b5164e4480ab3 Mon Sep 17 00:00:00 2001 From: "Joe Richey joerichey@google.com" Date: Fri, 5 May 2017 15:01:44 -0700 Subject: [PATCH 51/57] Subcommand OnUsageError should be passed to app Not all of the Command components were being passed to the created App in the startApp function. --- command.go | 1 + command_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/command.go b/command.go index a83495e..bb0733c 100644 --- a/command.go +++ b/command.go @@ -291,6 +291,7 @@ func (c Command) startApp(ctx *Context) error { } else { app.Action = helpSubcommand.Action } + app.OnUsageError = c.OnUsageError for index, cc := range app.Commands { app.Commands[index].commandNamePath = []string{c.Name, cc.Name} diff --git a/command_test.go b/command_test.go index 10fff2d..4ad994c 100644 --- a/command_test.go +++ b/command_test.go @@ -178,6 +178,38 @@ func TestCommand_OnUsageError_WithWrongFlagValue(t *testing.T) { } } +func TestCommand_OnUsageError_WithSubcommand(t *testing.T) { + app := NewApp() + app.Commands = []Command{ + { + Name: "bar", + Subcommands: []Command{ + { + Name: "baz", + }, + }, + Flags: []Flag{ + IntFlag{Name: "flag"}, + }, + OnUsageError: func(c *Context, err error, _ bool) error { + if !strings.HasPrefix(err.Error(), "invalid value \"wrong\"") { + t.Errorf("Expect an invalid value error, but got \"%v\"", err) + } + return errors.New("intercepted: " + err.Error()) + }, + }, + } + + err := app.Run([]string{"foo", "bar", "--flag=wrong"}) + if err == nil { + t.Fatalf("expected to receive error from Run, got none") + } + + if !strings.HasPrefix(err.Error(), "intercepted: invalid value") { + t.Errorf("Expect an intercepted error, but got \"%v\"", err) + } +} + func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) { app := NewApp() app.ErrWriter = ioutil.Discard From c64d74a5d989d144e33854e303f20981f9e8f049 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 24 Apr 2017 14:54:35 -0700 Subject: [PATCH 52/57] Do not double print errors from Before() They should be handled by HandleExitCoder() as `After()` errors are. --- app.go | 1 - command.go | 2 -- 2 files changed, 3 deletions(-) diff --git a/app.go b/app.go index 26a4d4d..51fc45d 100644 --- a/app.go +++ b/app.go @@ -240,7 +240,6 @@ func (a *App) Run(arguments []string) (err error) { if a.Before != nil { beforeErr := a.Before(context) if beforeErr != nil { - fmt.Fprintf(a.Writer, "%v\n\n", beforeErr) ShowAppHelp(context) HandleExitCoder(beforeErr) err = beforeErr diff --git a/command.go b/command.go index bb0733c..23de294 100644 --- a/command.go +++ b/command.go @@ -197,8 +197,6 @@ func (c Command) Run(ctx *Context) (err error) { if c.Before != nil { err = c.Before(context) if err != nil { - fmt.Fprintln(context.App.Writer, err) - fmt.Fprintln(context.App.Writer) ShowCommandHelp(context, c.Name) HandleExitCoder(err) return err From 48db8e24356a42996e7d6b0b62c55e0df1d4492e Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 29 Jan 2017 21:05:21 -0800 Subject: [PATCH 53/57] Display UsageText on Commands and Subcommands If it is set, otherwise build the usage. Fixes #592 --- help.go | 4 ++-- help_test.go | 45 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/help.go b/help.go index 216ff78..57ec98d 100644 --- a/help.go +++ b/help.go @@ -47,7 +47,7 @@ var CommandHelpTemplate = `NAME: {{.HelpName}} - {{.Usage}} USAGE: - {{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{if .Category}} + {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}}{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Category}} CATEGORY: {{.Category}}{{end}}{{if .Description}} @@ -67,7 +67,7 @@ var SubcommandHelpTemplate = `NAME: {{.HelpName}} - {{if .Description}}{{.Description}}{{else}}{{.Usage}}{{end}} USAGE: - {{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}} + {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} command{{if .VisibleFlags}} [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}} COMMANDS:{{range .VisibleCategories}}{{if .Name}} {{.Name}}:{{end}}{{range .VisibleCommands}} diff --git a/help_test.go b/help_test.go index 78d8973..70b6300 100644 --- a/help_test.go +++ b/help_test.go @@ -283,7 +283,6 @@ EXAMPLES: }, }, } - output := &bytes.Buffer{} app.Writer = output app.Run([]string{"foo", "help", "frobbly"}) @@ -301,6 +300,50 @@ EXAMPLES: } } +func TestShowSubcommandHelp_CommandUsageText(t *testing.T) { + app := &App{ + Commands: []Command{ + { + Name: "frobbly", + UsageText: "this is usage text", + }, + }, + } + + output := &bytes.Buffer{} + app.Writer = output + + app.Run([]string{"foo", "frobbly", "--help"}) + + if !strings.Contains(output.String(), "this is usage text") { + t.Errorf("expected output to include usage text; got: %q", output.String()) + } +} + +func TestShowSubcommandHelp_SubcommandUsageText(t *testing.T) { + app := &App{ + Commands: []Command{ + { + Name: "frobbly", + Subcommands: []Command{ + { + Name: "bobbly", + UsageText: "this is usage text", + }, + }, + }, + }, + } + + output := &bytes.Buffer{} + app.Writer = output + app.Run([]string{"foo", "frobbly", "bobbly", "--help"}) + + if !strings.Contains(output.String(), "this is usage text") { + t.Errorf("expected output to include usage text; got: %q", output.String()) + } +} + func TestShowAppHelp_HiddenCommand(t *testing.T) { app := &App{ Commands: []Command{ From a6dd54e94bbb0c70ec72a144d24e24e2247b3b2e Mon Sep 17 00:00:00 2001 From: Nicolas Gailly Date: Thu, 6 Jul 2017 16:20:09 +0200 Subject: [PATCH 54/57] make the basic example build-able. --- cli.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cli.go b/cli.go index 74fd101..90c07eb 100644 --- a/cli.go +++ b/cli.go @@ -12,6 +12,7 @@ // app.Usage = "say a greeting" // app.Action = func(c *cli.Context) error { // println("Greetings") +// return nil // } // // app.Run(os.Args) From 8164aa88ca94f405d0c73ddca4db3a57424bcf69 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 3 Aug 2017 15:07:56 -0400 Subject: [PATCH 55/57] Update travis config to only test on 1.8.x, and ensure we route to Ubuntu Trusty and a recent macOS image. --- .travis.yml | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index 890e185..1ee4604 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,27 +1,18 @@ language: go - sudo: false +dist: trusty cache: directories: - node_modules -go: -- 1.6.x -- 1.7.x -- 1.8.x -- master +go: 1.8.x matrix: - allow_failures: - - go: master include: - - go: 1.6.x - os: osx - - go: 1.7.x - os: osx - go: 1.8.x os: osx + osx_image: xcode8.3 before_script: - go get github.com/urfave/gfmrun/... || true From 184ffb5e573ba0656a123462715d360faaa0c897 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 3 Aug 2017 15:38:19 -0400 Subject: [PATCH 56/57] Use `os` matrix instead of explicit matrix.include --- .travis.yml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1ee4604..cf8d098 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,19 +1,17 @@ language: go sudo: false dist: trusty +osx_image: xcode8.3 +go: 1.8.x + +os: +- linux +- osx cache: directories: - node_modules -go: 1.8.x - -matrix: - include: - - go: 1.8.x - os: osx - osx_image: xcode8.3 - before_script: - go get github.com/urfave/gfmrun/... || true - go get golang.org/x/tools/cmd/goimports From f59114b4100b20e092035199640f5fe8bce07daa Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 3 Aug 2017 22:59:38 -0400 Subject: [PATCH 57/57] Update os, image, golang, and python versions used in appveyor --- appveyor.yml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index 698b188..1e1489c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,14 +1,16 @@ version: "{build}" -os: Windows Server 2012 R2 +os: Windows Server 2016 + +image: Visual Studio 2017 clone_folder: c:\gopath\src\github.com\urfave\cli environment: GOPATH: C:\gopath - GOVERSION: 1.6 - PYTHON: C:\Python27-x64 - PYTHON_VERSION: 2.7.x + GOVERSION: 1.8.x + PYTHON: C:\Python36-x64 + PYTHON_VERSION: 3.6.x PYTHON_ARCH: 64 install: