From 271b56c71b46d26543a729132cb7a0ecae5d2211 Mon Sep 17 00:00:00 2001
From: Dan Buch <daniel.buch@gmail.com>
Date: Thu, 28 Apr 2016 10:15:04 -0400
Subject: [PATCH] Cleanups based on feedback in #361

---
 CHANGELOG.md | 11 +++++++++++
 app.go       | 24 ++++++++++++++++--------
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 074bfb2..b03b52c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,17 @@
 
 ### Added
 - Support for placeholders in flag usage strings
+- Support for custom exit code by returning an error that fulfills
+`cli.ExitCoder` to `cli.App.Run`.
+
+### Deprecated
+- <a name="deprecated-cli-app-runandexitonerror"></a>
+`cli.App.RunAndExitOnError`, which should now be done by returning an error
+that fulfills `cli.ExitCoder` to `cli.App.Run`.
+- <a name="deprecated-cli-app-action-signature"></a>
+the legacy signature for `cli.App.Action` of `func(*cli.Context)`, which should
+now have a signature of `func(*cli.Context) error`, as defined by
+`cli.ActionFunc`.
 
 ## [1.14.0] - 2016-04-03 (backfilled 2016-04-25)
 ### Added
diff --git a/app.go b/app.go
index fac5fd9..c9830d4 100644
--- a/app.go
+++ b/app.go
@@ -11,6 +11,15 @@ import (
 	"time"
 )
 
+var (
+	errNonFuncAction = NewExitError("ERROR invalid Action type.  "+
+		"Must be a func of type `cli.ActionFunc`.  "+
+		"See https://github.com/codegangsta/cli/blob/master/CHANGELOG.md#deprecated-cli-app-action-signature", 2)
+	errInvalidActionSignature = NewExitError("ERROR invalid Action signature.  "+
+		"Must be `cli.ActionFunc`.  "+
+		"See https://github.com/codegangsta/cli/blob/master/CHANGELOG.md#deprecated-cli-app-action-signature", 2)
+)
+
 // App is the main structure of a cli application. It is recommended that
 // an app be created with the cli.NewApp() function
 type App struct {
@@ -215,8 +224,8 @@ func (a *App) Run(arguments []string) (err error) {
 // DEPRECATED: Another entry point to the cli app, takes care of passing arguments and error handling
 func (a *App) RunAndExitOnError() {
 	fmt.Fprintln(os.Stderr,
-		"DEPRECATED method `*cli.App.RunAndExitOnError`.  "+
-			"Use `*cli.App.Run` with an error type that fulfills `cli.ExitCoder`.")
+		"DEPRECATED cli.App.RunAndExitOnError.  "+
+			"See https://github.com/codegangsta/cli/blob/master/CHANGELOG.md#deprecated-cli-app-runandexitonerror")
 	if err := a.Run(os.Args); err != nil {
 		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)
@@ -382,11 +391,12 @@ func (a Author) String() string {
 }
 
 // HandleAction uses ✧✧✧reflection✧✧✧ to figure out if the given Action is an
-// ActionFunc, LegacyActionFunc, or some other invalid thing.  If it's an
-// ActionFunc or LegacyActionFunc, the func is run!
+// 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!
 func HandleAction(action interface{}, context *Context) error {
 	if reflect.TypeOf(action).Kind() != reflect.Func {
-		panic("given Action must be a func")
+		return errNonFuncAction
 	}
 
 	vals := reflect.ValueOf(action).Call([]reflect.Value{reflect.ValueOf(context)})
@@ -398,9 +408,7 @@ func HandleAction(action interface{}, context *Context) error {
 	}
 
 	if len(vals) > 1 {
-		fmt.Fprintln(os.Stderr,
-			"ERROR invalid Action signature.  Must be `cli.ActionFunc`")
-		panic("given Action has invalid return signature")
+		return errInvalidActionSignature
 	}
 
 	if err, ok := reflect.ValueOf(vals[0]).Interface().(error); ok {