Skip to content

Commit

Permalink
Exit non-zero if a unknown subcommand is given
Browse files Browse the repository at this point in the history
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 urfave#276
  • Loading branch information
jszwedko committed May 8, 2016
1 parent d3a4d54 commit 592f1d9
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 19 deletions.
10 changes: 7 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 16 additions & 16 deletions help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
}
Expand All @@ -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)
},
}

Expand All @@ -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
Expand Down
58 changes: 58 additions & 0 deletions help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package cli

import (
"bytes"
"flag"
"strings"
"testing"
)

Expand Down Expand Up @@ -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)
}
}

0 comments on commit 592f1d9

Please sign in to comment.