Repository: jawher/mow.cli (go)

Issue:
Global StringArg + command: contextual help does not work
Hi. First of all, thanks for this excellent package. I hope it will be developed further because it shows an enormous potential.

With the following example code (simplified from docs):

```go
package main

import (
	"fmt"
	"os"
	cli "github.com/jawher/mow.cli"
)

var filename *string

func main() {
	app := cli.App("app", "description")

	filename = app.StringArg("FILENAME", "", "a string argument")

	app.Command("command1", "this should show with -h", command1)

	app.Run(os.Args)
}

func command1(cmd *cli.Cmd) {
	cmd.Action = func() {
		fmt.Printf("Hello from command1")
	}
}
```
I see the following behavior:
```
./app command1 --help
Error: incorrect usage

Usage: app FILENAME COMMAND [arg...]

description
               
Arguments:     
  FILENAME     a string argument
               
Commands:      
  command1     this should show with -h
               
Run 'app COMMAND --help' for more information on a command.
```
I would expect that the contextual help worked here, or is this the intended semantics? I couldn't find anything about this in the documentation, and the sole example combining top level parameters with commands used options instead of string arguments.

In case this is the intended semantics, where can I find the rationale in docs? And if not, and you are kind enough to point me around the code that deals with this part, I hope I can be able to submit a patch (I will at least give it a try).

Thanks!

Pull request:
Skip validation in parents when help is requested for a command
For example:

* The app has a required argument
* The app has a command

If the program is invoked with:

`app command --help`

Before the fix:

An incorrect usage error will be printed, with the help message of the app

After the fix:

The help message for the command will be printed.

Fixes #118


Patch:
diff --git a/.gitignore b/.gitignore
index c033e7f..dfa4802 100644
--- a/.gitignore
+++ b/.gitignore
@@ -4,3 +4,4 @@ coverage.out
 README.generated.md
 README.original.md
 bin/
+.idea/
diff --git a/cli.go b/cli.go
index 4b0788d..6bce342 100644
--- a/cli.go
+++ b/cli.go
@@ -77,7 +77,7 @@ func (cli *Cli) parse(args []string, entry, inFlow, outFlow *flow.Step) error {
 }
 
 func (cli *Cli) versionSetAndRequested(args []string) bool {
-	return cli.version != nil && cli.isFlagSet(args, cli.version.option.Names)
+	return cli.version != nil && cli.isFirstItemAmong(args, cli.version.option.Names)
 }
 
 /*
diff --git a/commands.go b/commands.go
index 2bcb335..99b43ba 100644
--- a/commands.go
+++ b/commands.go
@@ -648,13 +648,32 @@ func formatEnvVarsForHelp(envVars string) string {
 }
 
 func (c *Cmd) parse(args []string, entry, inFlow, outFlow *flow.Step) error {
-	if c.helpRequested(args) {
+	helpIndex := c.helpIndex(args)
+	nargsLen := c.getOptsAndArgs(args)
+
+	if helpIndex >= 0 && helpIndex < nargsLen {
 		c.PrintLongHelp()
 		c.onError(errHelpRequested)
 		return nil
 	}
 
-	nargsLen := c.getOptsAndArgs(args)
+	// help was requested, but not for this command, skip the validation
+	if helpIndex >= 0 {
+		arg := args[nargsLen]
+		for _, sub := range c.commands {
+			if !sub.isAlias(arg) {
+				continue
+			}
+
+			if err := sub.doInit(); err != nil {
+				panic(err)
+			}
+
+			return sub.parse(args[nargsLen+1:], entry, nil, nil)
+		}
+		// impossible case
+		panic("wut")
+	}
 
 	if err := c.fsm.Parse(args[:nargsLen]); err != nil {
 		fmt.Fprintf(stdErr, "Error: %s\n", err.Error())
@@ -723,17 +742,28 @@ func (c *Cmd) parse(args []string, entry, inFlow, outFlow *flow.Step) error {
 
 }
 
-func (c *Cmd) helpRequested(args []string) bool {
-	return c.isFlagSet(args, []string{"-h", "--help"})
+func (c *Cmd) helpIndex(args []string) int {
+	searchSet := []string{"-h", "--help"}
+	for i, arg := range args {
+		if arg == "--" {
+			return -1
+		}
+		for _, searchArg := range searchSet {
+			if arg == searchArg {
+				return i
+			}
+		}
+	}
+	return -1
 }
 
-func (c *Cmd) isFlagSet(args []string, searchArgs []string) bool {
+func (c *Cmd) isFirstItemAmong(args []string, searchSet []string) bool {
 	if len(args) == 0 {
 		return false
 	}
 
 	arg := args[0]
-	for _, searchArg := range searchArgs {
+	for _, searchArg := range searchSet {
 		if arg == searchArg {
 			return true
 		}
diff --git a/go.mod b/go.mod
index 04ddba2..6fb5f19 100644
--- a/go.mod
+++ b/go.mod
@@ -1,7 +1,7 @@
 module github.com/jawher/mow.cli
 
 require (
-	github.com/stretchr/objx v0.2.0 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
 	github.com/stretchr/testify v1.4.0
 	gopkg.in/yaml.v2 v2.2.5 // indirect
 )
diff --git a/go.sum b/go.sum
index be96aa5..553229b 100644
--- a/go.sum
+++ b/go.sum
@@ -3,16 +3,11 @@ github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
-github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
-github.com/stretchr/objx v0.2.0 h1:Hbg2NidpLE8veEBkEZTL3CvlkUIVzuU9jDplZO54c48=
-github.com/stretchr/objx v0.2.0/go.mod h1:qt09Ya8vawLte6SNmTgCsAVtYtaKzEcn8ATUoHMkEqE=
-github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
-github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
 github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
-gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.5 h1:ymVxjfMaHvXD8RqPRmzHHsB3VvucivSkIAvJFDI5O3c=
 gopkg.in/yaml.v2 v2.2.5/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=


Test patch:
diff --git a/cli_test.go b/cli_test.go
index 6617d16..8e6ec49 100644
--- a/cli_test.go
+++ b/cli_test.go
@@ -752,36 +752,111 @@ func TestAppWithFloats64Arg(t *testing.T) {
 	}
 }
 
-func testHelpAndVersionWithOptionsEnd(flag string, t *testing.T) {
-	t.Logf("Testing help/version with --: flag=%q", flag)
-	defer suppressOutput()()
+func TestHelpCommandSkipsValidation(t *testing.T) {
+	t.Run("1 level deep", func(t *testing.T) {
+		for _, args := range [][]string{
+			{"app", "command", "-h"},
+			{"app", "--opt1", "command", "-h"},
+			{"app", "--opt1", "value1", "command", "-h"},
+			{"app", "--opt2", "command", "-h"},
+			{"app", "--opt2", "--opt3", "command", "-h"},
+			{"app", "--wrong", "command", "-h"},
+		} {
+			t.Run(fmt.Sprintf("%v", args), func(t *testing.T) {
+				app := App("app", "app desc")
+				app.StringArg("ARG", "", "arg desc")
+				app.StringOpt("opt1", "", "opt1 desc")
+				app.BoolOpt("opt2", false, "opt2 desc")
+				app.BoolOpt("opt3", false, "opt3 desc")
+				app.Command("command", "command desc", func(cmd *Cmd) {})
+
+				var out, stdErr string
+				defer captureAndRestoreOutput(&out, &stdErr)()
+
+				exitCalled := false
+				defer exitShouldBeCalledWith(t, 0, &exitCalled)()
 
-	exitCalled := false
-	defer exitShouldBeCalledWith(t, 0, &exitCalled)()
+				require.NoError(t,
+					// calling help on a command should skip validating the parents required arguments
+					app.Run(args))
 
-	app := App("x", "")
-	app.Version("v version", "1.0")
-	app.Spec = "CMD"
+				require.Equal(t, `
+Usage: app command
+
+command desc
+`, stdErr)
+			})
+		}
+	})
 
-	cmd := app.String(StringArg{Name: "CMD", Value: "", Desc: ""})
+	t.Run("2 level deep", func(t *testing.T) {
+		for _, args := range [][]string{
+			{"app", "command", "child", "-h"},
+			{"app", "--opt1", "command", "child", "-h"},
+			{"app", "--opt1", "value1", "command", "child", "-h"},
+			{"app", "--opt2", "command", "child", "-h"},
+			{"app", "--opt2", "--opt3", "command", "child", "-h"},
+			{"app", "--wrong", "command", "child", "-h"},
+		} {
+			t.Run(fmt.Sprintf("%v", args), func(t *testing.T) {
+				app := App("app", "app desc")
+				app.StringArg("ARG1", "", "arg1 desc")
+				app.StringOpt("opt1", "", "opt1 desc")
+				app.BoolOpt("opt2", false, "opt2 desc")
+				app.BoolOpt("opt3", false, "opt3 desc")
+
+				app.Command("command", "command desc", func(cmd *Cmd) {
+					cmd.StringArg("ARG2", "", "arg2 desc")
+
+					cmd.Command("child", "child desc", func(cmd *Cmd) {})
+				})
 
-	actionCalled := false
-	app.Action = func() {
-		actionCalled = true
-		require.Equal(t, flag, *cmd)
-	}
+				var out, stdErr string
+				defer captureAndRestoreOutput(&out, &stdErr)()
 
-	require.NoError(t,
-		app.Run([]string{"x", "--", flag}))
+				exitCalled := false
+				defer exitShouldBeCalledWith(t, 0, &exitCalled)()
 
-	require.True(t, actionCalled, "action should have been called")
-	require.False(t, exitCalled, "exit should not have been called")
+				require.NoError(t,
+					// calling help on a command should skip validating the parents required arguments
+					app.Run(args))
+
+				require.Equal(t, `
+Usage: app command child
+
+child desc
+`, stdErr)
+			})
+		}
+	})
 }
 
 func TestHelpAndVersionWithOptionsEnd(t *testing.T) {
-	for _, flag := range []string{"-h", "--help", "-v", "--version"} {
-		t.Run(flag, func(t *testing.T) {
-			testHelpAndVersionWithOptionsEnd(flag, t)
+	for _, opt := range []string{"-h", "--help", "-v", "--version"} {
+		t.Run(opt, func(t *testing.T) {
+			t.Logf("Testing help/version with --: opt=%q", opt)
+			defer suppressOutput()()
+
+			exitCalled := false
+			defer exitShouldBeCalledWith(t, 0, &exitCalled)()
+
+			app := App("x", "")
+			app.Version("v version", "1.0")
+			app.Spec = "CMD"
+
+			cmd := app.String(StringArg{Name: "CMD", Value: "", Desc: ""})
+
+			actionCalled := false
+			app.Action = func() {
+				actionCalled = true
+				require.Equal(t, opt, *cmd)
+			}
+
+			require.NoError(t,
+				app.Run([]string{"x", "--", opt}))
+
+			require.True(t, actionCalled, "action should have been called")
+			require.False(t, exitCalled, "exit should not have been called")
 		})
 	}
 }
@@ -876,7 +951,7 @@ func TestHelpMessage(t *testing.T) {
 				cmd.Hidden = true
 			})
 
-			fmt.Printf("calling app with %+v\n", cas.params)
+			t.Logf("calling app with %+v", cas.params)
 			require.NoError(t,
 				app.Run(cas.params))