Backport no argument reordering as an option

In the unreleased version 2, the argument reordering has been removed
(in f585ec7cb8) since it only worked if
all of the arguments appeared before all of the flags, but not if they
were intermixed which was of limited utility and caused some confusion.
This commit allows enabling of this future behavior via SkipArgReorder.

Ideally we'd support complete intermingling of flags and arguments, but
this is unlikely to happen until we switch flag parsers.

Fixes #515

command.go

@@ -46,6 +46,11 @@ type Command struct {
 	Flags []Flag
 	// Treat all flags as normal arguments if true
 	SkipFlagParsing bool
+	// Skip argument reordering which attempts to move flags before arguments,
+	// but only works if all flags appear after all arguments. This behavior was
+	// removed n version 2 since it only works under specific conditions so we
+	// backport here by exposing it as an option for compatibility.
+	SkipArgReorder bool
 	// Boolean to hide built-in help command
 	HideHelp bool
 	// Boolean to hide this command from help or completion
@@ -89,7 +94,9 @@ func (c Command) Run(ctx *Context) (err error) {
 	set := flagSet(c.Name, c.Flags)
 	set.SetOutput(ioutil.Discard)
 
-	if !c.SkipFlagParsing {
+	if c.SkipFlagParsing {
+		err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
+	} else if !c.SkipArgReorder {
 		firstFlagIndex := -1
 		terminatorIndex := -1
 		for index, arg := range ctx.Args() {
@@ -121,10 +128,6 @@ func (c Command) Run(ctx *Context) (err error) {
 		} else {
 			err = set.Parse(ctx.Args().Tail())
 		}
-	} else {
-		if c.SkipFlagParsing {
-			err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
-		}
 	}
 
 	if err != nil {

command_test.go

@@ -13,12 +13,18 @@ func TestCommandFlagParsing(t *testing.T) {
 	cases := []struct {
 		testArgs        []string
 		skipFlagParsing bool
+		skipArgReorder  bool
 		expectedErr     error
 	}{
-		{[]string{"blah", "blah", "-break"}, false, errors.New("flag provided but not defined: -break")}, // Test normal "not ignoring flags" flow
+		// Test normal "not ignoring flags" flow
-		{[]string{"blah", "blah"}, true, nil},                                                            // Test SkipFlagParsing without any args that look like flags
+		{[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break")},
-		{[]string{"blah", "-break"}, true, nil},                                                          // Test SkipFlagParsing with random flag arg
+
-		{[]string{"blah", "-help"}, true, nil},                                                           // Test SkipFlagParsing with "special" help flag arg
+		// Test no arg reorder
+		{[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil},
+
+		{[]string{"test-cmd", "blah", "blah"}, true, false, nil},   // Test SkipFlagParsing without any args that look like flags
+		{[]string{"test-cmd", "blah", "-break"}, true, false, nil}, // Test SkipFlagParsing with random flag arg
+		{[]string{"test-cmd", "blah", "-help"}, true, false, nil},  // Test SkipFlagParsing with "special" help flag arg
 	}
 
 	for _, c := range cases {
@@ -30,15 +36,15 @@ func TestCommandFlagParsing(t *testing.T) {
 		context := NewContext(app, set, nil)
 
 		command := Command{
-			Name:        "test-cmd",
+			Name:            "test-cmd",
-			Aliases:     []string{"tc"},
+			Aliases:         []string{"tc"},
-			Usage:       "this is for testing",
+			Usage:           "this is for testing",
-			Description: "testing",
+			Description:     "testing",
-			Action:      func(_ *Context) error { return nil },
+			Action:          func(_ *Context) error { return nil },
+			SkipFlagParsing: c.skipFlagParsing,
+			SkipArgReorder:  c.skipArgReorder,
 		}
 
-		command.SkipFlagParsing = c.skipFlagParsing
-
 		err := command.Run(context)
 
 		expect(t, err, c.expectedErr)