Merge pull request #398 from codegangsta/do-not-reorder-flags

Remove reordering of flags and arguments

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 {