From 6c2c80959b882999af6d7e3d3256c01d1c82e5fc Mon Sep 17 00:00:00 2001 From: Cody Moore Date: Wed, 5 Oct 2016 11:17:03 -0700 Subject: [PATCH 1/9] Typo in comments Typo changing `implents` to `implements` --- errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/errors.go b/errors.go index c7d8c2f..ddef369 100644 --- a/errors.go +++ b/errors.go @@ -24,7 +24,7 @@ func NewMultiError(err ...error) MultiError { return MultiError{Errors: err} } -// Error implents the error interface. +// Error implements the error interface. func (m MultiError) Error() string { errs := make([]string, len(m.Errors)) for i, err := range m.Errors { From c4a46a7df2858904db25b7eaabaf5a75458bea3f Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Wed, 12 Oct 2016 08:06:41 -0700 Subject: [PATCH 2/9] app: Add App.Description So you can describe what the application is for without requiring users to drill down into a particular command. --- app.go | 2 ++ help.go | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app.go b/app.go index b9adf46..e5c2839 100644 --- a/app.go +++ b/app.go @@ -42,6 +42,8 @@ type App struct { ArgsUsage string // Version of the program Version string + // Description of the program + Description string // List of commands to execute Commands []Command // List of flags to parse diff --git a/help.go b/help.go index ba34719..529e5ea 100644 --- a/help.go +++ b/help.go @@ -20,7 +20,10 @@ USAGE: {{if .Version}}{{if not .HideVersion}} VERSION: {{.Version}} - {{end}}{{end}}{{if len .Authors}} + {{end}}{{end}}{{if .Description}} +DESCRIPTION: + {{.Description}} +{{end}}{{if len .Authors}} AUTHOR(S): {{range .Authors}}{{.}}{{end}} {{end}}{{if .VisibleCommands}} From 508a23430b94565dc28414c9587d31626b3ce596 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 18 Oct 2016 20:56:31 -0700 Subject: [PATCH 3/9] Default app.Writer to os.Stdout As NewApp() does. Fixes #545 --- app.go | 4 ++++ app_test.go | 6 ++++++ 2 files changed, 10 insertions(+) diff --git a/app.go b/app.go index b9adf46..c7e2594 100644 --- a/app.go +++ b/app.go @@ -165,6 +165,10 @@ func (a *App) Setup() { if a.Metadata == nil { a.Metadata = make(map[string]interface{}) } + + if a.Writer == nil { + a.Writer = os.Stdout + } } // Run is the entry point to the cli app. Parses the arguments slice and routes diff --git a/app_test.go b/app_test.go index 23c8aa6..59fea69 100644 --- a/app_test.go +++ b/app_test.go @@ -202,6 +202,12 @@ func TestApp_Command(t *testing.T) { } } +func TestApp_Setup_defaultsWriter(t *testing.T) { + app := &App{} + app.Setup() + expect(t, app.Writer, os.Stdout) +} + func TestApp_CommandWithArgBeforeFlags(t *testing.T) { var parsedOption, firstArg string From 4cc2bad36ed25559dd5b3de9edec78def518fd15 Mon Sep 17 00:00:00 2001 From: Richard Kovacs Date: Fri, 7 Oct 2016 16:38:36 +0200 Subject: [PATCH 4/9] Display error instead of just say command is incorrect --- app.go | 4 ++-- app_test.go | 17 +++++++++++++++++ command.go | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/app.go b/app.go index b9adf46..c3462b7 100644 --- a/app.go +++ b/app.go @@ -194,7 +194,7 @@ func (a *App) Run(arguments []string) (err error) { HandleExitCoder(err) return err } - fmt.Fprintf(a.Writer, "%s\n\n", "Incorrect Usage.") + fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) ShowAppHelp(context) return err } @@ -315,7 +315,7 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { HandleExitCoder(err) return err } - fmt.Fprintf(a.Writer, "%s\n\n", "Incorrect Usage.") + fmt.Fprintf(a.Writer, "%s %s\n\n", "Incorrect Usage.", err.Error()) ShowSubcommandHelp(context) return err } diff --git a/app_test.go b/app_test.go index 23c8aa6..e2e4087 100644 --- a/app_test.go +++ b/app_test.go @@ -252,6 +252,23 @@ func TestApp_RunAsSubcommandParseFlags(t *testing.T) { expect(t, context.String("lang"), "spanish") } +func TestApp_RunAsSubCommandIncorrectUsage(t *testing.T) { + a := App{ + Flags: []Flag{ + StringFlag{Name: "--foo"}, + }, + Writer: bytes.NewBufferString(""), + } + + set := flag.NewFlagSet("", flag.ContinueOnError) + set.Parse([]string{"", "---foo"}) + c := &Context{flagSet: set} + + err := a.RunAsSubcommand(c) + + expect(t, err, errors.New("bad flag syntax: ---foo")) +} + func TestApp_CommandWithFlagBeforeTerminator(t *testing.T) { var parsedOption string var args []string diff --git a/command.go b/command.go index 96253b6..d955249 100644 --- a/command.go +++ b/command.go @@ -138,7 +138,7 @@ func (c Command) Run(ctx *Context) (err error) { HandleExitCoder(err) return err } - fmt.Fprintln(ctx.App.Writer, "Incorrect Usage.") + fmt.Fprintln(ctx.App.Writer, "Incorrect Usage:", err.Error()) fmt.Fprintln(ctx.App.Writer) ShowCommandHelp(ctx, c.Name) return err From c516bce8f12bf57771f9be57baa7c895e2039477 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 13 Oct 2016 16:54:57 +0200 Subject: [PATCH 5/9] flags: provide a type to sort flags Signed-off-by: Antonio Murdaca --- README.md | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++ flag.go | 15 +++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/README.md b/README.md index a1e537c..bb5f61e 100644 --- a/README.md +++ b/README.md @@ -30,6 +30,7 @@ applications in an expressive way. * [Flags](#flags) + [Placeholder Values](#placeholder-values) + [Alternate Names](#alternate-names) + + [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) * [Subcommands](#subcommands) @@ -450,6 +451,56 @@ That flag can then be set with `--lang spanish` or `-l spanish`. Note that giving two different forms of the same flag in the same command invocation is an error. +#### Ordering + +Flags for the application and commands are shown in the order they are defined. +However, it's possible to sort them from outside this library by using `FlagsByName` +with `sort`. + +For example this: + + +``` go +package main + +import ( + "os" + "sort" + + "github.com/urfave/cli" +) + +func main() { + app := cli.NewApp() + + app.Flags = []cli.Flag { + cli.StringFlag{ + Name: "lang, l", + Value: "english", + Usage: "Language for the greeting", + }, + cli.StringFlag{ + Name: "config, c", + Usage: "Load configuration from `FILE`", + }, + } + + sort.Sort(cli.FlagsByName(app.Flags)) + + app.Run(os.Args) +} +``` + +Will result in help output like: + +``` +--config FILE, -c FILE Load configuration from FILE +--lang value, -l value Language for the greeting (default: "english") +``` + #### Values from the Environment You can also have the default value set from the environment via `EnvVar`. e.g. diff --git a/flag.go b/flag.go index e748c02..1ff28d3 100644 --- a/flag.go +++ b/flag.go @@ -37,6 +37,21 @@ var HelpFlag = BoolFlag{ // to display a flag. var FlagStringer FlagStringFunc = stringifyFlag +// FlagsByName is a slice of Flag. +type FlagsByName []Flag + +func (f FlagsByName) Len() int { + return len(f) +} + +func (f FlagsByName) Less(i, j int) bool { + return f[i].GetName() < f[j].GetName() +} + +func (f FlagsByName) Swap(i, j int) { + f[i], f[j] = f[j], f[i] +} + // Flag is a common interface related to parsing flags in cli. // For more advanced flag parsing techniques, it is recommended that // this interface be implemented. From d913b71c72fbd5857a4e5219d53023d14f0eb346 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Fri, 21 Oct 2016 10:42:30 +0200 Subject: [PATCH 6/9] .travis.yml: add go 1.7.x Signed-off-by: Antonio Murdaca --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index a6a1386..94836d7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,7 @@ go: - 1.4.2 - 1.5.x - 1.6.x +- 1.7.x - master matrix: @@ -20,6 +21,8 @@ matrix: include: - go: 1.6.x os: osx + - go: 1.7.x + os: osx before_script: - go get github.com/urfave/gfmrun/... || true From 0c143a2a268ff019a99698fe2e6d9e7bfed088c7 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 22 Oct 2016 15:58:07 -0700 Subject: [PATCH 7/9] app: Fix trailing space for Author.String() This code initially landed with lots of space: '{name} <{email}> ' or: '{name} ' in 3d718330 (app, help: add support for multiple authors, 2015-01-31). The doubled space between the name and email was removed in c6592bb4 (app, help: add backwards compatibility for Authors, 2015-02-21), but a trailing space remained in both the email and email-less cases. This commit removes that trailing space. --- app.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index e5c2839..77eb141 100644 --- a/app.go +++ b/app.go @@ -458,10 +458,10 @@ type Author struct { func (a Author) String() string { e := "" if a.Email != "" { - e = "<" + a.Email + "> " + e = " <" + a.Email + ">" } - return fmt.Sprintf("%v %v", a.Name, e) + return fmt.Sprintf("%v%v", a.Name, e) } // HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an From 3c2bce5807d92a07ae1d3adf503a9811525e3e4b Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 22 Oct 2016 16:02:21 -0700 Subject: [PATCH 8/9] help: Cleanup AppHelpTemplate trailing whitespace Most of the changes here remove trailing whitespace, but I also add code to select "AUTHOR" or "AUTHORS" as appropriate instead of the previous "AUTHOR(S)". The template for listing with an entry per line is: {{range $index, $entry := pipeline}}{{if $index}} {{end}}{{$entry}}{{end}} That range syntax is discussed in [1]. Also add a unit test, which tests both these whitespace changes and also the earlier App.Description addition. [1]: https://golang.org/pkg/text/template/#hdr-Variables --- app_test.go | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++- help.go | 31 +++++++++++++++-------------- 2 files changed, 72 insertions(+), 16 deletions(-) diff --git a/app_test.go b/app_test.go index 23c8aa6..2b541f8 100644 --- a/app_test.go +++ b/app_test.go @@ -90,7 +90,62 @@ func ExampleApp_Run_subcommand() { // Hello, Jeremy } -func ExampleApp_Run_help() { +func ExampleApp_Run_appHelp() { + // set args for examples sake + os.Args = []string{"greet", "help"} + + app := NewApp() + app.Name = "greet" + app.Version = "0.1.0" + app.Description = "This is how we describe greet the app" + app.Authors = []Author{ + {Name: "Harrison", Email: "harrison@lolwut.com"}, + {Name: "Oliver Allen", Email: "oliver@toyshop.com"}, + } + app.Flags = []Flag{ + StringFlag{Name: "name", Value: "bob", Usage: "a name to say"}, + } + app.Commands = []Command{ + { + Name: "describeit", + Aliases: []string{"d"}, + Usage: "use it to see a description", + Description: "This is how we describe describeit the function", + Action: func(c *Context) error { + fmt.Printf("i like to describe things") + return nil + }, + }, + } + app.Run(os.Args) + // Output: + // NAME: + // greet - A new cli application + // + // USAGE: + // greet [global options] command [command options] [arguments...] + // + // VERSION: + // 0.1.0 + // + // DESCRIPTION: + // This is how we describe greet the app + // + // AUTHORS: + // Harrison + // Oliver Allen + // + // COMMANDS: + // describeit, d use it to see a description + // help, h Shows a list of commands or help for one command + // + // GLOBAL OPTIONS: + // --name value a name to say (default: "bob") + // --help, -h show help + // --version, -v print the version +} + +func ExampleApp_Run_commandHelp() { // set args for examples sake os.Args = []string{"greet", "h", "describeit"} diff --git a/help.go b/help.go index 529e5ea..515f744 100644 --- a/help.go +++ b/help.go @@ -16,27 +16,28 @@ var AppHelpTemplate = `NAME: {{.Name}} - {{.Usage}} USAGE: - {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}} - {{if .Version}}{{if not .HideVersion}} + {{if .UsageText}}{{.UsageText}}{{else}}{{.HelpName}} {{if .VisibleFlags}}[global options]{{end}}{{if .Commands}} command [command options]{{end}} {{if .ArgsUsage}}{{.ArgsUsage}}{{else}}[arguments...]{{end}}{{end}}{{if .Version}}{{if not .HideVersion}} + VERSION: - {{.Version}} - {{end}}{{end}}{{if .Description}} + {{.Version}}{{end}}{{end}}{{if .Description}} + DESCRIPTION: - {{.Description}} -{{end}}{{if len .Authors}} -AUTHOR(S): - {{range .Authors}}{{.}}{{end}} - {{end}}{{if .VisibleCommands}} + {{.Description}}{{end}}{{if len .Authors}} + +AUTHOR{{with $length := len .Authors}}{{if ne 1 $length}}S{{end}}{{end}}: + {{range $index, $author := .Authors}}{{if $index}} + {{end}}{{$author}}{{end}}{{end}}{{if .VisibleCommands}} + COMMANDS:{{range .VisibleCategories}}{{if .Name}} {{.Name}}:{{end}}{{range .VisibleCommands}} - {{join .Names ", "}}{{"\t"}}{{.Usage}}{{end}} -{{end}}{{end}}{{if .VisibleFlags}} + {{join .Names ", "}}{{"\t"}}{{.Usage}}{{end}}{{end}}{{end}}{{if .VisibleFlags}} + GLOBAL OPTIONS: - {{range .VisibleFlags}}{{.}} - {{end}}{{end}}{{if .Copyright}} + {{range $index, $option := .VisibleFlags}}{{if $index}} + {{end}}{{$option}}{{end}}{{end}}{{if .Copyright}} + COPYRIGHT: - {{.Copyright}} - {{end}} + {{.Copyright}}{{end}} ` // CommandHelpTemplate is the text template for the command help topic. From b377b5d9e93d2359a7ffe16fc1bb902b3df291b6 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Tue, 1 Nov 2016 20:33:12 -0700 Subject: [PATCH 9/9] Use type assertions rather than reflection to determine how to call the `Action` This has some benefits, but results in possibly less informative error messaging; however, given that there are only two accepted types, I think the error messaging is sufficient. --- app.go | 52 +++++++++++----------------------------------------- app_test.go | 6 +++--- 2 files changed, 14 insertions(+), 44 deletions(-) diff --git a/app.go b/app.go index dd2d2df..26cf09a 100644 --- a/app.go +++ b/app.go @@ -6,9 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" - "reflect" "sort" - "strings" "time" ) @@ -19,11 +17,8 @@ var ( contactSysadmin = "This is an error in the application. Please contact the distributor of this application if this is not you." - errNonFuncAction = NewExitError("ERROR invalid Action type. "+ - fmt.Sprintf("Must be a func of type `cli.ActionFunc`. %s", contactSysadmin)+ - fmt.Sprintf("See %s", appActionDeprecationURL), 2) - errInvalidActionSignature = NewExitError("ERROR invalid Action signature. "+ - fmt.Sprintf("Must be `cli.ActionFunc`. %s", contactSysadmin)+ + errInvalidActionType = NewExitError("ERROR invalid Action type. "+ + fmt.Sprintf("Must be `func(*Context`)` or `func(*Context) error). %s", contactSysadmin)+ fmt.Sprintf("See %s", appActionDeprecationURL), 2) ) @@ -468,41 +463,16 @@ func (a Author) String() string { return fmt.Sprintf("%v%v", a.Name, e) } -// HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an -// ActionFunc, a func with the legacy signature for Action, or some other -// invalid thing. If it's an ActionFunc or a func with the legacy signature for -// Action, the func is run! +// HandleAction attempts to figure out which Action signature was used. If +// it's an ActionFunc or a func with the legacy signature for Action, the func +// is run! func HandleAction(action interface{}, context *Context) (err error) { - defer func() { - if r := recover(); r != nil { - // Try to detect a known reflection error from *this scope*, rather than - // swallowing all panics that may happen when calling an Action func. - s := fmt.Sprintf("%v", r) - if strings.HasPrefix(s, "reflect: ") && strings.Contains(s, "too many input arguments") { - err = NewExitError(fmt.Sprintf("ERROR unknown Action error: %v.", r), 2) - } else { - panic(r) - } - } - }() - - if reflect.TypeOf(action).Kind() != reflect.Func { - return errNonFuncAction - } - - vals := reflect.ValueOf(action).Call([]reflect.Value{reflect.ValueOf(context)}) - - if len(vals) == 0 { + if a, ok := action.(func(*Context) error); ok { + return a(context) + } else if a, ok := action.(func(*Context)); ok { // deprecated function signature + a(context) return nil + } else { + return errInvalidActionType } - - if len(vals) > 1 { - return errInvalidActionSignature - } - - if retErr, ok := vals[0].Interface().(error); vals[0].IsValid() && ok { - return retErr - } - - return err } diff --git a/app_test.go b/app_test.go index e36a8c2..40a598d 100644 --- a/app_test.go +++ b/app_test.go @@ -1492,7 +1492,7 @@ func TestHandleAction_WithNonFuncAction(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type.") { t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error()) } @@ -1516,7 +1516,7 @@ func TestHandleAction_WithInvalidFuncSignature(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR unknown Action error") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { t.Fatalf("expected an unknown Action error, but got: %v", exitErr.Error()) } @@ -1540,7 +1540,7 @@ func TestHandleAction_WithInvalidFuncReturnSignature(t *testing.T) { t.Fatalf("expected to receive a *ExitError") } - if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action signature") { + if !strings.HasPrefix(exitErr.Error(), "ERROR invalid Action type") { t.Fatalf("expected an invalid Action signature error, but got: %v", exitErr.Error()) }