Skip to content

Commit

Permalink
Only look for help and version flags in the first argument
Browse files Browse the repository at this point in the history
" mow.cli" used to search for -h, --help, -v, --version, ... shortcuts
in all the arguments.
This causes an issue with `--` (all what follows is an arg), e.g.:

```
app -- -h
```

Would still display the app help instead of treating `-h` as the
argument it is.

Also, this PR changes how the library handles encountering help and
version flags.
Instead of calling os.exit, the value of `errorHandling` is now used to
decide.
If it is `ContinueOnError`, than exit is not called and an nil error is
returned from `app.Run`.
The old behaviour is still available with `ExitOnError`.

Fixes #41
  • Loading branch information
jawher committed Aug 15, 2017
1 parent 82aefbe commit 636e26b
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 36 deletions.
4 changes: 2 additions & 2 deletions cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,14 @@ func (cli *Cli) parse(args []string, entry, inFlow, outFlow *step) error {
// After that, we just call Cmd.parse() for the default behavior
if cli.versionSetAndRequested(args) {
cli.PrintVersion()
exiter(0)
cli.onError(errVersionRequested)
return nil
}
return cli.Cmd.parse(args, entry, inFlow, outFlow)
}

func (cli *Cli) versionSetAndRequested(args []string) bool {
return cli.version != nil && cli.isArgSet(args, cli.version.option.names)
return cli.version != nil && cli.isFlagSet(args, cli.version.option.names)
}

/*
Expand Down
92 changes: 83 additions & 9 deletions cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,33 +47,44 @@ func TestImplicitSpec(t *testing.T) {
require.True(t, called, "Exec wasn't called")
}

func TestHelpShortcut(t *testing.T) {
func testHelpAndVersionWithOptionsEnd(flag string, t *testing.T) {
t.Logf("Testing help/version with --: flag=%q", flag)
defer suppressOutput()()

exitCalled := false
defer exitShouldBeCalledWith(t, 2, &exitCalled)()
defer exitShouldBeCalledWith(t, 0, &exitCalled)()

app := App("x", "")
app.Spec = "Y"
app.Version("v version", "1.0")
app.Spec = "CMD"

app.String(StringArg{Name: "Y", Value: "", Desc: ""})
cmd := app.String(StringArg{Name: "CMD", Value: "", Desc: ""})

actionCalled := false
app.Action = func() {
actionCalled = true
require.Equal(t, flag, *cmd)
}
app.Run([]string{"x", "y", "-h", "z"})

require.False(t, actionCalled, "action should not have been called")
require.True(t, exitCalled, "exit should have been called")
app.Run([]string{"x", "--", flag})

require.True(t, actionCalled, "action should have been called")
require.False(t, exitCalled, "exit should not have been called")

}

func TestHelpAndVersionWithOptionsEnd(t *testing.T) {
for _, flag := range []string{"-h", "--help", "-v", "--version"} {
testHelpAndVersionWithOptionsEnd(flag, t)
}
}

func TestHelpMessage(t *testing.T) {
var out, err string
defer captureAndRestoreOutput(&out, &err)()

exitCalled := false
defer exitShouldBeCalledWith(t, 2, &exitCalled)()
defer exitShouldBeCalledWith(t, 0, &exitCalled)()

app := App("app", "App Desc")
app.Spec = "[-o] ARG"
Expand Down Expand Up @@ -104,7 +115,7 @@ func TestLongHelpMessage(t *testing.T) {
defer captureAndRestoreOutput(&out, &err)()

exitCalled := false
defer exitShouldBeCalledWith(t, 2, &exitCalled)()
defer exitShouldBeCalledWith(t, 0, &exitCalled)()

app := App("app", "App Desc")
app.LongDesc = "Longer App Desc"
Expand Down Expand Up @@ -190,20 +201,83 @@ func TestContinueOnError(t *testing.T) {
require.False(t, called, "Exec should NOT have been called")
}

func TestContinueOnErrorWithHelpAndVersion(t *testing.T) {
defer exitShouldNotCalled(t)()
defer suppressOutput()()

app := App("say", "")
app.Version("v", "1.0")
app.String(StringOpt{Name: "f", Value: "", Desc: ""})
app.Spec = "-f"
app.ErrorHandling = flag.ContinueOnError
called := false
app.Action = func() {
called = true
}

{
err := app.Run([]string{"say", "-h"})
require.Nil(t, err)
require.False(t, called, "Exec should NOT have been called")
}

{
err := app.Run([]string{"say", "-v"})
require.Nil(t, err)
require.False(t, called, "Exec should NOT have been called")
}

}

func TestExitOnError(t *testing.T) {
defer suppressOutput()()

exitCalled := false
defer exitShouldBeCalledWith(t, 2, &exitCalled)()

app := App("x", "")
app.ErrorHandling = flag.ExitOnError
app.Spec = "Y"

app.String(StringArg{Name: "Y", Value: "", Desc: ""})

app.Run([]string{"x", "y", "z"})
require.True(t, exitCalled, "exit should have been called")
}

func TestExitOnErrorWithHelp(t *testing.T) {
defer suppressOutput()()

exitCalled := false
defer exitShouldBeCalledWith(t, 0, &exitCalled)()

app := App("x", "")
app.Spec = "Y"
app.ErrorHandling = flag.ExitOnError

app.String(StringArg{Name: "Y", Value: "", Desc: ""})

app.Run([]string{"x", "-h"})
require.True(t, exitCalled, "exit should have been called")
}

func TestExitOnErrorWithVersion(t *testing.T) {
defer suppressOutput()()

exitCalled := false
defer exitShouldBeCalledWith(t, 0, &exitCalled)()

app := App("x", "")
app.Version("v", "1.0")
app.Spec = "Y"
app.ErrorHandling = flag.ExitOnError

app.String(StringArg{Name: "Y", Value: "", Desc: ""})

app.Run([]string{"x", "-v"})
require.True(t, exitCalled, "exit should have been called")
}

func TestPanicOnError(t *testing.T) {
defer suppressOutput()()

Expand Down
50 changes: 25 additions & 25 deletions commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,18 +275,20 @@ func (c *Cmd) doInit() error {
}

func (c *Cmd) onError(err error) {
if err != nil {
switch c.ErrorHandling {
case flag.ExitOnError:
exiter(2)
case flag.PanicOnError:
panic(err)
}
} else {
if err == errHelpRequested || err == errVersionRequested {
if c.ErrorHandling == flag.ExitOnError {
exiter(2)
exiter(0)
}
return
}

switch c.ErrorHandling {
case flag.ExitOnError:
exiter(2)
case flag.PanicOnError:
panic(err)
}

}

/*
Expand Down Expand Up @@ -401,7 +403,7 @@ func (c *Cmd) formatDescription(desc, envVar string) string {
func (c *Cmd) parse(args []string, entry, inFlow, outFlow *step) error {
if c.helpRequested(args) {
c.PrintLongHelp()
c.onError(nil)
c.onError(errHelpRequested)
return nil
}

Expand Down Expand Up @@ -471,26 +473,24 @@ func (c *Cmd) parse(args []string, entry, inFlow, outFlow *step) error {

}

func (c *Cmd) isArgSet(args []string, searchArgs []string) bool {
for _, arg := range args {
for _, sub := range c.commands {
if sub.isAlias(arg) {
return false
}
}
for _, searchArg := range searchArgs {
if arg == searchArg {
return true
}
func (c *Cmd) helpRequested(args []string) bool {
return c.isFlagSet(args, []string{"-h", "--help"})
}

func (c *Cmd) isFlagSet(args []string, searchArgs []string) bool {
if len(args) == 0 {
return false
}

arg := args[0]
for _, searchArg := range searchArgs {
if arg == searchArg {
return true
}
}
return false
}

func (c *Cmd) helpRequested(args []string) bool {
return c.isArgSet(args, []string{"-h", "--help"})
}

func (c *Cmd) getOptsAndArgs(args []string) int {
consumed := 0

Expand Down
10 changes: 10 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package cli

import (
"errors"
)

var (
errHelpRequested = errors.New("Help requested")
errVersionRequested = errors.New("Version requested")
)

0 comments on commit 636e26b

Please sign in to comment.