From b5d06bd2a9cddb942cf282cad11c97c251b6f6c7 Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Thu, 17 Nov 2016 16:58:46 +0100 Subject: [PATCH 01/42] 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 02/42] 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 6503447ae720ddabdb8902f7aebef525191da02f Mon Sep 17 00:00:00 2001 From: Antonio Fdez Date: Sat, 19 Nov 2016 22:37:11 +0100 Subject: [PATCH 03/42] 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 c48a82964028acd0f19ee17257789f7c9f5afc78 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 09:29:43 -0700 Subject: [PATCH 04/42] Allow custom exit err handlers --- app.go | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/app.go b/app.go index 51fc45d..5d7ed8b 100644 --- a/app.go +++ b/app.go @@ -83,6 +83,8 @@ type App struct { Writer io.Writer // ErrWriter writes error output ErrWriter io.Writer + // Execute this function to handle ExitErrors + ExitErrHandler ExitErrHandlerFunc // Other custom info Metadata map[string]interface{} // Carries a function which returns app specific info. @@ -207,7 +209,7 @@ func (a *App) Run(arguments []string) (err error) { if err != nil { if a.OnUsageError != nil { err := a.OnUsageError(context, err, false) - HandleExitCoder(err) + a.handleExitCoder(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -241,7 +243,7 @@ func (a *App) Run(arguments []string) (err error) { beforeErr := a.Before(context) if beforeErr != nil { ShowAppHelp(context) - HandleExitCoder(beforeErr) + a.handleExitCoder(beforeErr) err = beforeErr return err } @@ -263,7 +265,7 @@ func (a *App) Run(arguments []string) (err error) { // Run default Action err = HandleAction(a.Action, context) - HandleExitCoder(err) + a.handleExitCoder(err) return err } @@ -330,7 +332,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if err != nil { if a.OnUsageError != nil { err = a.OnUsageError(context, err, true) - HandleExitCoder(err) + a.handleExitCoder(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -352,7 +354,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { defer func() { afterErr := a.After(context) if afterErr != nil { - HandleExitCoder(err) + a.handleExitCoder(err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -365,7 +367,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.Before != nil { beforeErr := a.Before(context) if beforeErr != nil { - HandleExitCoder(beforeErr) + a.handleExitCoder(beforeErr) err = beforeErr return err } @@ -383,7 +385,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { // Run default Action err = HandleAction(a.Action, context) - HandleExitCoder(err) + a.handleExitCoder(err) return err } @@ -464,6 +466,14 @@ func (a *App) appendFlag(flag Flag) { } } +func (a *App) handleExitCoder(err error) { + if a.ExitErrHandler != nil { + a.ExitErrHandler(err) + } else { + HandleExitCoder(err) + } +} + // Author represents someone who has contributed to a cli project. type Author struct { Name string // The Authors name From 538742687bbd979a7b4f975468af76ce5cffb972 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 09:31:53 -0700 Subject: [PATCH 05/42] Add ExitErrHandlerFunc type --- funcs.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/funcs.go b/funcs.go index cba5e6c..4737faf 100644 --- a/funcs.go +++ b/funcs.go @@ -26,3 +26,7 @@ type OnUsageErrorFunc func(context *Context, err error, isSubcommand bool) error // FlagStringFunc is used by the help generation to display a flag, which is // expected to be a single line. type FlagStringFunc func(Flag) string + +// ExitErrHandlerFunc is executed if provided in order to handle ExitError values +// returned by Actions and Before/After functions. +type ExitErrHandlerFunc func(error) From 827da610b4bff0ffbc06cd2d92eddae552f7d1a2 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 09:33:54 -0700 Subject: [PATCH 06/42] Add a bit more documentation --- app.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index 5d7ed8b..4138f32 100644 --- a/app.go +++ b/app.go @@ -83,7 +83,8 @@ type App struct { Writer io.Writer // ErrWriter writes error output ErrWriter io.Writer - // Execute this function to handle ExitErrors + // Execute this function to handle ExitErrors. If not provided, HandleExitCoder is provided to + // function as a default, so this is optional. ExitErrHandler ExitErrHandlerFunc // Other custom info Metadata map[string]interface{} From 80b09a4d1117ad69430582685e59dfe560caa948 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 11:20:41 -0700 Subject: [PATCH 07/42] Fix how to do defaults in app.go --- app.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/app.go b/app.go index 4138f32..f1a8f27 100644 --- a/app.go +++ b/app.go @@ -121,6 +121,7 @@ func NewApp() *App { Action: helpCommand.Action, Compiled: compileTime(), Writer: os.Stdout, + ExitErrHandler: HandleExitCoder, } } @@ -210,7 +211,7 @@ func (a *App) Run(arguments []string) (err error) { if err != nil { if a.OnUsageError != nil { err := a.OnUsageError(context, err, false) - a.handleExitCoder(err) + a.ExitErrHandler(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -244,7 +245,7 @@ func (a *App) Run(arguments []string) (err error) { beforeErr := a.Before(context) if beforeErr != nil { ShowAppHelp(context) - a.handleExitCoder(beforeErr) + a.ExitErrHandler(beforeErr) err = beforeErr return err } @@ -266,7 +267,7 @@ func (a *App) Run(arguments []string) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.handleExitCoder(err) + a.ExitErrHandler(err) return err } @@ -333,7 +334,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if err != nil { if a.OnUsageError != nil { err = a.OnUsageError(context, err, true) - a.handleExitCoder(err) + a.ExitErrHandler(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -355,7 +356,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { defer func() { afterErr := a.After(context) if afterErr != nil { - a.handleExitCoder(err) + a.ExitErrHandler(err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -368,7 +369,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.Before != nil { beforeErr := a.Before(context) if beforeErr != nil { - a.handleExitCoder(beforeErr) + a.ExitErrHandler(beforeErr) err = beforeErr return err } @@ -386,7 +387,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.handleExitCoder(err) + a.ExitErrHandler(err) return err } @@ -467,14 +468,6 @@ func (a *App) appendFlag(flag Flag) { } } -func (a *App) handleExitCoder(err error) { - if a.ExitErrHandler != nil { - a.ExitErrHandler(err) - } else { - HandleExitCoder(err) - } -} - // Author represents someone who has contributed to a cli project. type Author struct { Name string // The Authors name From ceee6408d5cbbb9f113157d0a62b1ffed1f2b510 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 13:02:05 -0700 Subject: [PATCH 08/42] Revert "Fix how to do defaults in app.go" This reverts commit 8906567dc2ad52fd31c50cf02fa606505a1323ba. --- app.go | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/app.go b/app.go index f1a8f27..e9ed7ab 100644 --- a/app.go +++ b/app.go @@ -121,7 +121,6 @@ func NewApp() *App { Action: helpCommand.Action, Compiled: compileTime(), Writer: os.Stdout, - ExitErrHandler: HandleExitCoder, } } @@ -211,7 +210,7 @@ func (a *App) Run(arguments []string) (err error) { if err != nil { if a.OnUsageError != nil { err := a.OnUsageError(context, err, false) - a.ExitErrHandler(err) + a.handleExitCoder(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -244,8 +243,9 @@ 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) - a.ExitErrHandler(beforeErr) + a.handleExitCoder(beforeErr) err = beforeErr return err } @@ -267,7 +267,7 @@ func (a *App) Run(arguments []string) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.ExitErrHandler(err) + a.handleExitCoder(err) return err } @@ -334,7 +334,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if err != nil { if a.OnUsageError != nil { err = a.OnUsageError(context, err, true) - a.ExitErrHandler(err) + a.handleExitCoder(err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -356,7 +356,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { defer func() { afterErr := a.After(context) if afterErr != nil { - a.ExitErrHandler(err) + a.handleExitCoder(err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -369,7 +369,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.Before != nil { beforeErr := a.Before(context) if beforeErr != nil { - a.ExitErrHandler(beforeErr) + a.handleExitCoder(beforeErr) err = beforeErr return err } @@ -387,7 +387,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.ExitErrHandler(err) + a.handleExitCoder(err) return err } @@ -468,6 +468,14 @@ func (a *App) appendFlag(flag Flag) { } } +func (a *App) handleExitCoder(err error) { + if a.ExitErrHandler != nil { + a.ExitErrHandler(err) + } else { + HandleExitCoder(err) + } +} + // Author represents someone who has contributed to a cli project. type Author struct { Name string // The Authors name From 9d61cbad0260bc7f2a72b07142a0120072e3800a Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Tue, 25 Apr 2017 12:45:08 -0700 Subject: [PATCH 09/42] Updated command.go to use App handleExitCoder --- command.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/command.go b/command.go index 23de294..fffa02f 100644 --- a/command.go +++ b/command.go @@ -167,7 +167,7 @@ func (c Command) Run(ctx *Context) (err error) { if err != nil { if c.OnUsageError != nil { err := c.OnUsageError(context, err, false) - HandleExitCoder(err) + context.App.handleExitCoder(err) return err } fmt.Fprintln(context.App.Writer, "Incorrect Usage:", err.Error()) @@ -184,7 +184,7 @@ func (c Command) Run(ctx *Context) (err error) { defer func() { afterErr := c.After(context) if afterErr != nil { - HandleExitCoder(err) + ctx.App.handleExitCoder(err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -198,7 +198,7 @@ func (c Command) Run(ctx *Context) (err error) { err = c.Before(context) if err != nil { ShowCommandHelp(context, c.Name) - HandleExitCoder(err) + context.App.handleExitCoder(err) return err } } @@ -210,7 +210,7 @@ func (c Command) Run(ctx *Context) (err error) { err = HandleAction(c.Action, context) if err != nil { - HandleExitCoder(err) + ctx.App.handleExitCoder(err) } return err } From 530df59178874f8d792d2d9cfd745464076f1eda Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Wed, 28 Jun 2017 09:52:12 -0700 Subject: [PATCH 10/42] Pass context into handleExitCoder --- app.go | 18 +++++++++--------- command.go | 8 ++++---- funcs.go | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/app.go b/app.go index e9ed7ab..48de351 100644 --- a/app.go +++ b/app.go @@ -210,7 +210,7 @@ func (a *App) Run(arguments []string) (err error) { if err != nil { if a.OnUsageError != nil { err := a.OnUsageError(context, err, false) - a.handleExitCoder(err) + a.handleExitCoder(context, err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -245,7 +245,7 @@ func (a *App) Run(arguments []string) (err error) { if beforeErr != nil { fmt.Fprintf(a.Writer, "%v\n\n", beforeErr) ShowAppHelp(context) - a.handleExitCoder(beforeErr) + a.handleExitCoder(context, beforeErr) err = beforeErr return err } @@ -267,7 +267,7 @@ func (a *App) Run(arguments []string) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.handleExitCoder(err) + a.handleExitCoder(context, err) return err } @@ -334,7 +334,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if err != nil { if a.OnUsageError != nil { err = a.OnUsageError(context, err, true) - a.handleExitCoder(err) + a.handleExitCoder(context, err) return err } fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) @@ -356,7 +356,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { defer func() { afterErr := a.After(context) if afterErr != nil { - a.handleExitCoder(err) + a.handleExitCoder(context, err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -369,7 +369,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.Before != nil { beforeErr := a.Before(context) if beforeErr != nil { - a.handleExitCoder(beforeErr) + a.handleExitCoder(context, beforeErr) err = beforeErr return err } @@ -387,7 +387,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { // Run default Action err = HandleAction(a.Action, context) - a.handleExitCoder(err) + a.handleExitCoder(context, err) return err } @@ -468,9 +468,9 @@ func (a *App) appendFlag(flag Flag) { } } -func (a *App) handleExitCoder(err error) { +func (a *App) handleExitCoder(context *Context, error) { if a.ExitErrHandler != nil { - a.ExitErrHandler(err) + a.ExitErrHandler(context, err) } else { HandleExitCoder(err) } diff --git a/command.go b/command.go index fffa02f..502fc9f 100644 --- a/command.go +++ b/command.go @@ -167,7 +167,7 @@ func (c Command) Run(ctx *Context) (err error) { if err != nil { if c.OnUsageError != nil { err := c.OnUsageError(context, err, false) - context.App.handleExitCoder(err) + context.App.handleExitCoder(context, err) return err } fmt.Fprintln(context.App.Writer, "Incorrect Usage:", err.Error()) @@ -184,7 +184,7 @@ func (c Command) Run(ctx *Context) (err error) { defer func() { afterErr := c.After(context) if afterErr != nil { - ctx.App.handleExitCoder(err) + context.App.handleExitCoder(context, err) if err != nil { err = NewMultiError(err, afterErr) } else { @@ -198,7 +198,7 @@ func (c Command) Run(ctx *Context) (err error) { err = c.Before(context) if err != nil { ShowCommandHelp(context, c.Name) - context.App.handleExitCoder(err) + context.App.handleExitCoder(context, err) return err } } @@ -210,7 +210,7 @@ func (c Command) Run(ctx *Context) (err error) { err = HandleAction(c.Action, context) if err != nil { - ctx.App.handleExitCoder(err) + context.App.handleExitCoder(context, err) } return err } diff --git a/funcs.go b/funcs.go index 4737faf..42023e2 100644 --- a/funcs.go +++ b/funcs.go @@ -29,4 +29,4 @@ type FlagStringFunc func(Flag) string // ExitErrHandlerFunc is executed if provided in order to handle ExitError values // returned by Actions and Before/After functions. -type ExitErrHandlerFunc func(error) +type ExitErrHandlerFunc func(context *Context, error) From 172bb92059ed885c8b4249230f3ccbe9e3e1272b Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Wed, 28 Jun 2017 10:07:25 -0700 Subject: [PATCH 11/42] fix named parameter issue --- app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app.go b/app.go index 48de351..1876b5c 100644 --- a/app.go +++ b/app.go @@ -468,7 +468,7 @@ func (a *App) appendFlag(flag Flag) { } } -func (a *App) handleExitCoder(context *Context, error) { +func (a *App) handleExitCoder(context *Context, err error) { if a.ExitErrHandler != nil { a.ExitErrHandler(context, err) } else { From 71bdf81f5a65dc253482cb727c2ae973ae3b3830 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Wed, 28 Jun 2017 10:10:11 -0700 Subject: [PATCH 12/42] sigh... fix one more named parameter issue --- funcs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/funcs.go b/funcs.go index 42023e2..2274415 100644 --- a/funcs.go +++ b/funcs.go @@ -29,4 +29,4 @@ type FlagStringFunc func(Flag) string // ExitErrHandlerFunc is executed if provided in order to handle ExitError values // returned by Actions and Before/After functions. -type ExitErrHandlerFunc func(context *Context, error) +type ExitErrHandlerFunc func(context *Context, err error) From 58450552ee1bada60f4175897aff8d69f7c904a1 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Wed, 28 Jun 2017 12:52:50 -0700 Subject: [PATCH 13/42] Add Test --- app_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/app_test.go b/app_test.go index e14ddaf..63e28c7 100644 --- a/app_test.go +++ b/app_test.go @@ -1661,6 +1661,42 @@ func TestHandleAction_WithInvalidFuncReturnSignature(t *testing.T) { } } +func TestHandleExitCoder_Default(t *testing.T) { + app := NewApp() + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + + ctx := NewContext(app, fs, nil) + app.handleExitCoder(ctx, errors.New("Default Behavior Error")) + + output := fakeErrWriter.String() + if !strings.Contains(output, "Default") { + t.Fatalf("Expected Default Behavior from Error Handler but got: %s", output) + } +} + +func TestHandleExitCoder_Custom(t *testing.T) { + app := NewApp() + fs, err := flagSet(app.Name, app.Flags) + if err != nil { + t.Errorf("error creating FlagSet: %s", err) + } + + app.ExitErrHandler = func(_ *Context, _ error) { + fmt.Fprintln(ErrWriter, "I'm a Custom error handler, I print what I want!") + } + + ctx := NewContext(app, fs, nil) + app.handleExitCoder(ctx, errors.New("Default Behavior Error")) + + output := fakeErrWriter.String() + if !strings.Contains(output, "Custom") { + t.Fatalf("Expected Custom Behavior from Error Handler but got: %s", output) + } +} + func TestHandleAction_WithUnknownPanic(t *testing.T) { defer func() { refute(t, recover(), nil) }() From 5d528e2052b3e7a49293d6aa0fac245047ea61e3 Mon Sep 17 00:00:00 2001 From: Tyler Davis Date: Wed, 28 Jun 2017 13:04:09 -0700 Subject: [PATCH 14/42] use exit errors in uts --- app_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app_test.go b/app_test.go index 63e28c7..9ff6246 100644 --- a/app_test.go +++ b/app_test.go @@ -1669,7 +1669,7 @@ func TestHandleExitCoder_Default(t *testing.T) { } ctx := NewContext(app, fs, nil) - app.handleExitCoder(ctx, errors.New("Default Behavior Error")) + app.handleExitCoder(ctx, NewExitError("Default Behavior Error", 42)) output := fakeErrWriter.String() if !strings.Contains(output, "Default") { @@ -1689,7 +1689,7 @@ func TestHandleExitCoder_Custom(t *testing.T) { } ctx := NewContext(app, fs, nil) - app.handleExitCoder(ctx, errors.New("Default Behavior Error")) + app.handleExitCoder(ctx, NewExitError("Default Behavior Error", 42)) output := fakeErrWriter.String() if !strings.Contains(output, "Custom") { From 8164aa88ca94f405d0c73ddca4db3a57424bcf69 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 3 Aug 2017 15:07:56 -0400 Subject: [PATCH 15/42] 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 16/42] 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 17/42] 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: From e1fa109a3195a9fedcb635841ca1907b764ada1f Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Thu, 10 Aug 2017 17:54:24 -0700 Subject: [PATCH 18/42] Define flag source precedence in README Fixes #646 --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 2bbbd8e..883cc10 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,7 @@ applications in an expressive way. + [Ordering](#ordering) + [Values from the Environment](#values-from-the-environment) + [Values from alternate input sources (YAML, TOML, and others)](#values-from-alternate-input-sources-yaml-toml-and-others) + + [Precedence](#precedence) * [Subcommands](#subcommands) * [Subcommands categories](#subcommands-categories) * [Exit code](#exit-code) @@ -656,6 +657,15 @@ func main() { } ``` +#### Precedence + +The precedence for flag value sources is as follows (highest to lowest): + +0. Command line flag value from user +0. Environment variable (if specified) +0. Configuration file (if specified) +0. Default defined on the flag + ### Subcommands Subcommands can be defined for a more git-like command line app. From cfb38830724cc34fedffe9a2a29fb54fa9169cd1 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Thu, 10 Aug 2017 18:42:03 -0700 Subject: [PATCH 19/42] Prepare CHANGELOG for v1.20.0 release --- CHANGELOG.md | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07f7546..401eae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,49 @@ ## [Unreleased] +## 1.20.0 - 2017-08-10 + +### Fixed + +* `HandleExitCoder` is now correctly iterates over all errors in + a `MultiError`. The exit code is the exit code of the last error or `1` if + there are no `ExitCoder`s in the `MultiError`. +* Fixed YAML file loading on Windows (previously would fail validate the file path) +* Subcommand `Usage`, `Description`, `ArgsUsage`, `OnUsageError` correctly + propogated +* `ErrWriter` is now passed downwards through command structure to avoid the + need to redefine it +* Pass `Command` context into `OnUsageError` rather than parent context so that + all fields are avaiable +* Errors occuring in `Before` funcs are no longer double printed +* Use `UsageText` in the help templates for commands and subcommands if + defined; otherwise build the usage as before (was previously ignoring this + field) +* `IsSet` and `GlobalIsSet` now correctly return whether a flag is set if + a program calls `Set` or `GlobalSet` directly after flag parsing (would + previously only return `true` if the flag was set during parsing) + +### Changed + +* No longer exit the program on command/subcommand error if the error raised is + not an `OsExiter`. This exiting behavior was introduced in 1.19.0, but was + determined to be a regression in functionality. See [the + PR](https://github.com/urfave/cli/pull/595) for discussion. + +### Added + +* `CommandsByName` type was added to make it easy to sort `Command`s by name, + alphabetically +* `altsrc` now handles loading of string and int arrays from TOML +* Support for definition of custom help templates for `App` via + `CustomAppHelpTemplate` +* Support for arbitrary key/value fields on `App` to be used with + `CustomAppHelpTemplate` via `ExtraInfo` +* `HelpFlag`, `VersionFlag`, and `BashCompletionFlag` changed to explictly be + `cli.Flag`s allowing for the use of custom flags satisfying the `cli.Flag` + interface to be used. + + ## [1.19.1] - 2016-11-21 ### Fixed From 1d334f10ce73c2b9e65c50a2290a86be3c743ff2 Mon Sep 17 00:00:00 2001 From: "Alan D. Cabrera" Date: Fri, 8 Sep 2017 10:37:48 -0700 Subject: [PATCH 20/42] Add newline before command categories The simple formatting change adds a nice blank line before each command category. Documentation in README.md is also updated to be more accurate. --- README.md | 4 ++-- help.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 883cc10..34055fe 100644 --- a/README.md +++ b/README.md @@ -761,11 +761,11 @@ func main() { }, { Name: "add", - Category: "template", + Category: "Template actions", }, { Name: "remove", - Category: "template", + Category: "Template actions", }, } diff --git a/help.go b/help.go index 57ec98d..ed084fc 100644 --- a/help.go +++ b/help.go @@ -29,6 +29,7 @@ AUTHOR{{with $length := len .Authors}}{{if ne 1 $length}}S{{end}}{{end}}: {{end}}{{$author}}{{end}}{{end}}{{if .VisibleCommands}} COMMANDS:{{range .VisibleCategories}}{{if .Name}} + {{.Name}}:{{end}}{{range .VisibleCommands}} {{join .Names ", "}}{{"\t"}}{{.Usage}}{{end}}{{end}}{{end}}{{if .VisibleFlags}} From 11d45572f9727acfbc93daa8565f379d396125d6 Mon Sep 17 00:00:00 2001 From: rliebz Date: Sat, 26 Aug 2017 07:42:25 -0400 Subject: [PATCH 21/42] Export funcs to configure flag prefix/env hints This will allow users to customize the prefix section or env hint section of the flag entries in the help menu without having to reimplement the rest of the logic required in defining FlagStringer. --- flag.go | 20 ++++++++++++++------ funcs.go | 8 ++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/flag.go b/flag.go index 877ff35..b17f5b9 100644 --- a/flag.go +++ b/flag.go @@ -37,6 +37,14 @@ var HelpFlag Flag = BoolFlag{ // to display a flag. var FlagStringer FlagStringFunc = stringifyFlag +// FlagNamePrefixer converts a full flag name and its placeholder into the help +// message flag prefix. This is used by the default FlagStringer. +var FlagNamePrefixer FlagNamePrefixFunc = prefixedNames + +// FlagEnvHinter annotates flag help message with the environment variable +// details. This is used by the default FlagStringer. +var FlagEnvHinter FlagEnvHintFunc = withEnvHint + // FlagsByName is a slice of Flag. type FlagsByName []Flag @@ -710,13 +718,13 @@ func stringifyFlag(f Flag) string { switch f.(type) { case IntSliceFlag: - return withEnvHint(fv.FieldByName("EnvVar").String(), + return FlagEnvHinter(fv.FieldByName("EnvVar").String(), stringifyIntSliceFlag(f.(IntSliceFlag))) case Int64SliceFlag: - return withEnvHint(fv.FieldByName("EnvVar").String(), + return FlagEnvHinter(fv.FieldByName("EnvVar").String(), stringifyInt64SliceFlag(f.(Int64SliceFlag))) case StringSliceFlag: - return withEnvHint(fv.FieldByName("EnvVar").String(), + return FlagEnvHinter(fv.FieldByName("EnvVar").String(), stringifyStringSliceFlag(f.(StringSliceFlag))) } @@ -744,8 +752,8 @@ func stringifyFlag(f Flag) string { usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString)) - return withEnvHint(fv.FieldByName("EnvVar").String(), - fmt.Sprintf("%s\t%s", prefixedNames(fv.FieldByName("Name").String(), placeholder), usageWithDefault)) + return FlagEnvHinter(fv.FieldByName("EnvVar").String(), + fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault)) } func stringifyIntSliceFlag(f IntSliceFlag) string { @@ -795,5 +803,5 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { } usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal)) - return fmt.Sprintf("%s\t%s", prefixedNames(name, placeholder), usageWithDefault) + return fmt.Sprintf("%s\t%s", FlagNamePrefixer(name, placeholder), usageWithDefault) } diff --git a/funcs.go b/funcs.go index cba5e6c..3ad3c6d 100644 --- a/funcs.go +++ b/funcs.go @@ -26,3 +26,11 @@ type OnUsageErrorFunc func(context *Context, err error, isSubcommand bool) error // FlagStringFunc is used by the help generation to display a flag, which is // expected to be a single line. type FlagStringFunc func(Flag) string + +// FlagNamePrefixFunc is used by the default FlagStringFunc to create prefix +// text for a flag's full name. +type FlagNamePrefixFunc func(fullName, placeholder string) string + +// FlagEnvHintFunc is used by the default FlagStringFunc to annotate flag help +// with the environment variable details. +type FlagEnvHintFunc func(envVar, str string) string From cbbe4c1a2c34e52c8ad0937c01c9c15ef407a6d5 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Mon, 18 Sep 2017 00:44:42 -0400 Subject: [PATCH 22/42] Add tests for custom flag prefix/env hints --- flag_test.go | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/flag_test.go b/flag_test.go index 1ccb639..bc840b5 100644 --- a/flag_test.go +++ b/flag_test.go @@ -158,6 +158,83 @@ func TestStringFlagWithEnvVarHelpOutput(t *testing.T) { } } +var prefixStringFlagTests = []struct { + name string + usage string + value string + prefixer FlagNamePrefixFunc + expected string +}{ + {"foo", "", "", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: foo, ph: value\t"}, + {"f", "", "", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: f, ph: value\t"}, + {"f", "The total `foo` desired", "all", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: f, ph: foo\tThe total foo desired (default: \"all\")"}, + {"test", "", "Something", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: test, ph: value\t(default: \"Something\")"}, + {"config,c", "Load configuration from `FILE`", "", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: config,c, ph: FILE\tLoad configuration from FILE"}, + {"config,c", "Load configuration from `CONFIG`", "config.json", func(a, b string) string { + return fmt.Sprintf("name: %s, ph: %s", a, b) + }, "name: config,c, ph: CONFIG\tLoad configuration from CONFIG (default: \"config.json\")"}, +} + +func TestFlagNamePrefixer(t *testing.T) { + defer func() { + FlagNamePrefixer = prefixedNames + }() + + for _, test := range prefixStringFlagTests { + FlagNamePrefixer = test.prefixer + flag := StringFlag{Name: test.name, Usage: test.usage, Value: test.value} + output := flag.String() + if output != test.expected { + t.Errorf("%q does not match %q", output, test.expected) + } + } +} + +var envHintFlagTests = []struct { + name string + env string + hinter FlagEnvHintFunc + expected string +}{ + {"foo", "", func(a, b string) string { + return fmt.Sprintf("env: %s, str: %s", a, b) + }, "env: , str: --foo value\t"}, + {"f", "", func(a, b string) string { + return fmt.Sprintf("env: %s, str: %s", a, b) + }, "env: , str: -f value\t"}, + {"foo", "ENV_VAR", func(a, b string) string { + return fmt.Sprintf("env: %s, str: %s", a, b) + }, "env: ENV_VAR, str: --foo value\t"}, + {"f", "ENV_VAR", func(a, b string) string { + return fmt.Sprintf("env: %s, str: %s", a, b) + }, "env: ENV_VAR, str: -f value\t"}, +} + +func TestFlagEnvHinter(t *testing.T) { + defer func() { + FlagEnvHinter = withEnvHint + }() + + for _, test := range envHintFlagTests { + FlagEnvHinter = test.hinter + flag := StringFlag{Name: test.name, EnvVar: test.env} + output := flag.String() + if output != test.expected { + t.Errorf("%q does not match %q", output, test.expected) + } + } +} + var stringSliceFlagTests = []struct { name string value *StringSlice From 67ee172e6da2cdad8e48af107eef0fbfd1e85eec Mon Sep 17 00:00:00 2001 From: Sebastian Sprenger Date: Fri, 6 Oct 2017 07:28:18 +0200 Subject: [PATCH 23/42] fix misspelling issue --- context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context.go b/context.go index db94191..012b9b5 100644 --- a/context.go +++ b/context.go @@ -73,7 +73,7 @@ func (c *Context) IsSet(name string) bool { // change in version 2 to add `IsSet` to the Flag interface to push the // responsibility closer to where the information required to determine // whether a flag is set by non-standard means such as environment - // variables is avaliable. + // variables is available. // // See https://github.com/urfave/cli/issues/294 for additional discussion flags := c.Command.Flags From c3cc74dac756e33c2919ab998481809e8720e068 Mon Sep 17 00:00:00 2001 From: Sebastian Sprenger Date: Fri, 6 Oct 2017 07:28:43 +0200 Subject: [PATCH 24/42] fix ineffective assigns --- app_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/app_test.go b/app_test.go index e14ddaf..54e0951 100644 --- a/app_test.go +++ b/app_test.go @@ -497,7 +497,6 @@ func TestApp_Float64Flag(t *testing.T) { } func TestApp_ParseSliceFlags(t *testing.T) { - var parsedOption, firstArg string var parsedIntSlice []int var parsedStringSlice []string @@ -511,8 +510,6 @@ func TestApp_ParseSliceFlags(t *testing.T) { Action: func(c *Context) error { parsedIntSlice = c.IntSlice("p") parsedStringSlice = c.StringSlice("ip") - parsedOption = c.String("option") - firstArg = c.Args().First() return nil }, } From c202606a17a763fcc1b320cac6cf584662e31364 Mon Sep 17 00:00:00 2001 From: Sebastian Sprenger Date: Fri, 6 Oct 2017 07:29:13 +0200 Subject: [PATCH 25/42] fix golint issues --- altsrc/map_input_source.go | 14 +++++++------- altsrc/toml_file_loader.go | 4 ++-- altsrc/yaml_file_loader.go | 4 ++-- app.go | 4 ++-- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go index b3169e0..a111eee 100644 --- a/altsrc/map_input_source.go +++ b/altsrc/map_input_source.go @@ -22,15 +22,15 @@ func nestedVal(name string, tree map[interface{}]interface{}) (interface{}, bool if sections := strings.Split(name, "."); len(sections) > 1 { node := tree for _, section := range sections[:len(sections)-1] { - if child, ok := node[section]; !ok { + child, ok := node[section] + if !ok { return nil, false - } else { - if ctype, ok := child.(map[interface{}]interface{}); !ok { - return nil, false - } else { - node = ctype - } } + ctype, ok := child.(map[interface{}]interface{}) + if !ok { + return nil, false + } + node = ctype } if val, ok := node[sections[len(sections)-1]]; ok { return val, true diff --git a/altsrc/toml_file_loader.go b/altsrc/toml_file_loader.go index 37870fc..87bb9d9 100644 --- a/altsrc/toml_file_loader.go +++ b/altsrc/toml_file_loader.go @@ -66,9 +66,9 @@ func unmarshalMap(i interface{}) (ret map[interface{}]interface{}, err error) { return ret, nil } -func (self *tomlMap) UnmarshalTOML(i interface{}) error { +func (tm *tomlMap) UnmarshalTOML(i interface{}) error { if tmp, err := unmarshalMap(i); err == nil { - self.Map = tmp + tm.Map = tmp } else { return err } diff --git a/altsrc/yaml_file_loader.go b/altsrc/yaml_file_loader.go index dd808d5..202469f 100644 --- a/altsrc/yaml_file_loader.go +++ b/altsrc/yaml_file_loader.go @@ -86,7 +86,7 @@ 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 { - return nil, fmt.Errorf("unable to determine how to load from path %s", filePath) } + + return nil, fmt.Errorf("unable to determine how to load from path %s", filePath) } diff --git a/app.go b/app.go index 51fc45d..5991226 100644 --- a/app.go +++ b/app.go @@ -491,7 +491,7 @@ func HandleAction(action interface{}, context *Context) (err error) { } else if a, ok := action.(func(*Context)); ok { // deprecated function signature a(context) return nil - } else { - return errInvalidActionType } + + return errInvalidActionType } From b44660ac3da2f8e651372c40ae803782bddea283 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Sat, 28 Oct 2017 03:00:11 -0400 Subject: [PATCH 26/42] Consider case when sorting strings This makes sorting flags and other sections consistent with how most command line tools function, by placing both flags `-A` and `-a` before a flag `-B`. --- category.go | 2 +- command.go | 2 +- flag.go | 2 +- sort.go | 29 +++++++++++++++++++++++++++++ sort_test.go | 30 ++++++++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 sort.go create mode 100644 sort_test.go diff --git a/category.go b/category.go index 1a60550..bf3c73c 100644 --- a/category.go +++ b/category.go @@ -10,7 +10,7 @@ type CommandCategory struct { } func (c CommandCategories) Less(i, j int) bool { - return c[i].Name < c[j].Name + return lexicographicLess(c[i].Name, c[j].Name) } func (c CommandCategories) Len() int { diff --git a/command.go b/command.go index 502fc9f..be8f8f0 100644 --- a/command.go +++ b/command.go @@ -73,7 +73,7 @@ func (c CommandsByName) Len() int { } func (c CommandsByName) Less(i, j int) bool { - return c[i].Name < c[j].Name + return lexicographicLess(c[i].Name, c[j].Name) } func (c CommandsByName) Swap(i, j int) { diff --git a/flag.go b/flag.go index b17f5b9..53fb8bb 100644 --- a/flag.go +++ b/flag.go @@ -53,7 +53,7 @@ func (f FlagsByName) Len() int { } func (f FlagsByName) Less(i, j int) bool { - return f[i].GetName() < f[j].GetName() + return lexicographicLess(f[i].GetName(), f[j].GetName()) } func (f FlagsByName) Swap(i, j int) { diff --git a/sort.go b/sort.go new file mode 100644 index 0000000..23d1c2f --- /dev/null +++ b/sort.go @@ -0,0 +1,29 @@ +package cli + +import "unicode" + +// lexicographicLess compares strings alphabetically considering case. +func lexicographicLess(i, j string) bool { + iRunes := []rune(i) + jRunes := []rune(j) + + lenShared := len(iRunes) + if lenShared > len(jRunes) { + lenShared = len(jRunes) + } + + for index := 0; index < lenShared; index++ { + ir := iRunes[index] + jr := jRunes[index] + + if lir, ljr := unicode.ToLower(ir), unicode.ToLower(jr); lir != ljr { + return lir < ljr + } + + if ir != jr { + return ir < jr + } + } + + return i < j +} diff --git a/sort_test.go b/sort_test.go new file mode 100644 index 0000000..662ef9b --- /dev/null +++ b/sort_test.go @@ -0,0 +1,30 @@ +package cli + +import "testing" + +var lexicographicLessTests = []struct { + i string + j string + expected bool +}{ + {"", "a", true}, + {"a", "", false}, + {"a", "a", false}, + {"a", "A", false}, + {"A", "a", true}, + {"aa", "a", false}, + {"a", "aa", true}, + {"a", "b", true}, + {"a", "B", true}, + {"A", "b", true}, + {"A", "B", true}, +} + +func TestLexicographicLess(t *testing.T) { + for _, test := range lexicographicLessTests { + actual := lexicographicLess(test.i, test.j) + if test.expected != actual { + t.Errorf(`expected string "%s" to come before "%s"`, test.i, test.j) + } + } +} From 21fcab0dee7dab6969e929cf1740306bae1e16ad Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Fri, 31 Mar 2017 16:24:15 +0900 Subject: [PATCH 27/42] ability to load variable from file --- context.go | 29 +++-- flag.go | 251 ++++++++++++++++---------------------------- flag_generated.go | 53 ++++++---- flag_test.go | 20 ++++ generate-flag-types | 1 + 5 files changed, 166 insertions(+), 188 deletions(-) diff --git a/context.go b/context.go index 012b9b5..552ee74 100644 --- a/context.go +++ b/context.go @@ -3,6 +3,7 @@ package cli import ( "errors" "flag" + "os" "reflect" "strings" "syscall" @@ -93,18 +94,26 @@ func (c *Context) IsSet(name string) bool { val = val.Elem() } - envVarValue := val.FieldByName("EnvVar") - if !envVarValue.IsValid() { - return + filePathValue := val.FieldByName("FilePath") + if filePathValue.IsValid() { + eachName(filePathValue.String(), func(filePath string) { + if _, err := os.Stat(filePath); err == nil { + c.setFlags[name] = true + return + } + }) } - eachName(envVarValue.String(), func(envVar string) { - envVar = strings.TrimSpace(envVar) - if _, ok := syscall.Getenv(envVar); ok { - c.setFlags[name] = true - return - } - }) + envVarValue := val.FieldByName("EnvVar") + if envVarValue.IsValid() { + eachName(envVarValue.String(), func(envVar string) { + envVar = strings.TrimSpace(envVar) + if _, ok := syscall.Getenv(envVar); ok { + c.setFlags[name] = true + return + } + }) + } }) } } diff --git a/flag.go b/flag.go index 53fb8bb..4042093 100644 --- a/flag.go +++ b/flag.go @@ -3,6 +3,7 @@ package cli import ( "flag" "fmt" + "io/ioutil" "reflect" "runtime" "strconv" @@ -120,15 +121,9 @@ func (f GenericFlag) Apply(set *flag.FlagSet) { // 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 { - 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 - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + if err := val.Set(envVal); err != nil { + return fmt.Errorf("could not parse %s as value for flag %s: %s", envVal, f.Name, err) } } @@ -171,21 +166,15 @@ func (f StringSliceFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f StringSliceFlag) ApplyWithError(set *flag.FlagSet) error { - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - newVal := &StringSlice{} - for _, s := range strings.Split(envVal, ",") { - s = strings.TrimSpace(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 + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + newVal := &StringSlice{} + for _, s := range strings.Split(envVal, ",") { + s = strings.TrimSpace(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 } eachName(f.Name, func(name string) { @@ -234,21 +223,15 @@ func (f IntSliceFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f IntSliceFlag) ApplyWithError(set *flag.FlagSet) error { - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - newVal := &IntSlice{} - for _, s := range strings.Split(envVal, ",") { - s = strings.TrimSpace(s) - 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 - break + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + newVal := &IntSlice{} + for _, s := range strings.Split(envVal, ",") { + s = strings.TrimSpace(s) + 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 } eachName(f.Name, func(name string) { @@ -297,21 +280,15 @@ func (f Int64SliceFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error { - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - newVal := &Int64Slice{} - for _, s := range strings.Split(envVal, ",") { - s = strings.TrimSpace(s) - 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 - break + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + newVal := &Int64Slice{} + for _, s := range strings.Split(envVal, ",") { + s = strings.TrimSpace(s) + 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 } eachName(f.Name, func(name string) { @@ -332,23 +309,15 @@ func (f BoolFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f BoolFlag) ApplyWithError(set *flag.FlagSet) error { val := false - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - if envVal == "" { - val = false - break - } - - envValBool, err := strconv.ParseBool(envVal) - if err != nil { - return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) - } - - val = envValBool - break + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + if envVal == "" { + val = false + } else { + envValBool, err := strconv.ParseBool(envVal) + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) } + val = envValBool } } @@ -372,23 +341,16 @@ func (f BoolTFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f BoolTFlag) ApplyWithError(set *flag.FlagSet) error { val := true - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - if envVal == "" { - val = false - break - } - envValBool, err := strconv.ParseBool(envVal) - if err != nil { - return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) - } - - val = envValBool - break + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + if envVal == "" { + val = false + } else { + envValBool, err := strconv.ParseBool(envVal) + if err != nil { + return fmt.Errorf("could not parse %s as bool value for flag %s: %s", envVal, f.Name, err) } + val = envValBool } } @@ -411,14 +373,8 @@ func (f StringFlag) Apply(set *flag.FlagSet) { // ApplyWithError populates the flag given the flag set and environment func (f StringFlag) ApplyWithError(set *flag.FlagSet) error { - if f.EnvVar != "" { - for _, envVar := range strings.Split(f.EnvVar, ",") { - envVar = strings.TrimSpace(envVar) - if envVal, ok := syscall.Getenv(envVar); ok { - f.Value = envVal - break - } - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + f.Value = envVal } eachName(f.Name, func(name string) { @@ -440,18 +396,12 @@ func (f IntFlag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as int value for flag %s: %s", envVal, f.Name, err) - } - f.Value = int(envValInt) - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValInt, err := strconv.ParseInt(envVal, 0, 64) + 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) } eachName(f.Name, func(name string) { @@ -473,19 +423,13 @@ func (f Int64Flag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as int value for flag %s: %s", envVal, f.Name, err) - } - - f.Value = envValInt - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValInt, err := strconv.ParseInt(envVal, 0, 64) + if err != nil { + return fmt.Errorf("could not parse %s as int value for flag %s: %s", envVal, f.Name, err) } + + f.Value = envValInt } eachName(f.Name, func(name string) { @@ -507,19 +451,13 @@ func (f UintFlag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as uint value for flag %s: %s", envVal, f.Name, err) - } - - f.Value = uint(envValInt) - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValInt, err := strconv.ParseUint(envVal, 0, 64) + 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) } eachName(f.Name, func(name string) { @@ -541,19 +479,13 @@ func (f Uint64Flag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as uint64 value for flag %s: %s", envVal, f.Name, err) - } - - f.Value = uint64(envValInt) - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValInt, err := strconv.ParseUint(envVal, 0, 64) + 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) } eachName(f.Name, func(name string) { @@ -575,19 +507,13 @@ func (f DurationFlag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as duration for flag %s: %s", envVal, f.Name, err) - } - - f.Value = envValDuration - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValDuration, err := time.ParseDuration(envVal) + if err != nil { + return fmt.Errorf("could not parse %s as duration for flag %s: %s", envVal, f.Name, err) } + + f.Value = envValDuration } eachName(f.Name, func(name string) { @@ -609,19 +535,13 @@ func (f Float64Flag) Apply(set *flag.FlagSet) { // 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 { - return fmt.Errorf("could not parse %s as float64 value for flag %s: %s", envVal, f.Name, err) - } - - f.Value = float64(envValFloat) - break - } + if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + envValFloat, err := strconv.ParseFloat(envVal, 10) + 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) } eachName(f.Name, func(name string) { @@ -805,3 +725,18 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal)) return fmt.Sprintf("%s\t%s", FlagNamePrefixer(name, placeholder), usageWithDefault) } + +func flagFromFileEnv(filePath, envName string) (val string, ok bool) { + for _, envVar := range strings.Split(envName, ",") { + envVar = strings.TrimSpace(envVar) + if envVal, ok := syscall.Getenv(envVar); ok { + return envVal, true + } + } + if filePath != "" { + if data, err := ioutil.ReadFile(filePath); err == nil { + return string(data), true + } + } + return +} diff --git a/flag_generated.go b/flag_generated.go index 491b619..001576c 100644 --- a/flag_generated.go +++ b/flag_generated.go @@ -13,6 +13,7 @@ type BoolFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Destination *bool } @@ -60,6 +61,7 @@ type BoolTFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Destination *bool } @@ -107,6 +109,7 @@ type DurationFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value time.Duration Destination *time.Duration @@ -155,6 +158,7 @@ type Float64Flag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value float64 Destination *float64 @@ -200,11 +204,12 @@ func lookupFloat64(name string, set *flag.FlagSet) float64 { // GenericFlag is a flag with type Generic type GenericFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value Generic + Name string + Usage string + EnvVar string + FilePath string + Hidden bool + Value Generic } // String returns a readable representation of this value @@ -250,6 +255,7 @@ type Int64Flag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value int64 Destination *int64 @@ -298,6 +304,7 @@ type IntFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value int Destination *int @@ -343,11 +350,12 @@ func lookupInt(name string, set *flag.FlagSet) int { // IntSliceFlag is a flag with type *IntSlice type IntSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *IntSlice + Name string + Usage string + EnvVar string + FilePath string + Hidden bool + Value *IntSlice } // String returns a readable representation of this value @@ -390,11 +398,12 @@ func lookupIntSlice(name string, set *flag.FlagSet) []int { // Int64SliceFlag is a flag with type *Int64Slice type Int64SliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *Int64Slice + Name string + Usage string + EnvVar string + FilePath string + Hidden bool + Value *Int64Slice } // String returns a readable representation of this value @@ -440,6 +449,7 @@ type StringFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value string Destination *string @@ -485,11 +495,12 @@ func lookupString(name string, set *flag.FlagSet) string { // StringSliceFlag is a flag with type *StringSlice type StringSliceFlag struct { - Name string - Usage string - EnvVar string - Hidden bool - Value *StringSlice + Name string + Usage string + EnvVar string + FilePath string + Hidden bool + Value *StringSlice } // String returns a readable representation of this value @@ -535,6 +546,7 @@ type Uint64Flag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value uint64 Destination *uint64 @@ -583,6 +595,7 @@ type UintFlag struct { Name string Usage string EnvVar string + FilePath string Hidden bool Value uint Destination *uint diff --git a/flag_test.go b/flag_test.go index bc840b5..66836f3 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1290,3 +1290,23 @@ func TestParseGenericFromEnvCascade(t *testing.T) { } a.Run([]string{"run"}) } + +// func TestFlagFromFile(t *testing.T) { +// temp, err := ioutil.TempFile("", "urfave_cli_test") +// if err != nil { +// t.Error(err) +// return +// } +// io.WriteString(temp, "abc") +// temp.Close() +// defer func() { +// os.Remove(temp.Name()) +// }() +// +// if want, got := flagFromFileEnv("file-does-not-exist", "123"), "123"; want != got { +// t.Errorf("Did not expect %v - Got %v", want, got) +// } +// if want, got := flagFromFile(temp.Name(), "123"), "abc"; want != got { +// t.Errorf("Did not expect %v - Got %v", want, got) +// } +// } diff --git a/generate-flag-types b/generate-flag-types index 7147381..1358857 100755 --- a/generate-flag-types +++ b/generate-flag-types @@ -142,6 +142,7 @@ def _write_cli_flag_types(outfile, types): Name string Usage string EnvVar string + FilePath string Hidden bool """.format(**typedef)) From c698b821b896e9723d53c4ad1e81680f39a8cdc1 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Sat, 1 Apr 2017 12:37:06 +0900 Subject: [PATCH 28/42] unit tests for load from file --- flag_test.go | 55 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/flag_test.go b/flag_test.go index 66836f3..98c2fb9 100644 --- a/flag_test.go +++ b/flag_test.go @@ -2,6 +2,8 @@ package cli import ( "fmt" + "io" + "io/ioutil" "os" "reflect" "regexp" @@ -1291,22 +1293,37 @@ func TestParseGenericFromEnvCascade(t *testing.T) { a.Run([]string{"run"}) } -// func TestFlagFromFile(t *testing.T) { -// temp, err := ioutil.TempFile("", "urfave_cli_test") -// if err != nil { -// t.Error(err) -// return -// } -// io.WriteString(temp, "abc") -// temp.Close() -// defer func() { -// os.Remove(temp.Name()) -// }() -// -// if want, got := flagFromFileEnv("file-does-not-exist", "123"), "123"; want != got { -// t.Errorf("Did not expect %v - Got %v", want, got) -// } -// if want, got := flagFromFile(temp.Name(), "123"), "abc"; want != got { -// t.Errorf("Did not expect %v - Got %v", want, got) -// } -// } +func TestFlagFromFile(t *testing.T) { + os.Clearenv() + os.Setenv("APP_FOO", "123") + + temp, err := ioutil.TempFile("", "urfave_cli_test") + if err != nil { + t.Error(err) + return + } + io.WriteString(temp, "abc") + temp.Close() + defer func() { + os.Remove(temp.Name()) + }() + + var filePathTests = []struct { + path string + name string + expected string + }{ + {"file-does-not-exist", "APP_BAR", ""}, + {"file-does-not-exist", "APP_FOO", "123"}, + {"file-does-not-exist", "APP_FOO,APP_BAR", "123"}, + {temp.Name(), "APP_FOO", "123"}, + {temp.Name(), "APP_BAR", "abc"}, + } + + for _, filePathTest := range filePathTests { + got, _ := flagFromFileEnv(filePathTest.path, filePathTest.name) + if want := filePathTest.expected; got != want { + t.Errorf("Did not expect %v - Want %v", got, want) + } + } +} From 4cc453ba6792515a8013340f8919e6c4b44851b7 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Sat, 1 Apr 2017 12:55:46 +0900 Subject: [PATCH 29/42] document field in README --- README.md | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/README.md b/README.md index 34055fe..aa8f2cb 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ applications in an expressive way. + [Alternate Names](#alternate-names) + [Ordering](#ordering) + [Values from the Environment](#values-from-the-environment) + + [Values from files](#values-from-files) + [Values from alternate input sources (YAML, TOML, and others)](#values-from-alternate-input-sources-yaml-toml-and-others) + [Precedence](#precedence) * [Subcommands](#subcommands) @@ -587,6 +588,38 @@ func main() { } ``` +#### Values from Files + +You can also have the default value set from file via `FilePath`. e.g. + + +``` go +package main + +import ( + "os" + + "github.com/urfave/cli" +) + +func main() { + app := cli.NewApp() + + app.Flags = []cli.Flag { + cli.StringFlag{ + Name: "password, p", + Usage: "password for the mysql database", + FilePath: "/etc/mysql/password", + }, + } + + app.Run(os.Args) +} +``` + #### Values from alternate input sources (YAML, TOML, and others) There is a separate package altsrc that adds support for getting flag values From 18a556e1927fbe11c31fae47a7e3acf275ef6ae4 Mon Sep 17 00:00:00 2001 From: Brad Rydzewski Date: Mon, 10 Apr 2017 16:45:51 +0200 Subject: [PATCH 30/42] fix FilePath documentation in README.md --- README.md | 7 ++++-- flag.go | 62 +++++++++++++++++++++++++++++++++++++++++----------- flag_test.go | 2 +- funcs.go | 3 +++ 4 files changed, 58 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index aa8f2cb..a2fd41d 100644 --- a/README.md +++ b/README.md @@ -588,13 +588,13 @@ func main() { } ``` -#### Values from Files +#### Values from files You can also have the default value set from file via `FilePath`. e.g. ``` go package main @@ -620,6 +620,9 @@ func main() { } ``` +Note that default values set from file (e.g. `FilePath`) take precedence over +default values set from the enviornment (e.g. `EnvVar`). + #### Values from alternate input sources (YAML, TOML, and others) There is a separate package altsrc that adds support for getting flag values diff --git a/flag.go b/flag.go index 4042093..68942ae 100644 --- a/flag.go +++ b/flag.go @@ -46,6 +46,10 @@ var FlagNamePrefixer FlagNamePrefixFunc = prefixedNames // details. This is used by the default FlagStringer. var FlagEnvHinter FlagEnvHintFunc = withEnvHint +// FlagFileHinter annotates flag help message with the environment variable +// details. This is used by the default FlagStringer. +var FlagFileHinter FlagFileHintFunc = withFileHint + // FlagsByName is a slice of Flag. type FlagsByName []Flag @@ -625,6 +629,14 @@ func withEnvHint(envVar, str string) string { return str + envText } +func withFileHint(filePath, str string) string { + fileText := "" + if filePath != "" { + fileText = fmt.Sprintf(" [%s]", filePath) + } + return str + fileText +} + func flagValue(f Flag) reflect.Value { fv := reflect.ValueOf(f) for fv.Kind() == reflect.Ptr { @@ -638,14 +650,33 @@ func stringifyFlag(f Flag) string { switch f.(type) { case IntSliceFlag: - return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - stringifyIntSliceFlag(f.(IntSliceFlag))) + return FlagFileHinter( + fv.FieldByName("FilePath").String(), + FlagEnvHinter( + fv.FieldByName("EnvVar").String(), + stringifyIntSliceFlag(f.(IntSliceFlag)), + ), + ) case Int64SliceFlag: - return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - stringifyInt64SliceFlag(f.(Int64SliceFlag))) + // return FlagEnvHinter(fv.FieldByName("EnvVar").String(), + // stringifyInt64SliceFlag(f.(Int64SliceFlag))) + return FlagFileHinter( + fv.FieldByName("FilePath").String(), + FlagEnvHinter( + fv.FieldByName("EnvVar").String(), + stringifyInt64SliceFlag(f.(Int64SliceFlag)), + ), + ) case StringSliceFlag: - return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - stringifyStringSliceFlag(f.(StringSliceFlag))) + // return FlagEnvHinter(fv.FieldByName("EnvVar").String(), + // stringifyStringSliceFlag(f.(StringSliceFlag))) + return FlagFileHinter( + fv.FieldByName("FilePath").String(), + FlagEnvHinter( + fv.FieldByName("EnvVar").String(), + stringifyStringSliceFlag(f.(StringSliceFlag)), + ), + ) } placeholder, usage := unquoteUsage(fv.FieldByName("Usage").String()) @@ -672,8 +703,13 @@ func stringifyFlag(f Flag) string { usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString)) - return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault)) + return FlagFileHinter( + fv.FieldByName("FilePath").String(), + FlagEnvHinter( + fv.FieldByName("EnvVar").String(), + fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault), + ), + ) } func stringifyIntSliceFlag(f IntSliceFlag) string { @@ -727,16 +763,16 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { } func flagFromFileEnv(filePath, envName string) (val string, ok bool) { + if filePath != "" { + if data, err := ioutil.ReadFile(filePath); err == nil { + return string(data), true + } + } for _, envVar := range strings.Split(envName, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { return envVal, true } } - if filePath != "" { - if data, err := ioutil.ReadFile(filePath); err == nil { - return string(data), true - } - } return } diff --git a/flag_test.go b/flag_test.go index 98c2fb9..fb9163b 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1316,7 +1316,7 @@ func TestFlagFromFile(t *testing.T) { {"file-does-not-exist", "APP_BAR", ""}, {"file-does-not-exist", "APP_FOO", "123"}, {"file-does-not-exist", "APP_FOO,APP_BAR", "123"}, - {temp.Name(), "APP_FOO", "123"}, + {temp.Name(), "APP_FOO", "abc"}, {temp.Name(), "APP_BAR", "abc"}, } diff --git a/funcs.go b/funcs.go index b335dbf..0036b11 100644 --- a/funcs.go +++ b/funcs.go @@ -39,3 +39,6 @@ type FlagNamePrefixFunc func(fullName, placeholder string) string // with the environment variable details. type FlagEnvHintFunc func(envVar, str string) string +// FlagFileHintFunc is used by the default FlagStringFunc to annotate flag help +// with the file path details. +type FlagFileHintFunc func(filePath, str string) string From f971fca2b2664c4dec0cee24225dc3c415211498 Mon Sep 17 00:00:00 2001 From: Jacob McCann Date: Thu, 26 Oct 2017 13:08:03 -0500 Subject: [PATCH 31/42] Allow FilePath to take []string --- flag.go | 22 +++++++++------------- flag_test.go | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/flag.go b/flag.go index 68942ae..f8d4f51 100644 --- a/flag.go +++ b/flag.go @@ -125,9 +125,9 @@ func (f GenericFlag) Apply(set *flag.FlagSet) { // provided by the user for parsing by the flag func (f GenericFlag) ApplyWithError(set *flag.FlagSet) error { val := f.Value - if envVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { - if err := val.Set(envVal); err != nil { - return fmt.Errorf("could not parse %s as value for flag %s: %s", envVal, f.Name, err) + if fileEnvVal, ok := flagFromFileEnv(f.FilePath, f.EnvVar); ok { + if err := val.Set(fileEnvVal); err != nil { + return fmt.Errorf("could not parse %s as value for flag %s: %s", fileEnvVal, f.Name, err) } } @@ -658,8 +658,6 @@ func stringifyFlag(f Flag) string { ), ) case Int64SliceFlag: - // return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - // stringifyInt64SliceFlag(f.(Int64SliceFlag))) return FlagFileHinter( fv.FieldByName("FilePath").String(), FlagEnvHinter( @@ -668,8 +666,6 @@ func stringifyFlag(f Flag) string { ), ) case StringSliceFlag: - // return FlagEnvHinter(fv.FieldByName("EnvVar").String(), - // stringifyStringSliceFlag(f.(StringSliceFlag))) return FlagFileHinter( fv.FieldByName("FilePath").String(), FlagEnvHinter( @@ -763,16 +759,16 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { } func flagFromFileEnv(filePath, envName string) (val string, ok bool) { - if filePath != "" { - if data, err := ioutil.ReadFile(filePath); err == nil { - return string(data), true - } - } for _, envVar := range strings.Split(envName, ",") { envVar = strings.TrimSpace(envVar) if envVal, ok := syscall.Getenv(envVar); ok { return envVal, true } } - return + for _, fileVar := range strings.Split(filePath, ",") { + if data, err := ioutil.ReadFile(fileVar); err == nil { + return string(data), true + } + } + return "", false } diff --git a/flag_test.go b/flag_test.go index fb9163b..98c2fb9 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1316,7 +1316,7 @@ func TestFlagFromFile(t *testing.T) { {"file-does-not-exist", "APP_BAR", ""}, {"file-does-not-exist", "APP_FOO", "123"}, {"file-does-not-exist", "APP_FOO,APP_BAR", "123"}, - {temp.Name(), "APP_FOO", "abc"}, + {temp.Name(), "APP_FOO", "123"}, {temp.Name(), "APP_BAR", "abc"}, } From 43c8c02cf5a10196e5a4c458fdbfee90a561e97c Mon Sep 17 00:00:00 2001 From: zhuchensong Date: Mon, 17 Apr 2017 00:47:04 +0800 Subject: [PATCH 32/42] Support POSIX-style short flag combining --- command.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index be8f8f0..189209d 100644 --- a/command.go +++ b/command.go @@ -142,7 +142,19 @@ func (c Command) Run(ctx *Context) (err error) { flagArgs = args[firstFlagIndex:] } - err = set.Parse(append(flagArgs, regularArgs...)) + // separate combined flags + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) >2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-" + string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) + } + } + + err = set.Parse(append(flagArgsSeparated, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } From fd5382e7a539858cc19d7eed7755f7102bae5da9 Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 13 Nov 2017 15:28:23 -0600 Subject: [PATCH 33/42] Combine bool short names Adds the ability to allow the combination of bool short-name options. For example, cmd foobar -ov This is done through a bool "UseShortOptionHandler" set in the command struct. Built upon PR #621 Signed-off-by: baude --- README.md | 21 +++++++++++++++++++++ command.go | 26 ++++++++++++++++---------- command_test.go | 36 ++++++++++++++++++++---------------- flag_test.go | 25 +++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index a2fd41d..6096701 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ applications in an expressive way. * [Version Flag](#version-flag) + [Customization](#customization-2) + [Full API Example](#full-api-example) + * [Combining short Bool options](#combining-short-bool-options) - [Contribution Guidelines](#contribution-guidelines) @@ -1410,6 +1411,26 @@ func wopAction(c *cli.Context) error { } ``` +### Combining short Bool options + +Traditional use of boolean options using their shortnames look like this: +``` +# cmd foobar -s -o +``` + +Suppose you want users to be able to combine your bool options with their shortname. This +can be done using the **UseShortOptionHandling** bool in your commands. Suppose your program +has a two bool flags such as *serve* and *option* with the short options of *-o* and +*-s* respectively. With **UseShortOptionHandling** set to *true*, a user can use a syntax +like: +``` +# cmd foobar -so +``` + +If you enable the **UseShortOptionHandling*, then you must not use any flags that have a single +leading *-* or this will result in failures. For example, **-option** can no longer be used. Flags +with two leading dashes (such as **--options**) are still valid. + ## Contribution Guidelines Feel free to put up a pull request to fix a bug or maybe add a feature. I will diff --git a/command.go b/command.go index 189209d..b559811 100644 --- a/command.go +++ b/command.go @@ -55,6 +55,10 @@ type Command struct { HideHelp bool // Boolean to hide this command from help or completion Hidden bool + // Boolean to enable short-option handling so user can combine several + // single-character bool arguements into one + // i.e. foobar -o -v -> foobar -ov + UseShortOptionHandling bool // Full name of command for help, defaults to full command name, including parent commands. HelpName string @@ -141,20 +145,22 @@ func (c Command) Run(ctx *Context) (err error) { } else { flagArgs = args[firstFlagIndex:] } - // separate combined flags - var flagArgsSeparated []string - for _, flagArg := range flagArgs { - if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) >2 { - for _, flagChar := range flagArg[1:] { - flagArgsSeparated = append(flagArgsSeparated, "-" + string(flagChar)) + if c.UseShortOptionHandling { + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) } - } else { - flagArgsSeparated = append(flagArgsSeparated, flagArg) } + err = set.Parse(append(flagArgsSeparated, regularArgs...)) + } else { + err = set.Parse(append(flagArgs, regularArgs...)) } - - err = set.Parse(append(flagArgsSeparated, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } diff --git a/command_test.go b/command_test.go index 4ad994c..d9d7094 100644 --- a/command_test.go +++ b/command_test.go @@ -11,20 +11,23 @@ import ( func TestCommandFlagParsing(t *testing.T) { cases := []struct { - testArgs []string - skipFlagParsing bool - skipArgReorder bool - expectedErr error + testArgs []string + skipFlagParsing bool + skipArgReorder bool + expectedErr error + UseShortOptionHandling bool }{ // Test normal "not ignoring flags" flow - {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break")}, + {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break"), false}, // Test no arg reorder - {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil}, + {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false}, + + {[]string{"test-cmd", "blah", "blah"}, true, false, nil, false}, // Test SkipFlagParsing without any args that look like flags + {[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg + {[]string{"test-cmd", "blah", "-help"}, true, false, nil, false}, // Test SkipFlagParsing with "special" help flag arg + {[]string{"test-cmd", "blah"}, false, false, nil, true}, // Test UseShortOptionHandling - {[]string{"test-cmd", "blah", "blah"}, true, false, nil}, // Test SkipFlagParsing without any args that look like flags - {[]string{"test-cmd", "blah", "-break"}, true, false, nil}, // Test SkipFlagParsing with random flag arg - {[]string{"test-cmd", "blah", "-help"}, true, false, nil}, // Test SkipFlagParsing with "special" help flag arg } for _, c := range cases { @@ -36,13 +39,14 @@ func TestCommandFlagParsing(t *testing.T) { context := NewContext(app, set, nil) command := Command{ - Name: "test-cmd", - Aliases: []string{"tc"}, - Usage: "this is for testing", - Description: "testing", - Action: func(_ *Context) error { return nil }, - SkipFlagParsing: c.skipFlagParsing, - SkipArgReorder: c.skipArgReorder, + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(_ *Context) error { return nil }, + SkipFlagParsing: c.skipFlagParsing, + SkipArgReorder: c.skipArgReorder, + UseShortOptionHandling: c.UseShortOptionHandling, } err := command.Run(context) diff --git a/flag_test.go b/flag_test.go index 98c2fb9..da9fd73 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1048,6 +1048,31 @@ func TestParseMultiBool(t *testing.T) { a.Run([]string{"run", "--serve"}) } +func TestParseBoolShortOptionHandle(t *testing.T) { + a := App{ + Commands: []Command{ + { + Name: "foobar", + UseShortOptionHandling: true, + Action: func(ctx *Context) error { + if ctx.Bool("serve") != true { + t.Errorf("main name not set") + } + if ctx.Bool("option") != true { + t.Errorf("short name not set") + } + return nil + }, + Flags: []Flag{ + BoolFlag{Name: "serve, s"}, + BoolFlag{Name: "option, o"}, + }, + }, + }, + } + a.Run([]string{"run", "foobar", "-so"}) +} + func TestParseDestinationBool(t *testing.T) { var dest bool a := App{ From 37b7abb1c491c8c3630a2a98bb02a7051efbcc06 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Tue, 21 Nov 2017 15:21:31 -0700 Subject: [PATCH 34/42] dont clobber slices with envvar Signed-off-by: Joshua Rubin --- flag.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/flag.go b/flag.go index f8d4f51..d4a4d41 100644 --- a/flag.go +++ b/flag.go @@ -178,7 +178,11 @@ func (f StringSliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as string value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { @@ -235,7 +239,11 @@ func (f IntSliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as int slice value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { @@ -292,7 +300,11 @@ func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as int64 slice value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { From ceaac7c9152121e6ba0f3b492b3254d61346f92a Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 20 Nov 2017 09:32:03 -0600 Subject: [PATCH 35/42] Handle ShortOptions and SkipArgReorder There was a bug in parsing when both ShortOptions and SkipArgReorder were being used together. Signed-off-by: baude --- command.go | 129 ++++++++++++++++++++++++++++++++---------------- command_test.go | 1 + 2 files changed, 87 insertions(+), 43 deletions(-) diff --git a/command.go b/command.go index b559811..7d0357b 100644 --- a/command.go +++ b/command.go @@ -115,57 +115,29 @@ func (c Command) Run(ctx *Context) (err error) { return err } set.SetOutput(ioutil.Discard) - + firstFlagIndex, terminatorIndex := getIndexes(ctx) + flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex) + if c.UseShortOptionHandling { + flagArgs = translateShortOptions(flagArgs) + } if c.SkipFlagParsing { err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...)) } else if !c.SkipArgReorder { - firstFlagIndex := -1 - terminatorIndex := -1 - for index, arg := range ctx.Args() { - if arg == "--" { - terminatorIndex = index - break - } else if arg == "-" { - // Do nothing. A dash alone is not really a flag. - continue - } else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 { - firstFlagIndex = index - } - } - if firstFlagIndex > -1 { - args := ctx.Args() - regularArgs := make([]string, len(args[1:firstFlagIndex])) - copy(regularArgs, args[1:firstFlagIndex]) - - var flagArgs []string - if terminatorIndex > -1 { - flagArgs = args[firstFlagIndex:terminatorIndex] - regularArgs = append(regularArgs, args[terminatorIndex:]...) - } else { - flagArgs = args[firstFlagIndex:] - } - // separate combined flags - if c.UseShortOptionHandling { - var flagArgsSeparated []string - for _, flagArg := range flagArgs { - if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { - for _, flagChar := range flagArg[1:] { - flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) - } - } else { - flagArgsSeparated = append(flagArgsSeparated, flagArg) - } - } - err = set.Parse(append(flagArgsSeparated, regularArgs...)) - } else { - err = set.Parse(append(flagArgs, regularArgs...)) - } + err = set.Parse(append(flagArgs, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } + } else if c.UseShortOptionHandling { + if terminatorIndex == -1 && firstFlagIndex > -1 { + // Handle shortname AND no options + err = set.Parse(append(regularArgs, flagArgs...)) + } else { + // Handle shortname and options + err = set.Parse(flagArgs) + } } else { - err = set.Parse(ctx.Args().Tail()) + err = set.Parse(append(regularArgs, flagArgs...)) } nerr := normalizeFlags(c.Flags, set) @@ -233,6 +205,77 @@ func (c Command) Run(ctx *Context) (err error) { return err } +func getIndexes(ctx *Context) (int, int) { + firstFlagIndex := -1 + terminatorIndex := -1 + for index, arg := range ctx.Args() { + if arg == "--" { + terminatorIndex = index + break + } else if arg == "-" { + // Do nothing. A dash alone is not really a flag. + continue + } else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 { + firstFlagIndex = index + } + } + if len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 { + return -1, -1 + } + + return firstFlagIndex, terminatorIndex + +} + +// copyStringslice takes a string slice and copies it +func copyStringSlice(slice []string, start, end int) []string { + newSlice := make([]string, end-start) + copy(newSlice, slice[start:end]) + return newSlice +} + +// getAllArgs extracts and returns two string slices representing +// regularArgs and flagArgs +func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) { + var regularArgs []string + // if there are no options, the we set the index to 1 manually + if firstFlagIndex == -1 { + firstFlagIndex = 1 + regularArgs = copyStringSlice(args, 0, len(args)) + } else { + regularArgs = copyStringSlice(args, 1, firstFlagIndex) + } + var flagArgs []string + // a flag terminatorIndex was found in the input. we need to collect + // flagArgs based on it. + if terminatorIndex > -1 { + flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex) + additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args)) + regularArgs = append(regularArgs, additionalRegularArgs...) + for _, i := range additionalRegularArgs { + regularArgs = append(regularArgs, i) + } + } else { + flagArgs = args[firstFlagIndex:] + } + return flagArgs, regularArgs +} + +func translateShortOptions(flagArgs Args) []string { + // separate combined flags + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) + } + } + return flagArgsSeparated +} + // Names returns the names including short names and aliases. func (c Command) Names() []string { names := []string{c.Name} diff --git a/command_test.go b/command_test.go index d9d7094..e69750a 100644 --- a/command_test.go +++ b/command_test.go @@ -22,6 +22,7 @@ func TestCommandFlagParsing(t *testing.T) { // Test no arg reorder {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false}, + {[]string{"test-cmd", "blah", "blah", "-break", "ls", "-l"}, false, true, nil, true}, {[]string{"test-cmd", "blah", "blah"}, true, false, nil, false}, // Test SkipFlagParsing without any args that look like flags {[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg From c6eb2a051026c083d4e33591f8d6e95d5f4189dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lipt=C3=A1k?= Date: Thu, 30 Nov 2017 19:43:12 -0500 Subject: [PATCH 36/42] Correct go vet for Go tip https://travis-ci.org/cloudflare/logshare/jobs/309796141#L646 --- app.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app.go b/app.go index 60599b0..9add067 100644 --- a/app.go +++ b/app.go @@ -453,7 +453,6 @@ func (a *App) hasFlag(flag Flag) bool { } func (a *App) errWriter() io.Writer { - // When the app ErrWriter is nil use the package level one. if a.ErrWriter == nil { return ErrWriter From df562bf1a8626f2d16f91fcbf7230a5bdca3d592 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 3 Dec 2017 13:38:50 -0800 Subject: [PATCH 37/42] Refactor flag handling logic Refactor logic introduced by #686 --- command.go | 134 +++++++++++++++++++++-------------------------------- 1 file changed, 52 insertions(+), 82 deletions(-) diff --git a/command.go b/command.go index 7d0357b..66a58b5 100644 --- a/command.go +++ b/command.go @@ -1,6 +1,7 @@ package cli import ( + "flag" "fmt" "io/ioutil" "sort" @@ -110,43 +111,7 @@ func (c Command) Run(ctx *Context) (err error) { ) } - set, err := flagSet(c.Name, c.Flags) - if err != nil { - return err - } - set.SetOutput(ioutil.Discard) - firstFlagIndex, terminatorIndex := getIndexes(ctx) - flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex) - if c.UseShortOptionHandling { - flagArgs = translateShortOptions(flagArgs) - } - if c.SkipFlagParsing { - err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...)) - } else if !c.SkipArgReorder { - if firstFlagIndex > -1 { - err = set.Parse(append(flagArgs, regularArgs...)) - } else { - err = set.Parse(ctx.Args().Tail()) - } - } else if c.UseShortOptionHandling { - if terminatorIndex == -1 && firstFlagIndex > -1 { - // Handle shortname AND no options - err = set.Parse(append(regularArgs, flagArgs...)) - } else { - // Handle shortname and options - err = set.Parse(flagArgs) - } - } else { - err = set.Parse(append(regularArgs, flagArgs...)) - } - - 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 - } + set, err := c.parseFlags(ctx.Args().Tail()) context := NewContext(ctx.App, set, ctx) context.Command = c @@ -205,60 +170,65 @@ func (c Command) Run(ctx *Context) (err error) { return err } -func getIndexes(ctx *Context) (int, int) { - firstFlagIndex := -1 - terminatorIndex := -1 - for index, arg := range ctx.Args() { - if arg == "--" { - terminatorIndex = index - break - } else if arg == "-" { - // Do nothing. A dash alone is not really a flag. - continue - } else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 { - firstFlagIndex = index - } +func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { + set, err := flagSet(c.Name, c.Flags) + if err != nil { + return nil, err } - if len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 { - return -1, -1 + set.SetOutput(ioutil.Discard) + + if c.SkipFlagParsing { + return set, set.Parse(append([]string{c.Name, "--"}, args...)) } - return firstFlagIndex, terminatorIndex + if c.UseShortOptionHandling { + args = translateShortOptions(args) + } -} + if !c.SkipArgReorder { + args = reorderArgs(args) + } -// copyStringslice takes a string slice and copies it -func copyStringSlice(slice []string, start, end int) []string { - newSlice := make([]string, end-start) - copy(newSlice, slice[start:end]) - return newSlice -} + err = set.Parse(args) + if err != nil { + return nil, err + } -// getAllArgs extracts and returns two string slices representing -// regularArgs and flagArgs -func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) { - var regularArgs []string - // if there are no options, the we set the index to 1 manually - if firstFlagIndex == -1 { - firstFlagIndex = 1 - regularArgs = copyStringSlice(args, 0, len(args)) - } else { - regularArgs = copyStringSlice(args, 1, firstFlagIndex) + err = normalizeFlags(c.Flags, set) + if err != nil { + return nil, err } - var flagArgs []string - // a flag terminatorIndex was found in the input. we need to collect - // flagArgs based on it. - if terminatorIndex > -1 { - flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex) - additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args)) - regularArgs = append(regularArgs, additionalRegularArgs...) - for _, i := range additionalRegularArgs { - regularArgs = append(regularArgs, i) + + return set, nil +} + +// reorderArgs moves all flags before arguments as this is what flag expects +func reorderArgs(args []string) []string { + var nonflags, flags []string + + readFlagValue := false + for i, arg := range args { + if arg == "--" { + nonflags = append(nonflags, args[i:]...) + break + } + + if readFlagValue { + readFlagValue = false + flags = append(flags, arg) + continue + } + + if arg != "-" && strings.HasPrefix(arg, "-") { + flags = append(flags, arg) + + readFlagValue = !strings.Contains(arg, "=") + } else { + nonflags = append(nonflags, arg) } - } else { - flagArgs = args[firstFlagIndex:] } - return flagArgs, regularArgs + + return append(flags, nonflags...) } func translateShortOptions(flagArgs Args) []string { From 0671b166dcacb3dc1215ba65bf986dab194581dc Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 4 Dec 2017 09:23:40 -0800 Subject: [PATCH 38/42] Add tests for flag reordering --- command_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/command_test.go b/command_test.go index e69750a..c235e4a 100644 --- a/command_test.go +++ b/command_test.go @@ -243,3 +243,41 @@ func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) { t.Fatal(err) } } + +func TestCommandFlagReordering(t *testing.T) { + cases := []struct { + testArgs []string + expectedValue string + expectedArgs []string + expectedErr error + }{ + {[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, "foo", []string{"some-arg"}, nil}, + {[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, "foo", []string{"some-arg"}, nil}, + {[]string{"some-exec", "some-command", "--flag=foo", "some-arg"}, "foo", []string{"some-arg"}, nil}, + } + + for _, c := range cases { + value := "" + args := []string{} + app := &App{ + Commands: []Command{ + { + Name: "some-command", + Flags: []Flag{ + StringFlag{Name: "flag"}, + }, + Action: func(c *Context) { + fmt.Printf("%+v\n", c.String("flag")) + value = c.String("flag") + args = c.Args() + }, + }, + }, + } + + err := app.Run(c.testArgs) + expect(t, err, c.expectedErr) + expect(t, value, c.expectedValue) + expect(t, args, c.expectedArgs) + } +} From e38e4ae2d05acf5b5164c160a67fb7048e1358b0 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Fri, 29 Dec 2017 13:38:18 -0500 Subject: [PATCH 39/42] Fix regression of SkipFlagParsing behavior Introduced by df562bf1a8626f2d16f91fcbf7230a5bdca3d592 Was mistakenly prepending the command name. --- command.go | 2 +- command_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index 66a58b5..ed4a81a 100644 --- a/command.go +++ b/command.go @@ -178,7 +178,7 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { set.SetOutput(ioutil.Discard) if c.SkipFlagParsing { - return set, set.Parse(append([]string{c.Name, "--"}, args...)) + return set, set.Parse(append([]string{"--"}, args...)) } if c.UseShortOptionHandling { diff --git a/command_test.go b/command_test.go index c235e4a..c84b762 100644 --- a/command_test.go +++ b/command_test.go @@ -281,3 +281,39 @@ func TestCommandFlagReordering(t *testing.T) { expect(t, args, c.expectedArgs) } } + +func TestCommandSkipFlagParsing(t *testing.T) { + cases := []struct { + testArgs []string + expectedArgs []string + expectedErr error + }{ + {[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, []string{"some-arg", "--flag", "foo"}, nil}, + {[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, []string{"some-arg", "--flag=foo"}, nil}, + } + + for _, c := range cases { + value := "" + args := []string{} + app := &App{ + Commands: []Command{ + { + SkipFlagParsing: true, + Name: "some-command", + Flags: []Flag{ + StringFlag{Name: "flag"}, + }, + Action: func(c *Context) { + fmt.Printf("%+v\n", c.String("flag")) + value = c.String("flag") + args = c.Args() + }, + }, + }, + } + + err := app.Run(c.testArgs) + expect(t, err, c.expectedErr) + expect(t, args, c.expectedArgs) + } +} From d7555e172994da8d058334aa1fe69533b1685924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Fri, 26 Jan 2018 21:14:34 +0100 Subject: [PATCH 40/42] Fix unnecessary uses of Sprintf - use strconv directly - use concatenation for "%s%s" --- flag.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/flag.go b/flag.go index d4a4d41..b0cffc0 100644 --- a/flag.go +++ b/flag.go @@ -636,7 +636,7 @@ func withEnvHint(envVar, str string) string { suffix = "%" sep = "%, %" } - envText = fmt.Sprintf(" [%s%s%s]", prefix, strings.Join(strings.Split(envVar, ","), sep), suffix) + envText = " [" + prefix + strings.Join(strings.Split(envVar, ","), sep) + suffix + "]" } return str + envText } @@ -709,13 +709,13 @@ func stringifyFlag(f Flag) string { placeholder = defaultPlaceholder } - usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString)) + usageWithDefault := strings.TrimSpace(usage + defaultValueString) return FlagFileHinter( fv.FieldByName("FilePath").String(), FlagEnvHinter( fv.FieldByName("EnvVar").String(), - fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault), + FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder)+"\t"+usageWithDefault, ), ) } @@ -724,7 +724,7 @@ func stringifyIntSliceFlag(f IntSliceFlag) string { defaultVals := []string{} if f.Value != nil && len(f.Value.Value()) > 0 { for _, i := range f.Value.Value() { - defaultVals = append(defaultVals, fmt.Sprintf("%d", i)) + defaultVals = append(defaultVals, strconv.Itoa(i)) } } @@ -735,7 +735,7 @@ func stringifyInt64SliceFlag(f Int64SliceFlag) string { defaultVals := []string{} if f.Value != nil && len(f.Value.Value()) > 0 { for _, i := range f.Value.Value() { - defaultVals = append(defaultVals, fmt.Sprintf("%d", i)) + defaultVals = append(defaultVals, strconv.FormatInt(i, 10)) } } @@ -747,7 +747,7 @@ func stringifyStringSliceFlag(f StringSliceFlag) string { if f.Value != nil && len(f.Value.Value()) > 0 { for _, s := range f.Value.Value() { if len(s) > 0 { - defaultVals = append(defaultVals, fmt.Sprintf("%q", s)) + defaultVals = append(defaultVals, strconv.Quote(s)) } } } @@ -766,8 +766,8 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { defaultVal = fmt.Sprintf(" (default: %s)", strings.Join(defaultVals, ", ")) } - usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal)) - return fmt.Sprintf("%s\t%s", FlagNamePrefixer(name, placeholder), usageWithDefault) + usageWithDefault := strings.TrimSpace(usage + defaultVal) + return FlagNamePrefixer(name, placeholder) + "\t" + usageWithDefault } func flagFromFileEnv(filePath, envName string) (val string, ok bool) { From 3a87b13b01ac8628694f1e1b20bdb452cc0f54d2 Mon Sep 17 00:00:00 2001 From: Nico Windler Date: Sat, 10 Feb 2018 13:35:23 +0100 Subject: [PATCH 41/42] Fix args reordering when bool flags are present --- app_test.go | 33 +++++++++++++++++++++++++++++++++ command.go | 3 ++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app_test.go b/app_test.go index 5db9882..42fb149 100644 --- a/app_test.go +++ b/app_test.go @@ -329,6 +329,39 @@ func TestApp_CommandWithArgBeforeFlags(t *testing.T) { expect(t, firstArg, "my-arg") } +func TestApp_CommandWithArgBeforeBoolFlags(t *testing.T) { + var parsedOption, parsedSecondOption, firstArg string + var parsedBool, parsedSecondBool bool + + app := NewApp() + command := Command{ + Name: "cmd", + Flags: []Flag{ + StringFlag{Name: "option", Value: "", Usage: "some option"}, + StringFlag{Name: "secondOption", Value: "", Usage: "another option"}, + BoolFlag{Name: "boolflag", Usage: "some bool"}, + BoolFlag{Name: "b", Usage: "another bool"}, + }, + Action: func(c *Context) error { + parsedOption = c.String("option") + parsedSecondOption = c.String("secondOption") + parsedBool = c.Bool("boolflag") + parsedSecondBool = c.Bool("b") + firstArg = c.Args().First() + return nil + }, + } + app.Commands = []Command{command} + + app.Run([]string{"", "cmd", "my-arg", "--boolflag", "--option", "my-option", "-b", "--secondOption", "fancy-option"}) + + expect(t, parsedOption, "my-option") + expect(t, parsedSecondOption, "fancy-option") + expect(t, parsedBool, true) + expect(t, parsedSecondBool, true) + expect(t, firstArg, "my-arg") +} + func TestApp_RunAsSubcommandParseFlags(t *testing.T) { var context *Context diff --git a/command.go b/command.go index ed4a81a..2acb976 100644 --- a/command.go +++ b/command.go @@ -213,11 +213,12 @@ func reorderArgs(args []string) []string { break } - if readFlagValue { + if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { readFlagValue = false flags = append(flags, arg) continue } + readFlagValue = false if arg != "-" && strings.HasPrefix(arg, "-") { flags = append(flags, arg) From 45289ea7a0de564a71532e13b9916961a38abc8e Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 20 Feb 2018 12:40:43 -0500 Subject: [PATCH 42/42] Adjust contribution and maintainer prose per current reality --- CODE_OF_CONDUCT.md | 74 ++++++++++++++++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 19 ++++++++++++ MAINTAINERS.md | 1 + README.md | 20 +++---------- 4 files changed, 98 insertions(+), 16 deletions(-) create mode 100644 CODE_OF_CONDUCT.md create mode 100644 CONTRIBUTING.md create mode 100644 MAINTAINERS.md diff --git a/CODE_OF_CONDUCT.md b/CODE_OF_CONDUCT.md new file mode 100644 index 0000000..41ba294 --- /dev/null +++ b/CODE_OF_CONDUCT.md @@ -0,0 +1,74 @@ +# Contributor Covenant Code of Conduct + +## Our Pledge + +In the interest of fostering an open and welcoming environment, we as +contributors and maintainers pledge to making participation in our project and +our community a harassment-free experience for everyone, regardless of age, body +size, disability, ethnicity, gender identity and expression, level of experience, +education, socio-economic status, nationality, personal appearance, race, +religion, or sexual identity and orientation. + +## Our Standards + +Examples of behavior that contributes to creating a positive environment +include: + +* Using welcoming and inclusive language +* Being respectful of differing viewpoints and experiences +* Gracefully accepting constructive criticism +* Focusing on what is best for the community +* Showing empathy towards other community members + +Examples of unacceptable behavior by participants include: + +* The use of sexualized language or imagery and unwelcome sexual attention or + advances +* Trolling, insulting/derogatory comments, and personal or political attacks +* Public or private harassment +* Publishing others' private information, such as a physical or electronic + address, without explicit permission +* Other conduct which could reasonably be considered inappropriate in a + professional setting + +## Our Responsibilities + +Project maintainers are responsible for clarifying the standards of acceptable +behavior and are expected to take appropriate and fair corrective action in +response to any instances of unacceptable behavior. + +Project maintainers have the right and responsibility to remove, edit, or +reject comments, commits, code, wiki edits, issues, and other contributions +that are not aligned to this Code of Conduct, or to ban temporarily or +permanently any contributor for other behaviors that they deem inappropriate, +threatening, offensive, or harmful. + +## Scope + +This Code of Conduct applies both within project spaces and in public spaces +when an individual is representing the project or its community. Examples of +representing a project or community include using an official project e-mail +address, posting via an official social media account, or acting as an appointed +representative at an online or offline event. Representation of a project may be +further defined and clarified by project maintainers. + +## Enforcement + +Instances of abusive, harassing, or otherwise unacceptable behavior may be +reported by contacting Dan Buch at dan@meatballhat.com. All complaints will be +reviewed and investigated and will result in a response that is deemed necessary +and appropriate to the circumstances. The project team is obligated to maintain +confidentiality with regard to the reporter of an incident. Further details of +specific enforcement policies may be posted separately. + +Project maintainers who do not follow or enforce the Code of Conduct in good +faith may face temporary or permanent repercussions as determined by other +members of the project's leadership. + +## Attribution + +This Code of Conduct is adapted from the [Contributor Covenant][homepage], version 1.4, +available at https://www.contributor-covenant.org/version/1/4/code-of-conduct.html + +[homepage]: https://www.contributor-covenant.org + diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..329195e --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,19 @@ +## Contributing + +**NOTE**: the primary maintainer(s) may be found in +[./MAINTAINERS.md](./MAINTAINERS.md). + +Feel free to put up a pull request to fix a bug or maybe add a feature. I will +give it a code review and make sure that it does not break backwards +compatibility. If I or any other collaborators agree that it is in line with +the vision of the project, we will work with you to get the code into +a mergeable state and merge it into the master branch. + +If you have contributed something significant to the project, we will most +likely add you as a collaborator. As a collaborator you are given the ability +to merge others pull requests. It is very important that new code does not +break existing code, so be careful about what code you do choose to merge. + +If you feel like you have contributed to the project but have not yet been added +as a collaborator, we probably forgot to add you :sweat_smile:. Please open an +issue! diff --git a/MAINTAINERS.md b/MAINTAINERS.md new file mode 100644 index 0000000..f6bdd99 --- /dev/null +++ b/MAINTAINERS.md @@ -0,0 +1 @@ +- @meatballhat diff --git a/README.md b/README.md index 6096701..7750de7 100644 --- a/README.md +++ b/README.md @@ -9,9 +9,9 @@ cli [![top level coverage](https://gocover.io/_badge/github.com/urfave/cli?0 "top level coverage")](http://gocover.io/github.com/urfave/cli) / [![altsrc coverage](https://gocover.io/_badge/github.com/urfave/cli/altsrc?0 "altsrc coverage")](http://gocover.io/github.com/urfave/cli/altsrc) -**Notice:** This is the library formerly known as -`github.com/codegangsta/cli` -- Github will automatically redirect requests -to this repository, but we recommend updating your references for clarity. +This is the library formerly known as `github.com/codegangsta/cli` -- Github +will automatically redirect requests to this repository, but we recommend +updating your references for clarity. cli is a simple, fast, and fun package for building command line apps in Go. The goal is to enable developers to write fast and distributable command line @@ -1433,16 +1433,4 @@ with two leading dashes (such as **--options**) are still valid. ## Contribution Guidelines -Feel free to put up a pull request to fix a bug or maybe add a feature. I will -give it a code review and make sure that it does not break backwards -compatibility. If I or any other collaborators agree that it is in line with -the vision of the project, we will work with you to get the code into -a mergeable state and merge it into the master branch. - -If you have contributed something significant to the project, we will most -likely add you as a collaborator. As a collaborator you are given the ability -to merge others pull requests. It is very important that new code does not -break existing code, so be careful about what code you do choose to merge. - -If you feel like you have contributed to the project but have not yet been -added as a collaborator, we probably forgot to add you, please open an issue. +See [./CONTRIBUTING.md](./CONTRIBUTING.md)