diff --git a/CHANGELOG.md b/CHANGELOG.md index ea6c483..b0e1d48 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app_test.go b/app_test.go index ca4abce..a66cfa4 100644 --- a/app_test.go +++ b/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"} diff --git a/command.go b/command.go index 9ca7e51..7c94de3 100644 --- a/command.go +++ b/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 { diff --git a/command_test.go b/command_test.go index 2687212..5e51843 100644 --- a/command_test.go +++ b/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 {