Fix:(issue_976) All positional args and flags to be mixed in any order

command.go

@@ -591,24 +591,13 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) {
 			hasDefault := cmd.DefaultCommand != ""
 			isFlagName := checkStringSliceIncludes(name, cmd.FlagNames())
 
-			var (
-				isDefaultSubcommand   = false
-				defaultHasSubcommands = false
-			)
-
 			if hasDefault {
-				dc := cmd.Command(cmd.DefaultCommand)
-				defaultHasSubcommands = len(dc.Commands) > 0
-				for _, dcSub := range dc.Commands {
-					if checkStringSliceIncludes(name, dcSub.Names()) {
-						isDefaultSubcommand = true
-						break
-					}
-				}
+				tracef("using default command=%[1]q (cmd=%[2]q)", cmd.DefaultCommand, cmd.Name)
 			}
 
-			if isFlagName || (hasDefault && (defaultHasSubcommands && isDefaultSubcommand)) {
+			if isFlagName || hasDefault {
 				argsWithDefault := cmd.argsWithDefaultCommand(args)
+				tracef("using default command args=%[1]q (cmd=%[2]q)", argsWithDefault, cmd.Name)
 				if !reflect.DeepEqual(args, argsWithDefault) {
 					subCmd = cmd.Command(argsWithDefault.First())
 				}
@@ -656,7 +645,7 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) {
 		deferErr = cmd.handleExitCoder(ctx, err)
 	}
 
-	tracef("returning deferErr (cmd=%[1]q)", cmd.Name)
+	tracef("returning deferErr (cmd=%[1]q) %[2]q", cmd.Name, deferErr)
 	return deferErr
 }
 
@@ -802,14 +791,74 @@ func (cmd *Command) parseFlags(args Args) (Args, error) {
 	}
 
 	tracef("parsing flags iteratively tail=%[1]q (cmd=%[2]q)", args.Tail(), cmd.Name)
+	defer tracef("done parsing flags (cmd=%[1]q)", cmd.Name)
 
-	if err := parseIter(cmd.flagSet, cmd, args.Tail(), cmd.Root().shellCompletion); err != nil {
-		return cmd.Args(), err
-	}
+	rargs := args.Tail()
+	posArgs := []string{}
+	for {
+		tracef("rearrange:1 (cmd=%[1]q) %[2]q", cmd.Name, rargs)
+		for {
+			tracef("rearrange:2 (cmd=%[1]q) %[2]q %[3]q", cmd.Name, posArgs, rargs)
+
+			// no more args to parse. Break out of inner loop
+			if len(rargs) == 0 {
+				break
+			}
+
+			if strings.TrimSpace(rargs[0]) == "" {
+				break
+			}
+
+			// stop parsing once we see a "--"
+			if rargs[0] == "--" {
+				posArgs = append(posArgs, rargs...)
+				cmd.parsedArgs = &stringSliceArgs{posArgs}
+				return cmd.parsedArgs, nil
+			}
 
-	tracef("done parsing flags (cmd=%[1]q)", cmd.Name)
+			// let flagset parse this
+			if rargs[0][0] == '-' {
+				break
+			}
+
+			tracef("rearrange-3 (cmd=%[1]q) check %[2]q", cmd.Name, rargs[0])
+
+			// if there is a command by that name let the command handle the
+			// rest of the parsing
+			if cmd.Command(rargs[0]) != nil {
+				posArgs = append(posArgs, rargs...)
+				cmd.parsedArgs = &stringSliceArgs{posArgs}
+				return cmd.parsedArgs, nil
+			}
+
+			posArgs = append(posArgs, rargs[0])
+
+			// if this is the sole argument then
+			// break from inner loop
+			if len(rargs) == 1 {
+				rargs = []string{}
+				break
+			}
+
+			rargs = rargs[1:]
+		}
+		if err := parseIter(cmd.flagSet, cmd, rargs, cmd.Root().shellCompletion); err != nil {
+			posArgs = append(posArgs, cmd.flagSet.Args()...)
+			tracef("returning-1 (cmd=%[1]q) args %[2]q", cmd.Name, posArgs)
+			cmd.parsedArgs = &stringSliceArgs{posArgs}
+			return cmd.parsedArgs, err
+		}
+		tracef("rearrange-4 (cmd=%[1]q) check %[2]q", cmd.Name, cmd.flagSet.Args())
+		rargs = cmd.flagSet.Args()
+		if len(rargs) == 0 || strings.TrimSpace(rargs[0]) == "" || rargs[0] == "-" {
+			break
+		}
+	}
 
-	return cmd.Args(), nil
+	posArgs = append(posArgs, cmd.flagSet.Args()...)
+	tracef("returning-2 (cmd=%[1]q) args %[2]q", cmd.Name, posArgs)
+	cmd.parsedArgs = &stringSliceArgs{posArgs}
+	return cmd.parsedArgs, nil
 }
 
 // Names returns the names including short names and aliases.

command_test.go

@@ -160,10 +160,10 @@ func TestCommandFlagParsing(t *testing.T) {
 	}{
 		// Test normal "not ignoring flags" flow
 		{testArgs: []string{"test-cmd", "-break", "blah", "blah"}, skipFlagParsing: false, useShortOptionHandling: false, expectedErr: "flag provided but not defined: -break"},
-		{testArgs: []string{"test-cmd", "blah", "blah"}, skipFlagParsing: true, useShortOptionHandling: false},   // Test SkipFlagParsing without any args that look like flags
-		{testArgs: []string{"test-cmd", "blah", "-break"}, skipFlagParsing: true, useShortOptionHandling: false}, // Test SkipFlagParsing with random flag arg
-		{testArgs: []string{"test-cmd", "blah", "-help"}, skipFlagParsing: true, useShortOptionHandling: false},  // Test SkipFlagParsing with "special" help flag arg
-		{testArgs: []string{"test-cmd", "blah", "-h"}, skipFlagParsing: false, useShortOptionHandling: true},     // Test UseShortOptionHandling
+		{testArgs: []string{"test-cmd", "blah", "blah"}, skipFlagParsing: true, useShortOptionHandling: false},                                        // Test SkipFlagParsing without any args that look like flags
+		{testArgs: []string{"test-cmd", "blah", "-break"}, skipFlagParsing: true, useShortOptionHandling: false},                                      // Test SkipFlagParsing with random flag arg
+		{testArgs: []string{"test-cmd", "blah", "-help"}, skipFlagParsing: true, useShortOptionHandling: false},                                       // Test SkipFlagParsing with "special" help flag arg
+		{testArgs: []string{"test-cmd", "blah", "-h"}, skipFlagParsing: false, useShortOptionHandling: true, expectedErr: "No help topic for 'blah'"}, // Test UseShortOptionHandling
 	}
 
 	for _, c := range cases {
@@ -212,9 +212,9 @@ func TestParseAndRunShortOpts(t *testing.T) {
 		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
 		{testArgs: &stringSliceArgs{v: []string{"test", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
 		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
-		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "-invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "-invalid"}}},
-		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "--invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "--invalid"}}},
-		{testArgs: &stringSliceArgs{v: []string{"test", "-acfi", "not-arg", "arg1", "-invalid"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1", "-invalid"}}},
+		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
+		{testArgs: &stringSliceArgs{v: []string{"test", "-acf", "arg1", "--invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
+		{testArgs: &stringSliceArgs{v: []string{"test", "-acfi", "not-arg", "arg1", "-invalid"}}, expectedErr: "flag provided but not defined: -invalid"},
 		{testArgs: &stringSliceArgs{v: []string{"test", "-i", "ivalue"}}, expectedArgs: &stringSliceArgs{v: []string{}}},
 		{testArgs: &stringSliceArgs{v: []string{"test", "-i", "ivalue", "arg1"}}, expectedArgs: &stringSliceArgs{v: []string{"arg1"}}},
 		{testArgs: &stringSliceArgs{v: []string{"test", "-i"}}, expectedErr: "flag needs an argument: -i"},
@@ -639,9 +639,9 @@ func TestCommand_Command(t *testing.T) {
 }
 
 var defaultCommandTests = []struct {
-	cmdName    string
-	defaultCmd string
-	expected   bool
+	cmdName        string
+	defaultCmd     string
+	errNotExpected bool
 }{
 	{"foobar", "foobar", true},
 	{"batbaz", "foobar", true},
@@ -650,8 +650,8 @@ var defaultCommandTests = []struct {
 	{"", "foobar", true},
 	{"", "", true},
 	{" ", "", false},
-	{"bat", "batbaz", false},
-	{"nothing", "batbaz", false},
+	{"bat", "batbaz", true},
+	{"nothing", "batbaz", true},
 	{"nothing", "", false},
 }
 
@@ -668,7 +668,7 @@ func TestCommand_RunDefaultCommand(t *testing.T) {
 			}
 
 			err := cmd.Run(buildTestContext(t), []string{"c", test.cmdName})
-			if test.expected {
+			if test.errNotExpected {
 				assert.NoError(t, err)
 			} else {
 				assert.Error(t, err)
@@ -678,10 +678,10 @@ func TestCommand_RunDefaultCommand(t *testing.T) {
 }
 
 var defaultCommandSubCommandTests = []struct {
-	cmdName    string
-	subCmd     string
-	defaultCmd string
-	expected   bool
+	cmdName        string
+	subCmd         string
+	defaultCmd     string
+	errNotExpected bool
 }{
 	{"foobar", "", "foobar", true},
 	{"foobar", "carly", "foobar", true},
@@ -693,14 +693,14 @@ var defaultCommandSubCommandTests = []struct {
 	{"", "jimbob", "foobar", true},
 	{"", "j", "foobar", true},
 	{"", "carly", "foobar", true},
-	{"", "jimmers", "foobar", true},
+	{"", "jimmers", "foobar", false},
 	{"", "jimmers", "", true},
 	{" ", "jimmers", "foobar", false},
 	{"", "", "", true},
 	{" ", "", "", false},
 	{" ", "j", "", false},
-	{"bat", "", "batbaz", false},
-	{"nothing", "", "batbaz", false},
+	{"bat", "", "batbaz", true},
+	{"nothing", "", "batbaz", true},
 	{"nothing", "", "", false},
 	{"nothing", "j", "batbaz", false},
 	{"nothing", "carly", "", false},
@@ -726,7 +726,7 @@ func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) {
 			}
 
 			err := cmd.Run(buildTestContext(t), []string{"c", test.cmdName, test.subCmd})
-			if test.expected {
+			if test.errNotExpected {
 				assert.NoError(t, err)
 			} else {
 				assert.Error(t, err)
@@ -736,10 +736,10 @@ func TestCommand_RunDefaultCommandWithSubCommand(t *testing.T) {
 }
 
 var defaultCommandFlagTests = []struct {
-	cmdName    string
-	flag       string
-	defaultCmd string
-	expected   bool
+	cmdName        string
+	flag           string
+	defaultCmd     string
+	errNotExpected bool
 }{
 	{"foobar", "", "foobar", true},
 	{"foobar", "-c derp", "foobar", true},
@@ -754,14 +754,14 @@ var defaultCommandFlagTests = []struct {
 	{"", "--carly=derp", "foobar", true},
 	{"", "-j", "foobar", true},
 	{"", "-j", "", true},
-	{" ", "-j", "foobar", false},
+	{" ", "-j", "foobar", true},
 	{"", "", "", true},
 	{" ", "", "", false},
 	{" ", "-j", "", false},
-	{"bat", "", "batbaz", false},
-	{"nothing", "", "batbaz", false},
+	{"bat", "", "batbaz", true},
+	{"nothing", "", "batbaz", true},
 	{"nothing", "", "", false},
-	{"nothing", "--jimbob", "batbaz", false},
+	{"nothing", "--jimbob", "batbaz", true},
 	{"nothing", "--carly", "", false},
 }
 
@@ -810,7 +810,7 @@ func TestCommand_RunDefaultCommandWithFlags(t *testing.T) {
 			appArgs = append(appArgs, test.cmdName)
 
 			err := cmd.Run(buildTestContext(t), appArgs)
-			if test.expected {
+			if test.errNotExpected {
 				assert.NoError(t, err)
 			} else {
 				assert.Error(t, err)
@@ -1735,7 +1735,7 @@ func TestCommand_CommandNotFound(t *testing.T) {
 
 	_ = cmd.Run(buildTestContext(t), []string{"command", "foo"})
 
-	assert.Equal(t, 1, counts.CommandNotFound, 1)
+	assert.Equal(t, 1, counts.CommandNotFound)
 	assert.Equal(t, 0, counts.SubCommand)
 	assert.Equal(t, 1, counts.Total)
 }

examples_test.go

@@ -422,6 +422,7 @@ func ExampleCommand_Run_sliceValues() {
 			&cli.FloatSliceFlag{Name: "float64Slice"},
 			&cli.IntSliceFlag{Name: "intSlice"},
 		},
+		HideHelp: true,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			for i, v := range cmd.FlagNames() {
 				fmt.Printf("%d-%s %#v\n", i, v, cmd.Value(v))
@@ -454,6 +455,7 @@ func ExampleCommand_Run_mapValues() {
 		Flags: []cli.Flag{
 			&cli.StringMapFlag{Name: "stringMap"},
 		},
+		HideHelp: true,
 		Action: func(ctx context.Context, cmd *cli.Command) error {
 			for i, v := range cmd.FlagNames() {
 				fmt.Printf("%d-%s %#v\n", i, v, cmd.StringMap(v))

help.go

@@ -64,6 +64,8 @@ func helpCommandAction(ctx context.Context, cmd *Command) error {
 	args := cmd.Args()
 	firstArg := args.First()
 
+	tracef("doing help for cmd %[1]q with args %[2]q", cmd, args)
+
 	// This action can be triggered by a "default" action of a command
 	// or via cmd.Run when cmd == helpCmd. So we have following possibilities
 	//

help_test.go

@@ -175,7 +175,7 @@ func Test_helpCommand_Action_ErrorIfNoTopic(t *testing.T) {
 		flagSet: flag.NewFlagSet("test", 0),
 	}
 
-	_ = cmd.flagSet.Parse([]string{"foo"})
+	_ = cmd.Run(context.Background(), []string{"foo", "bar"})
 
 	err := helpCommandAction(context.Background(), cmd)
 	require.Error(t, err, "expected error from helpCommandAction()")
@@ -295,7 +295,7 @@ func Test_helpSubcommand_Action_ErrorIfNoTopic(t *testing.T) {
 	cmd := &Command{
 		flagSet: flag.NewFlagSet("test", 0),
 	}
-	_ = cmd.flagSet.Parse([]string{"foo"})
+	_ = cmd.Run(context.Background(), []string{"foo", "bar"})
 
 	err := helpCommandAction(context.Background(), cmd)
 	require.Error(t, err, "expected error from helpCommandAction(), but got nil")

parse.go

@@ -6,7 +6,6 @@ import (
 )
 
 type iterativeParser interface {
-	newFlagSet() (*flag.FlagSet, error)
 	useShortOptionHandling() bool
 }