diff --git a/app.go b/app.go index 76e869d37a..95d2038101 100644 --- a/app.go +++ b/app.go @@ -199,12 +199,12 @@ func (a *App) Run(arguments []string) (err error) { // always appends the completion flag at the end of the command shellComplete, arguments := checkShellCompleteFlag(a, arguments) - _, err = a.newFlagSet() + set, err := a.newFlagSet() if err != nil { return err } - set, err := parseIter(a, arguments[1:]) + err = parseIter(set, a, arguments[1:]) nerr := normalizeFlags(a.Flags, set) context := NewContext(a, set, nil) if nerr != nil { @@ -322,12 +322,12 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { } a.Commands = newCmds - _, err = a.newFlagSet() + set, err := a.newFlagSet() if err != nil { return err } - set, err := parseIter(a, ctx.Args().Tail()) + err = parseIter(set, a, ctx.Args().Tail()) nerr := normalizeFlags(a.Flags, set) context := NewContext(a, set, ctx) diff --git a/app_test.go b/app_test.go index bccede4959..33024ffeae 100644 --- a/app_test.go +++ b/app_test.go @@ -659,6 +659,17 @@ func TestApp_UseShortOptionHandling(t *testing.T) { expect(t, name, expected) } +func TestApp_UseShortOptionHandling_missing_value(t *testing.T) { + app := NewApp() + app.UseShortOptionHandling = true + app.Flags = []Flag{ + StringFlag{Name: "name, n"}, + } + + err := app.Run([]string{"", "-n"}) + expect(t, err, errors.New("flag needs an argument: -n")) +} + func TestApp_UseShortOptionHandlingCommand(t *testing.T) { var one, two bool var name string @@ -688,6 +699,21 @@ func TestApp_UseShortOptionHandlingCommand(t *testing.T) { expect(t, name, expected) } +func TestApp_UseShortOptionHandlingCommand_missing_value(t *testing.T) { + app := NewApp() + app.UseShortOptionHandling = true + command := Command{ + Name: "cmd", + Flags: []Flag{ + StringFlag{Name: "name, n"}, + }, + } + app.Commands = []Command{command} + + err := app.Run([]string{"", "cmd", "-n"}) + expect(t, err, errors.New("flag needs an argument: -n")) +} + func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) { var one, two bool var name string @@ -722,6 +748,25 @@ func TestApp_UseShortOptionHandlingSubCommand(t *testing.T) { expect(t, name, expected) } +func TestApp_UseShortOptionHandlingSubCommand_missing_value(t *testing.T) { + app := NewApp() + app.UseShortOptionHandling = true + command := Command{ + Name: "cmd", + } + subCommand := Command{ + Name: "sub", + Flags: []Flag{ + StringFlag{Name: "name, n"}, + }, + } + command.Subcommands = []Command{subCommand} + app.Commands = []Command{command} + + err := app.Run([]string{"", "cmd", "sub", "-n"}) + expect(t, err, errors.New("flag needs an argument: -n")) +} + func TestApp_Float64Flag(t *testing.T) { var meters float64 diff --git a/command.go b/command.go index 44a90de6b7..b10e4d4045 100644 --- a/command.go +++ b/command.go @@ -193,7 +193,12 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { args = reorderArgs(args) } - set, err := parseIter(c, args) + set, err := c.newFlagSet() + if err != nil { + return nil, err + } + + err = parseIter(set, c, args) if err != nil { return nil, err } diff --git a/command_test.go b/command_test.go index 56698fe1bb..6235b41db5 100644 --- a/command_test.go +++ b/command_test.go @@ -70,29 +70,34 @@ func TestParseAndRunShortOpts(t *testing.T) { {[]string{"foo", "test", "-af"}, nil, []string{}}, {[]string{"foo", "test", "-cf"}, nil, []string{}}, {[]string{"foo", "test", "-acf"}, nil, []string{}}, - {[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), []string{}}, + {[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, {[]string{"foo", "test", "-acf", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, - } - - var args []string - cmd := Command{ - Name: "test", - Usage: "this is for testing", - Description: "testing", - Action: func(c *Context) error { - args = c.Args() - return nil - }, - SkipArgReorder: true, - UseShortOptionHandling: true, - Flags: []Flag{ - BoolFlag{Name: "abc, a"}, - BoolFlag{Name: "cde, c"}, - BoolFlag{Name: "fgh, f"}, - }, + {[]string{"foo", "test", "-acfi", "not-arg", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, + {[]string{"foo", "test", "-i", "ivalue"}, nil, []string{}}, + {[]string{"foo", "test", "-i", "ivalue", "arg1"}, nil, []string{"arg1"}}, + {[]string{"foo", "test", "-i"}, errors.New("flag needs an argument: -i"), nil}, } for _, c := range cases { + var args []string + cmd := Command{ + Name: "test", + Usage: "this is for testing", + Description: "testing", + Action: func(c *Context) error { + args = c.Args() + return nil + }, + SkipArgReorder: true, + UseShortOptionHandling: true, + Flags: []Flag{ + BoolFlag{Name: "abc, a"}, + BoolFlag{Name: "cde, c"}, + BoolFlag{Name: "fgh, f"}, + StringFlag{Name: "ijk, i"}, + }, + } + app := NewApp() app.Commands = []Command{cmd} diff --git a/parse.go b/parse.go index 865accf102..2c2005c13a 100644 --- a/parse.go +++ b/parse.go @@ -14,22 +14,17 @@ type iterativeParser interface { // iteratively catch parsing errors. This way we achieve LR parsing without // transforming any arguments. Otherwise, there is no way we can discriminate // combined short options from common arguments that should be left untouched. -func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) { +func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { for { - set, err := ip.newFlagSet() - if err != nil { - return nil, err - } - - err = set.Parse(args) + err := set.Parse(args) if !ip.useShortOptionHandling() || err == nil { - return set, err + return err } errStr := err.Error() trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ") if errStr == trimmed { - return nil, err + return err } // regenerate the initial args with the split short opts @@ -42,7 +37,7 @@ func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) { shortOpts := splitShortOptions(set, trimmed) if len(shortOpts) == 1 { - return nil, err + return err } // add each short option and all remaining arguments @@ -50,6 +45,13 @@ func parseIter(ip iterativeParser, args []string) (*flag.FlagSet, error) { newArgs = append(newArgs, args[i+1:]...) args = newArgs } + + // Since custom parsing failed, replace the flag set before retrying + newSet, err := ip.newFlagSet() + if err != nil { + return err + } + *set = *newSet } }