Only look for help and version flags in the first argument (#42)

" 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

cli.go

@@ -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)
 }
 
 /*

cli_test.go

@@ -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"
@@ -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"
@@ -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()()
 

commands.go

@@ -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)
  }
+
 }
 
 /*
@@ -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
  }
 
@@ -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
 

errors.go

@@ -0,0 +1,10 @@
+package cli
+
+import (
+ "errors"
+)
+
+var (
+ errHelpRequested = errors.New("Help requested")
+ errVersionRequested = errors.New("Version requested")
+)