From 683e911d61b0be794eec4200056f0451b3976622 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Sun, 31 Aug 2014 11:40:31 -0700 Subject: [PATCH] Return error from app and command actions. See #66. This introduces a significant breaking change, and I don't expect it to be merged off-hand. I do think that it's worth discussion, since it seems like a very idiomatic choice in terms of how errors would be handled. A similar backwards-incompatible change was introduced in e6e64114, allowing the app's Run method to return an error. --- app.go | 5 ++--- app_test.go | 36 ++++++++++++++++++++++++------------ cli_test.go | 18 ++++++++++++------ command.go | 5 ++--- command_test.go | 4 ++-- flag_test.go | 48 ++++++++++++++++++++++++++++++++---------------- help.go | 6 ++++-- 7 files changed, 78 insertions(+), 44 deletions(-) diff --git a/app.go b/app.go index 66e541c..145611b 100644 --- a/app.go +++ b/app.go @@ -30,7 +30,7 @@ type App struct { // If a non-nil error is returned, no subcommands are run Before func(context *Context) error // The action to execute when no subcommands are specified - Action func(context *Context) + Action func(context *Context) error // Execute this function if the proper command cannot be found CommandNotFound func(context *Context, command string) // Compilation date @@ -127,8 +127,7 @@ func (a *App) Run(arguments []string) error { } // Run default Action - a.Action(context) - return nil + return a.Action(context) } // Another entry point to the cli app, takes care of passing arguments and error handling diff --git a/app_test.go b/app_test.go index 81d1174..cefa5fd 100644 --- a/app_test.go +++ b/app_test.go @@ -17,8 +17,9 @@ func ExampleApp() { app.Flags = []cli.Flag{ cli.StringFlag{Name: "name", Value: "bob", Usage: "a name to say"}, } - app.Action = func(c *cli.Context) { + app.Action = func(c *cli.Context) error { fmt.Printf("Hello %v\n", c.String("name")) + return nil } app.Run(os.Args) // Output: @@ -49,8 +50,9 @@ func ExampleAppSubcommand() { Usage: "Name of the person to greet", }, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { fmt.Println("Hello,", c.String("name")) + return nil }, }, }, @@ -77,8 +79,9 @@ func ExampleAppHelp() { ShortName: "d", Usage: "use it to see a description", Description: "This is how we describe describeit the function", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { fmt.Printf("i like to describe things") + return nil }, }, } @@ -107,15 +110,17 @@ func ExampleAppBashComplete() { ShortName: "d", Usage: "use it to see a description", Description: "This is how we describe describeit the function", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { fmt.Printf("i like to describe things") + return nil }, }, { Name: "next", Usage: "next example", Description: "more stuff to see when generating bash completion", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { fmt.Printf("the next example") + return nil }, }, } @@ -133,8 +138,9 @@ func TestApp_Run(t *testing.T) { s := "" app := cli.NewApp() - app.Action = func(c *cli.Context) { + app.Action = func(c *cli.Context) error { s = s + c.Args().First() + return nil } err := app.Run([]string{"command", "foo"}) @@ -179,9 +185,10 @@ func TestApp_CommandWithArgBeforeFlags(t *testing.T) { Flags: []cli.Flag{ cli.StringFlag{Name: "option", Value: "", Usage: "some option"}, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { parsedOption = c.String("option") firstArg = c.Args().First() + return nil }, } app.Commands = []cli.Command{command} @@ -199,8 +206,9 @@ func TestApp_Float64Flag(t *testing.T) { app.Flags = []cli.Flag{ cli.Float64Flag{Name: "height", Value: 1.5, Usage: "Set the height, in meters"}, } - app.Action = func(c *cli.Context) { + app.Action = func(c *cli.Context) error { meters = c.Float64("height") + return nil } app.Run([]string{"", "--height", "1.93"}) @@ -219,11 +227,12 @@ func TestApp_ParseSliceFlags(t *testing.T) { cli.IntSliceFlag{Name: "p", Value: &cli.IntSlice{}, Usage: "set one or more ip addr"}, cli.StringSliceFlag{Name: "ip", Value: &cli.StringSlice{}, Usage: "set one or more ports to open"}, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { parsedIntSlice = c.IntSlice("p") parsedStringSlice = c.StringSlice("ip") parsedOption = c.String("option") firstArg = c.Args().First() + return nil }, } app.Commands = []cli.Command{command} @@ -285,8 +294,9 @@ func TestApp_BeforeFunc(t *testing.T) { app.Commands = []cli.Command{ cli.Command{ Name: "sub", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { subcommandRun = true + return nil }, }, } @@ -381,8 +391,9 @@ func TestAppCommandNotFound(t *testing.T) { app.Commands = []cli.Command{ cli.Command{ Name: "bar", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { subcommandRun = true + return nil }, }, } @@ -407,10 +418,11 @@ func TestGlobalFlagsInSubcommands(t *testing.T) { Subcommands: []cli.Command{ { Name: "bar", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { if c.GlobalBool("debug") { subcommandRun = true } + return nil }, }, }, diff --git a/cli_test.go b/cli_test.go index 879a793..e330810 100644 --- a/cli_test.go +++ b/cli_test.go @@ -15,16 +15,18 @@ func Example() { Name: "add", ShortName: "a", Usage: "add a task to the list", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("added task: ", c.Args().First()) + return nil }, }, { Name: "complete", ShortName: "c", Usage: "complete a task on the list", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("completed task: ", c.Args().First()) + return nil }, }, } @@ -54,8 +56,9 @@ func ExampleSubcommand() { Usage: "Name of the person to greet", }, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("Hello, ", c.String("name")) + return nil }, }, { Name: "spanish", @@ -68,8 +71,9 @@ func ExampleSubcommand() { Usage: "Surname of the person to greet", }, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("Hola, ", c.String("surname")) + return nil }, }, { Name: "french", @@ -82,16 +86,18 @@ func ExampleSubcommand() { Usage: "Nickname of the person to greet", }, }, - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("Bonjour, ", c.String("nickname")) + return nil }, }, }, }, { Name: "bye", Usage: "says goodbye", - Action: func(c *cli.Context) { + Action: func(c *cli.Context) error { println("bye") + return nil }, }, } diff --git a/command.go b/command.go index 5622b38..362db59 100644 --- a/command.go +++ b/command.go @@ -22,7 +22,7 @@ type Command struct { // If a non-nil error is returned, no sub-subcommands are run Before func(context *Context) error // The function to call when this command is invoked - Action func(context *Context) + Action func(context *Context) error // List of child commands Subcommands []Command // List of flags to parse @@ -98,8 +98,7 @@ func (c Command) Run(ctx *Context) error { return nil } context.Command = c - c.Action(context) - return nil + return c.Action(context) } // Returns true if Command.Name or Command.ShortName matches given name diff --git a/command_test.go b/command_test.go index c0f556a..06c2203 100644 --- a/command_test.go +++ b/command_test.go @@ -20,7 +20,7 @@ func TestCommandDoNotIgnoreFlags(t *testing.T) { ShortName: "tc", Usage: "this is for testing", Description: "testing", - Action: func(_ *cli.Context) {}, + Action: func(_ *cli.Context) error { return nil }, } err := command.Run(c) @@ -40,7 +40,7 @@ func TestCommandIgnoreFlags(t *testing.T) { ShortName: "tc", Usage: "this is for testing", Description: "testing", - Action: func(_ *cli.Context) {}, + Action: func(_ *cli.Context) error { return nil }, SkipFlagParsing: true, } err := command.Run(c) diff --git a/flag_test.go b/flag_test.go index bc5059c..9aa1cff 100644 --- a/flag_test.go +++ b/flag_test.go @@ -297,13 +297,14 @@ func TestParseMultiString(t *testing.T) { Flags: []cli.Flag{ cli.StringFlag{Name: "serve, s"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.String("serve") != "10" { t.Errorf("main name not set") } if ctx.String("s") != "10" { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run", "-s", "10"}) } @@ -314,13 +315,14 @@ func TestParseMultiStringFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.StringFlag{Name: "count, c", EnvVar: "APP_COUNT"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.String("count") != "20" { t.Errorf("main name not set") } if ctx.String("c") != "20" { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run"}) } @@ -330,13 +332,14 @@ func TestParseMultiStringSlice(t *testing.T) { Flags: []cli.Flag{ cli.StringSliceFlag{Name: "serve, s", Value: &cli.StringSlice{}}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"10", "20"}) { t.Errorf("main name not set") } if !reflect.DeepEqual(ctx.StringSlice("s"), []string{"10", "20"}) { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run", "-s", "10", "-s", "20"}) } @@ -348,13 +351,14 @@ func TestParseMultiStringSliceFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.StringSliceFlag{Name: "intervals, i", Value: &cli.StringSlice{}, EnvVar: "APP_INTERVALS"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.StringSlice("i"), []string{"20", "30", "40"}) { t.Errorf("short name not set from env") } + return nil }, }).Run([]string{"run"}) } @@ -364,13 +368,14 @@ func TestParseMultiInt(t *testing.T) { Flags: []cli.Flag{ cli.IntFlag{Name: "serve, s"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Int("serve") != 10 { t.Errorf("main name not set") } if ctx.Int("s") != 10 { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run", "-s", "10"}) @@ -382,13 +387,14 @@ func TestParseMultiIntFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.IntFlag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Int("timeout") != 10 { t.Errorf("main name not set") } if ctx.Int("t") != 10 { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run"}) @@ -399,13 +405,14 @@ func TestParseMultiIntSlice(t *testing.T) { Flags: []cli.Flag{ cli.IntSliceFlag{Name: "serve, s", Value: &cli.IntSlice{}}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{10, 20}) { t.Errorf("main name not set") } if !reflect.DeepEqual(ctx.IntSlice("s"), []int{10, 20}) { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run", "-s", "10", "-s", "20"}) } @@ -417,13 +424,14 @@ func TestParseMultiIntSliceFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.IntSliceFlag{Name: "intervals, i", Value: &cli.IntSlice{}, EnvVar: "APP_INTERVALS"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.IntSlice("i"), []int{20, 30, 40}) { t.Errorf("short name not set from env") } + return nil }, }).Run([]string{"run"}) } @@ -433,13 +441,14 @@ func TestParseMultiFloat64(t *testing.T) { Flags: []cli.Flag{ cli.Float64Flag{Name: "serve, s"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Float64("serve") != 10.2 { t.Errorf("main name not set") } if ctx.Float64("s") != 10.2 { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run", "-s", "10.2"}) @@ -451,13 +460,14 @@ func TestParseMultiFloat64FromEnv(t *testing.T) { Flags: []cli.Flag{ cli.Float64Flag{Name: "timeout, t", EnvVar: "APP_TIMEOUT_SECONDS"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Float64("timeout") != 15.5 { t.Errorf("main name not set") } if ctx.Float64("t") != 15.5 { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run"}) @@ -468,13 +478,14 @@ func TestParseMultiBool(t *testing.T) { Flags: []cli.Flag{ cli.BoolFlag{Name: "serve, s"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Bool("serve") != true { t.Errorf("main name not set") } if ctx.Bool("s") != true { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run", "--serve"}) @@ -486,13 +497,14 @@ func TestParseMultiBoolFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.BoolFlag{Name: "debug, d", EnvVar: "APP_DEBUG"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.Bool("debug") != true { t.Errorf("main name not set from env") } if ctx.Bool("d") != true { t.Errorf("short name not set from env") } + return nil }, } a.Run([]string{"run"}) @@ -503,13 +515,14 @@ func TestParseMultiBoolT(t *testing.T) { Flags: []cli.Flag{ cli.BoolTFlag{Name: "serve, s"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.BoolT("serve") != true { t.Errorf("main name not set") } if ctx.BoolT("s") != true { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run", "--serve"}) @@ -521,13 +534,14 @@ func TestParseMultiBoolTFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.BoolTFlag{Name: "debug, d", EnvVar: "APP_DEBUG"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if ctx.BoolT("debug") != false { t.Errorf("main name not set from env") } if ctx.BoolT("d") != false { t.Errorf("short name not set from env") } + return nil }, } a.Run([]string{"run"}) @@ -556,13 +570,14 @@ func TestParseGeneric(t *testing.T) { Flags: []cli.Flag{ cli.GenericFlag{Name: "serve, s", Value: &Parser{}}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.Generic("serve"), &Parser{"10", "20"}) { t.Errorf("main name not set") } if !reflect.DeepEqual(ctx.Generic("s"), &Parser{"10", "20"}) { t.Errorf("short name not set") } + return nil }, } a.Run([]string{"run", "-s", "10,20"}) @@ -574,13 +589,14 @@ func TestParseGenericFromEnv(t *testing.T) { Flags: []cli.Flag{ cli.GenericFlag{Name: "serve, s", Value: &Parser{}, EnvVar: "APP_SERVE"}, }, - Action: func(ctx *cli.Context) { + Action: func(ctx *cli.Context) error { if !reflect.DeepEqual(ctx.Generic("serve"), &Parser{"20", "30"}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.Generic("s"), &Parser{"20", "30"}) { t.Errorf("short name not set from env") } + return nil }, } a.Run([]string{"run"}) diff --git a/help.go b/help.go index 5020cb6..4997dde 100644 --- a/help.go +++ b/help.go @@ -69,13 +69,14 @@ var helpCommand = Command{ Name: "help", ShortName: "h", Usage: "Shows a list of commands or help for one command", - Action: func(c *Context) { + Action: func(c *Context) error { args := c.Args() if args.Present() { ShowCommandHelp(c, args.First()) } else { ShowAppHelp(c) } + return nil }, } @@ -83,13 +84,14 @@ var helpSubcommand = Command{ Name: "help", ShortName: "h", Usage: "Shows a list of commands or help for one command", - Action: func(c *Context) { + Action: func(c *Context) error { args := c.Args() if args.Present() { ShowCommandHelp(c, args.First()) } else { ShowSubcommandHelp(c) } + return nil }, }