Remove reordering of flags and arguments

This was introduced by #36, but only worked in the specific case of all
arguments being passed before all flags. If the user mixed them, they
ended up with odd parsing behavior where the arguments were reordered
(causing #103 and #355).

Given the tradeoffs I think we should remove support for flag
reordering.

Fixes #103
Fixes #355

CHANGELOG.md

@@ -8,8 +8,15 @@
 
 ### Removed
 - the ability to specify `&StringSlice{...string}` or `&IntSlice{...int}`.
-To migrate to the new API, you may choose to run [this helper
-(python) script](./cli-migrate-slice-types).
+  To migrate to the new API, you may choose to run [this helper
+  (python) script](./cli-migrate-slice-types).
+- The optimistic reordering of arguments and flags introduced by
+  https://github.com/codegangsta/cli/pull/36. This behavior only worked when
+  all arguments appeared before all flags, but caused [weird issues with boolean
+  flags](https://github.com/codegangsta/cli/issues/103) and [reordering of the
+  arguments](https://github.com/codegangsta/cli/issues/355) when the user
+  attempted to mix flags and arguments. Given the trade-offs we removed support
+  for this reordering.
 
 ## [Unreleased] - (1.x series)
 ### Added

app_test.go

@@ -189,29 +189,6 @@ func TestApp_Command(t *testing.T) {
 	}
 }
 
-func TestApp_CommandWithArgBeforeFlags(t *testing.T) {
-	var parsedOption, firstArg string
-
-	app := NewApp()
-	command := Command{
-		Name: "cmd",
-		Flags: []Flag{
-			StringFlag{Name: "option", Value: "", Usage: "some option"},
-		},
-		Action: func(c *Context) error {
-			parsedOption = c.String("option")
-			firstArg = c.Args().First()
-			return nil
-		},
-	}
-	app.Commands = []Command{command}
-
-	app.Run([]string{"", "cmd", "my-arg", "--option", "my-option"})
-
-	expect(t, parsedOption, "my-option")
-	expect(t, firstArg, "my-arg")
-}
-
 func TestApp_RunAsSubcommandParseFlags(t *testing.T) {
 	var context *Context
 
@@ -257,7 +234,7 @@ func TestApp_CommandWithFlagBeforeTerminator(t *testing.T) {
 	}
 	app.Commands = []Command{command}
 
-	app.Run([]string{"", "cmd", "my-arg", "--option", "my-option", "--", "--notARealFlag"})
+	app.Run([]string{"", "cmd", "--option", "my-option", "my-arg", "--", "--notARealFlag"})
 
 	expect(t, parsedOption, "my-option")
 	expect(t, args[0], "my-arg")
@@ -342,7 +319,7 @@ func TestApp_ParseSliceFlags(t *testing.T) {
 	}
 	app.Commands = []Command{command}
 
-	app.Run([]string{"", "cmd", "my-arg", "-p", "22", "-p", "80", "-ip", "8.8.8.8", "-ip", "8.8.4.4"})
+	app.Run([]string{"", "cmd", "-p", "22", "-p", "80", "-ip", "8.8.8.8", "-ip", "8.8.4.4", "my-arg"})
 
 	IntsEquals := func(a, b []int) bool {
 		if len(a) != len(b) {
@@ -398,7 +375,7 @@ func TestApp_ParseSliceFlagsWithMissingValue(t *testing.T) {
 	}
 	app.Commands = []Command{command}
 
-	app.Run([]string{"", "cmd", "my-arg", "-a", "2", "-str", "A"})
+	app.Run([]string{"", "cmd", "-a", "2", "-str", "A", "my-arg"})
 
 	var expectedIntSlice = []int{2}
 	var expectedStringSlice = []string{"A"}

command.go

@@ -86,42 +86,10 @@ func (c Command) Run(ctx *Context) (err error) {
 	set := flagSet(c.Name, c.Flags)
 	set.SetOutput(ioutil.Discard)
 
-	if !c.SkipFlagParsing {
-		firstFlagIndex := -1
-		terminatorIndex := -1
-		for index, arg := range ctx.Args() {
-			if arg == "--" {
-				terminatorIndex = index
-				break
-			} else if arg == "-" {
-				// Do nothing. A dash alone is not really a flag.
-				continue
-			} else if strings.HasPrefix(arg, "-") && firstFlagIndex == -1 {
-				firstFlagIndex = index
-			}
-		}
-
-		if firstFlagIndex > -1 {
-			args := ctx.Args()
-			regularArgs := make([]string, len(args[1:firstFlagIndex]))
-			copy(regularArgs, args[1:firstFlagIndex])
-
-			var flagArgs []string
-			if terminatorIndex > -1 {
-				flagArgs = args[firstFlagIndex:terminatorIndex]
-				regularArgs = append(regularArgs, args[terminatorIndex:]...)
-			} else {
-				flagArgs = args[firstFlagIndex:]
-			}
-
-			err = set.Parse(append(flagArgs, regularArgs...))
-		} else {
-			err = set.Parse(ctx.Args().Tail())
-		}
+	if c.SkipFlagParsing {
+		err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
 	} else {
-		if c.SkipFlagParsing {
-			err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...))
-		}
+		err = set.Parse(ctx.Args().Tail())
 	}
 
 	if err != nil {

command_test.go

@@ -15,10 +15,10 @@ func TestCommandFlagParsing(t *testing.T) {
 		skipFlagParsing bool
 		expectedErr     error
 	}{
-		{[]string{"blah", "blah", "-break"}, false, errors.New("flag provided but not defined: -break")}, // Test normal "not ignoring flags" flow
-		{[]string{"blah", "blah"}, true, nil},                                                            // Test SkipFlagParsing without any args that look like flags
-		{[]string{"blah", "-break"}, true, nil},                                                          // Test SkipFlagParsing with random flag arg
-		{[]string{"blah", "-help"}, true, nil},                                                           // Test SkipFlagParsing with "special" help flag arg
+		{[]string{"test-cmd", "-break", "blah", "blah"}, false, errors.New("flag provided but not defined: -break")}, // Test normal "not ignoring flags" flow
+		{[]string{"test-cmd", "blah", "blah"}, true, nil},                                                            // Test SkipFlagParsing without any args that look like flags
+		{[]string{"test-cmd", "-break", "blah"}, true, nil},                                                          // Test SkipFlagParsing with random flag arg
+		{[]string{"test-cmd", "-help", "blah"}, true, nil},                                                           // Test SkipFlagParsing with "special" help flag arg
 	}
 
 	for _, c := range cases {