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

@@ -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 a.Action(context)
-	return nil
 }
 
 // Another entry point to the cli app, takes care of passing arguments and error handling

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
 					},
 				},
 			},

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
 			},
 		},
 	}

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 c.Action(context)
-	return nil
 }
 
 // Returns true if Command.Name or Command.ShortName matches given name

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)

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"})

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
 	},
 }