From 592f1d97e5a73a1f446de8675c01c9dcbfc3f6a5 Mon Sep 17 00:00:00 2001
From: Jesse Szwedko <jesse.szwedko@gmail.com>
Date: Sat, 7 May 2016 17:22:09 -0700
Subject: [PATCH] Exit non-zero if a unknown subcommand is given

Currently it just prints the help message and exits 0.

We do this by modifying the helpCommand and helpSubcommand cli.Commands
to return an error if they are called with an unknown subcommand. This
propogates up to the app which exits with 3 and prints the error.

Thanks to @danslimmon for the initial approach!

Fixes #276
---
 CHANGELOG.md | 10 ++++++---
 help.go      | 32 ++++++++++++++---------------
 help_test.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 19 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index d7ea0a6..d4de0b9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,11 +8,15 @@
 
 ### Changed
 - `Float64Flag`, `IntFlag`, and `DurationFlag` default values are no longer
-quoted in help text output.
+  quoted in help text output.
 - All flag types now include `(default: {value})` strings following usage when a
-default value can be (reasonably) detected.
+  default value can be (reasonably) detected.
 - `IntSliceFlag` and `StringSliceFlag` usage strings are now more consistent
-with non-slice flag types
+  with non-slice flag types
+- Apps now exit with a code of 3 if an unknown subcommand is specified
+  (previously they printed "No help topic for...", but still exited 0. This
+  makes it easier to script around apps built using `cli` since they can trust
+  that a 0 exit code indicated a successful execution.
 
 ## [1.16.0] - 2016-05-02
 ### Added
diff --git a/help.go b/help.go
index 45e8603..259e452 100644
--- a/help.go
+++ b/help.go
@@ -81,10 +81,10 @@ var helpCommand = Command{
 	Action: func(c *Context) error {
 		args := c.Args()
 		if args.Present() {
-			ShowCommandHelp(c, args.First())
-		} else {
-			ShowAppHelp(c)
+			return ShowCommandHelp(c, args.First())
 		}
+
+		ShowAppHelp(c)
 		return nil
 	},
 }
@@ -97,11 +97,10 @@ var helpSubcommand = Command{
 	Action: func(c *Context) error {
 		args := c.Args()
 		if args.Present() {
-			ShowCommandHelp(c, args.First())
-		} else {
-			ShowSubcommandHelp(c)
+			return ShowCommandHelp(c, args.First())
 		}
-		return nil
+
+		return ShowSubcommandHelp(c)
 	},
 }
 
@@ -127,30 +126,31 @@ func DefaultAppComplete(c *Context) {
 }
 
 // Prints help for the given command
-func ShowCommandHelp(ctx *Context, command string) {
+func ShowCommandHelp(ctx *Context, command string) error {
 	// show the subcommand help for a command with subcommands
 	if command == "" {
 		HelpPrinter(ctx.App.Writer, SubcommandHelpTemplate, ctx.App)
-		return
+		return nil
 	}
 
 	for _, c := range ctx.App.Commands {
 		if c.HasName(command) {
 			HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c)
-			return
+			return nil
 		}
 	}
 
-	if ctx.App.CommandNotFound != nil {
-		ctx.App.CommandNotFound(ctx, command)
-	} else {
-		fmt.Fprintf(ctx.App.Writer, "No help topic for '%v'\n", command)
+	if ctx.App.CommandNotFound == nil {
+		return NewExitError(fmt.Sprintf("No help topic for '%v'", command), 3)
 	}
+
+	ctx.App.CommandNotFound(ctx, command)
+	return nil
 }
 
 // Prints help for the given subcommand
-func ShowSubcommandHelp(c *Context) {
-	ShowCommandHelp(c, c.Command.Name)
+func ShowSubcommandHelp(c *Context) error {
+	return ShowCommandHelp(c, c.Command.Name)
 }
 
 // Prints the version number of the App
diff --git a/help_test.go b/help_test.go
index ee5c25c..0fabdba 100644
--- a/help_test.go
+++ b/help_test.go
@@ -2,6 +2,8 @@ package cli
 
 import (
 	"bytes"
+	"flag"
+	"strings"
 	"testing"
 )
 
@@ -110,3 +112,59 @@ func Test_Version_Custom_Flags(t *testing.T) {
 		t.Errorf("unexpected output: %s", output.String())
 	}
 }
+
+func Test_helpCommand_Action_ErrorIfNoTopic(t *testing.T) {
+	app := NewApp()
+
+	set := flag.NewFlagSet("test", 0)
+	set.Parse([]string{"foo"})
+
+	c := NewContext(app, set, nil)
+
+	err := helpCommand.Action.(func(*Context) error)(c)
+
+	if err == nil {
+		t.Fatalf("expected error from helpCommand.Action(), but got nil")
+	}
+
+	exitErr, ok := err.(*ExitError)
+	if !ok {
+		t.Fatalf("expected ExitError from helpCommand.Action(), but instead got: %v", err.Error())
+	}
+
+	if !strings.HasPrefix(exitErr.Error(), "No help topic for") {
+		t.Fatalf("expected an unknown help topic error, but got: %v", exitErr.Error())
+	}
+
+	if exitErr.exitCode != 3 {
+		t.Fatalf("expected exit value = 3, got %d instead", exitErr.exitCode)
+	}
+}
+
+func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) {
+	app := NewApp()
+
+	set := flag.NewFlagSet("test", 0)
+	set.Parse([]string{"foo"})
+
+	c := NewContext(app, set, nil)
+
+	err := helpSubcommand.Action.(func(*Context) error)(c)
+
+	if err == nil {
+		t.Fatalf("expected error from helpCommand.Action(), but got nil")
+	}
+
+	exitErr, ok := err.(*ExitError)
+	if !ok {
+		t.Fatalf("expected ExitError from helpCommand.Action(), but instead got: %v", err.Error())
+	}
+
+	if !strings.HasPrefix(exitErr.Error(), "No help topic for") {
+		t.Fatalf("expected an unknown help topic error, but got: %v", exitErr.Error())
+	}
+
+	if exitErr.exitCode != 3 {
+		t.Fatalf("expected exit value = 3, got %d instead", exitErr.exitCode)
+	}
+}