Subcommand parsing logic stops at first flag

[christophermancini]

Noticed this behavior on an internal tool, essentially, if you have flags defined for a command, then execute a subcommand of that command with a flag before the subcommand, then it identifies the command as the subcommand.

For example, if we take the test cases @ https://github.com/mitchellh/cli/blob/master/cli_test.go#L1459-L1489 and change it to:

func TestCLISubcommand_nested(t *testing.T) {
	testCases := []struct {
		args       []string
		subcommand string
	}{
		{[]string{"bar"}, "bar"},
		{[]string{"foo", "-h"}, "foo"},
		{[]string{"-h", "bar"}, "bar"},
		{[]string{"foo", "bar", "-h"}, "foo bar"},
		{[]string{"foo", "bar", "baz", "-h"}, "foo bar baz"},
		{[]string{"foo", "bar", "-h", "baz"}, "foo bar"}, // expected match would be `foo bar baz`
		{[]string{"-h", "foo", "bar"}, "foo bar"},
	}

	for _, testCase := range testCases {
		cli := &CLI{
			Args: testCase.args,
			Commands: map[string]CommandFactory{
				"foo bar": func() (Command, error) {
					return new(MockCommand), nil
				},
				"foo bar baz": func() (Command, error) {
					return new(MockCommand), nil
				},
			},
		}
		result := cli.Subcommand()

		if result != testCase.subcommand {
			t.Errorf("Expected %#v, got %#v. Args: %#v",
				testCase.subcommand, result, testCase.args)
		}
	}
}

I guess the question is, with other tools like cobra which handle flags anywhere within the arguments, does it make sense to change this library's behavior?

[mitchellh]

Without having a deeper understanding of the flag package in use (since this library has no opinion on it, it just passes []string), we can’t tell whether -h is a single flag or if baz is an argument to -h. If you’re using the Go standard flag package, its allowed for -h baz to be an argument to -h.

However, this cli library DOES handle -h as a special case (for help). In that case, I think its a valid question if whether this should be help for foo bar baz… and I can see that being totally reasonable.

So, if this issue is talking about flags generally, I don’t think this behavior can be changed. But if we’re talking about the help flag specifically there is something here I think.

Please let me know if I’m misunderstanding.

[christophermancini]

Hey @mitchellh , the issue is focusing on flags generally, not specificallly -h which was used just as an example taken from the test case. I completely understand your point about the trailing argument possibly being an argument for the flag, however, in this case, it is a registered command.

The behavior I am proposing would be to identify foo bar baz as the subcommand when a flag comes before baz, e.g. foo bar --myflag baz, since foo bar baz was registered as a command for the CLI. If the arguments that follow the first flag are not part of a registered command, it continues to behave like it does today.

[apparentlymart]

FWIW over in Terraform we are intentionally making a distinction between options that appear before the command (global options) and options that appear after the command (command-specific options) and so I think I agree with @mitchellh that making this library itself try to recognize options prior to the command could have adverse affects on existing callers making different assumptions.

In the case of Terraform, we achieve this in a kinda-awkward way of pre-analyzing the original args, noticing the index of the first non-flag argument, and then splitting the args slice into a "global flags" portion and an "everything else" portion. We then pass the global flags portion to the Go flags package and the "everything else" portion to mitchellh/cli. The individual subcommands then in turn separately call the Go flags package again to analyze the command-specific options after mitchellh/cli already did the command handling. Given that, certain changes implemented inside mitchellh/cli might not be breaking for Terraform, since Terraform is already effectively "hiding" the global flags portion from mitchellh/cli.