From 43c8c02cf5a10196e5a4c458fdbfee90a561e97c Mon Sep 17 00:00:00 2001 From: zhuchensong Date: Mon, 17 Apr 2017 00:47:04 +0800 Subject: [PATCH 01/10] Support POSIX-style short flag combining --- command.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index be8f8f0..189209d 100644 --- a/command.go +++ b/command.go @@ -142,7 +142,19 @@ func (c Command) Run(ctx *Context) (err error) { flagArgs = args[firstFlagIndex:] } - err = set.Parse(append(flagArgs, regularArgs...)) + // separate combined flags + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) >2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-" + string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) + } + } + + err = set.Parse(append(flagArgsSeparated, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } From fd5382e7a539858cc19d7eed7755f7102bae5da9 Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 13 Nov 2017 15:28:23 -0600 Subject: [PATCH 02/10] Combine bool short names Adds the ability to allow the combination of bool short-name options. For example, cmd foobar -ov This is done through a bool "UseShortOptionHandler" set in the command struct. Built upon PR #621 Signed-off-by: baude --- README.md | 21 +++++++++++++++++++++ command.go | 26 ++++++++++++++++---------- command_test.go | 36 ++++++++++++++++++++---------------- flag_test.go | 25 +++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index a2fd41d..6096701 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ applications in an expressive way. * [Version Flag](#version-flag) + [Customization](#customization-2) + [Full API Example](#full-api-example) + * [Combining short Bool options](#combining-short-bool-options) - [Contribution Guidelines](#contribution-guidelines) @@ -1410,6 +1411,26 @@ func wopAction(c *cli.Context) error { } ``` +### Combining short Bool options + +Traditional use of boolean options using their shortnames look like this: +``` +# cmd foobar -s -o +``` + +Suppose you want users to be able to combine your bool options with their shortname. This +can be done using the **UseShortOptionHandling** bool in your commands. Suppose your program +has a two bool flags such as *serve* and *option* with the short options of *-o* and +*-s* respectively. With **UseShortOptionHandling** set to *true*, a user can use a syntax +like: +``` +# cmd foobar -so +``` + +If you enable the **UseShortOptionHandling*, then you must not use any flags that have a single +leading *-* or this will result in failures. For example, **-option** can no longer be used. Flags +with two leading dashes (such as **--options**) are still valid. + ## Contribution Guidelines Feel free to put up a pull request to fix a bug or maybe add a feature. I will diff --git a/command.go b/command.go index 189209d..b559811 100644 --- a/command.go +++ b/command.go @@ -55,6 +55,10 @@ type Command struct { HideHelp bool // Boolean to hide this command from help or completion Hidden bool + // Boolean to enable short-option handling so user can combine several + // single-character bool arguements into one + // i.e. foobar -o -v -> foobar -ov + UseShortOptionHandling bool // Full name of command for help, defaults to full command name, including parent commands. HelpName string @@ -141,20 +145,22 @@ func (c Command) Run(ctx *Context) (err error) { } else { flagArgs = args[firstFlagIndex:] } - // separate combined flags - var flagArgsSeparated []string - for _, flagArg := range flagArgs { - if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) >2 { - for _, flagChar := range flagArg[1:] { - flagArgsSeparated = append(flagArgsSeparated, "-" + string(flagChar)) + if c.UseShortOptionHandling { + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) } - } else { - flagArgsSeparated = append(flagArgsSeparated, flagArg) } + err = set.Parse(append(flagArgsSeparated, regularArgs...)) + } else { + err = set.Parse(append(flagArgs, regularArgs...)) } - - err = set.Parse(append(flagArgsSeparated, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } diff --git a/command_test.go b/command_test.go index 4ad994c..d9d7094 100644 --- a/command_test.go +++ b/command_test.go @@ -11,20 +11,23 @@ import ( func TestCommandFlagParsing(t *testing.T) { cases := []struct { - testArgs []string - skipFlagParsing bool - skipArgReorder bool - expectedErr error + testArgs []string + skipFlagParsing bool + skipArgReorder bool + expectedErr error + UseShortOptionHandling bool }{ // Test normal "not ignoring flags" flow - {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break")}, + {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break"), false}, // Test no arg reorder - {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil}, + {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false}, + + {[]string{"test-cmd", "blah", "blah"}, true, false, nil, false}, // Test SkipFlagParsing without any args that look like flags + {[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg + {[]string{"test-cmd", "blah", "-help"}, true, false, nil, false}, // Test SkipFlagParsing with "special" help flag arg + {[]string{"test-cmd", "blah"}, false, false, nil, true}, // Test UseShortOptionHandling - {[]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 { @@ -36,13 +39,14 @@ func TestCommandFlagParsing(t *testing.T) { context := NewContext(app, set, nil) command := Command{ - Name: "test-cmd", - Aliases: []string{"tc"}, - Usage: "this is for testing", - Description: "testing", - Action: func(_ *Context) error { return nil }, - SkipFlagParsing: c.skipFlagParsing, - SkipArgReorder: c.skipArgReorder, + Name: "test-cmd", + Aliases: []string{"tc"}, + Usage: "this is for testing", + Description: "testing", + Action: func(_ *Context) error { return nil }, + SkipFlagParsing: c.skipFlagParsing, + SkipArgReorder: c.skipArgReorder, + UseShortOptionHandling: c.UseShortOptionHandling, } err := command.Run(context) diff --git a/flag_test.go b/flag_test.go index 98c2fb9..da9fd73 100644 --- a/flag_test.go +++ b/flag_test.go @@ -1048,6 +1048,31 @@ func TestParseMultiBool(t *testing.T) { a.Run([]string{"run", "--serve"}) } +func TestParseBoolShortOptionHandle(t *testing.T) { + a := App{ + Commands: []Command{ + { + Name: "foobar", + UseShortOptionHandling: true, + Action: func(ctx *Context) error { + if ctx.Bool("serve") != true { + t.Errorf("main name not set") + } + if ctx.Bool("option") != true { + t.Errorf("short name not set") + } + return nil + }, + Flags: []Flag{ + BoolFlag{Name: "serve, s"}, + BoolFlag{Name: "option, o"}, + }, + }, + }, + } + a.Run([]string{"run", "foobar", "-so"}) +} + func TestParseDestinationBool(t *testing.T) { var dest bool a := App{ From 37b7abb1c491c8c3630a2a98bb02a7051efbcc06 Mon Sep 17 00:00:00 2001 From: Joshua Rubin Date: Tue, 21 Nov 2017 15:21:31 -0700 Subject: [PATCH 03/10] dont clobber slices with envvar Signed-off-by: Joshua Rubin --- flag.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/flag.go b/flag.go index f8d4f51..d4a4d41 100644 --- a/flag.go +++ b/flag.go @@ -178,7 +178,11 @@ func (f StringSliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as string value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { @@ -235,7 +239,11 @@ func (f IntSliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as int slice value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { @@ -292,7 +300,11 @@ func (f Int64SliceFlag) ApplyWithError(set *flag.FlagSet) error { return fmt.Errorf("could not parse %s as int64 slice value for flag %s: %s", envVal, f.Name, err) } } - f.Value = newVal + if f.Value == nil { + f.Value = newVal + } else { + *f.Value = *newVal + } } eachName(f.Name, func(name string) { From ceaac7c9152121e6ba0f3b492b3254d61346f92a Mon Sep 17 00:00:00 2001 From: baude Date: Mon, 20 Nov 2017 09:32:03 -0600 Subject: [PATCH 04/10] Handle ShortOptions and SkipArgReorder There was a bug in parsing when both ShortOptions and SkipArgReorder were being used together. Signed-off-by: baude --- command.go | 129 ++++++++++++++++++++++++++++++++---------------- command_test.go | 1 + 2 files changed, 87 insertions(+), 43 deletions(-) diff --git a/command.go b/command.go index b559811..7d0357b 100644 --- a/command.go +++ b/command.go @@ -115,57 +115,29 @@ func (c Command) Run(ctx *Context) (err error) { return err } set.SetOutput(ioutil.Discard) - + firstFlagIndex, terminatorIndex := getIndexes(ctx) + flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex) + if c.UseShortOptionHandling { + flagArgs = translateShortOptions(flagArgs) + } 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() { - 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:] - } - // separate combined flags - if c.UseShortOptionHandling { - var flagArgsSeparated []string - for _, flagArg := range flagArgs { - if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { - for _, flagChar := range flagArg[1:] { - flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) - } - } else { - flagArgsSeparated = append(flagArgsSeparated, flagArg) - } - } - err = set.Parse(append(flagArgsSeparated, regularArgs...)) - } else { - err = set.Parse(append(flagArgs, regularArgs...)) - } + err = set.Parse(append(flagArgs, regularArgs...)) } else { err = set.Parse(ctx.Args().Tail()) } + } else if c.UseShortOptionHandling { + if terminatorIndex == -1 && firstFlagIndex > -1 { + // Handle shortname AND no options + err = set.Parse(append(regularArgs, flagArgs...)) + } else { + // Handle shortname and options + err = set.Parse(flagArgs) + } } else { - err = set.Parse(ctx.Args().Tail()) + err = set.Parse(append(regularArgs, flagArgs...)) } nerr := normalizeFlags(c.Flags, set) @@ -233,6 +205,77 @@ func (c Command) Run(ctx *Context) (err error) { return err } +func getIndexes(ctx *Context) (int, int) { + 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 len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 { + return -1, -1 + } + + return firstFlagIndex, terminatorIndex + +} + +// copyStringslice takes a string slice and copies it +func copyStringSlice(slice []string, start, end int) []string { + newSlice := make([]string, end-start) + copy(newSlice, slice[start:end]) + return newSlice +} + +// getAllArgs extracts and returns two string slices representing +// regularArgs and flagArgs +func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) { + var regularArgs []string + // if there are no options, the we set the index to 1 manually + if firstFlagIndex == -1 { + firstFlagIndex = 1 + regularArgs = copyStringSlice(args, 0, len(args)) + } else { + regularArgs = copyStringSlice(args, 1, firstFlagIndex) + } + var flagArgs []string + // a flag terminatorIndex was found in the input. we need to collect + // flagArgs based on it. + if terminatorIndex > -1 { + flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex) + additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args)) + regularArgs = append(regularArgs, additionalRegularArgs...) + for _, i := range additionalRegularArgs { + regularArgs = append(regularArgs, i) + } + } else { + flagArgs = args[firstFlagIndex:] + } + return flagArgs, regularArgs +} + +func translateShortOptions(flagArgs Args) []string { + // separate combined flags + var flagArgsSeparated []string + for _, flagArg := range flagArgs { + if strings.HasPrefix(flagArg, "-") && strings.HasPrefix(flagArg, "--") == false && len(flagArg) > 2 { + for _, flagChar := range flagArg[1:] { + flagArgsSeparated = append(flagArgsSeparated, "-"+string(flagChar)) + } + } else { + flagArgsSeparated = append(flagArgsSeparated, flagArg) + } + } + return flagArgsSeparated +} + // Names returns the names including short names and aliases. func (c Command) Names() []string { names := []string{c.Name} diff --git a/command_test.go b/command_test.go index d9d7094..e69750a 100644 --- a/command_test.go +++ b/command_test.go @@ -22,6 +22,7 @@ func TestCommandFlagParsing(t *testing.T) { // Test no arg reorder {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false}, + {[]string{"test-cmd", "blah", "blah", "-break", "ls", "-l"}, false, true, nil, true}, {[]string{"test-cmd", "blah", "blah"}, true, false, nil, false}, // Test SkipFlagParsing without any args that look like flags {[]string{"test-cmd", "blah", "-break"}, true, false, nil, false}, // Test SkipFlagParsing with random flag arg From c6eb2a051026c083d4e33591f8d6e95d5f4189dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Lipt=C3=A1k?= Date: Thu, 30 Nov 2017 19:43:12 -0500 Subject: [PATCH 05/10] Correct go vet for Go tip https://travis-ci.org/cloudflare/logshare/jobs/309796141#L646 --- app.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app.go b/app.go index 60599b0..9add067 100644 --- a/app.go +++ b/app.go @@ -453,7 +453,6 @@ func (a *App) hasFlag(flag Flag) bool { } func (a *App) errWriter() io.Writer { - // When the app ErrWriter is nil use the package level one. if a.ErrWriter == nil { return ErrWriter From df562bf1a8626f2d16f91fcbf7230a5bdca3d592 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Sun, 3 Dec 2017 13:38:50 -0800 Subject: [PATCH 06/10] Refactor flag handling logic Refactor logic introduced by #686 --- command.go | 134 +++++++++++++++++++++-------------------------------- 1 file changed, 52 insertions(+), 82 deletions(-) diff --git a/command.go b/command.go index 7d0357b..66a58b5 100644 --- a/command.go +++ b/command.go @@ -1,6 +1,7 @@ package cli import ( + "flag" "fmt" "io/ioutil" "sort" @@ -110,43 +111,7 @@ func (c Command) Run(ctx *Context) (err error) { ) } - set, err := flagSet(c.Name, c.Flags) - if err != nil { - return err - } - set.SetOutput(ioutil.Discard) - firstFlagIndex, terminatorIndex := getIndexes(ctx) - flagArgs, regularArgs := getAllArgs(ctx.Args(), firstFlagIndex, terminatorIndex) - if c.UseShortOptionHandling { - flagArgs = translateShortOptions(flagArgs) - } - if c.SkipFlagParsing { - err = set.Parse(append([]string{"--"}, ctx.Args().Tail()...)) - } else if !c.SkipArgReorder { - if firstFlagIndex > -1 { - err = set.Parse(append(flagArgs, regularArgs...)) - } else { - err = set.Parse(ctx.Args().Tail()) - } - } else if c.UseShortOptionHandling { - if terminatorIndex == -1 && firstFlagIndex > -1 { - // Handle shortname AND no options - err = set.Parse(append(regularArgs, flagArgs...)) - } else { - // Handle shortname and options - err = set.Parse(flagArgs) - } - } else { - err = set.Parse(append(regularArgs, flagArgs...)) - } - - nerr := normalizeFlags(c.Flags, set) - if nerr != nil { - fmt.Fprintln(ctx.App.Writer, nerr) - fmt.Fprintln(ctx.App.Writer) - ShowCommandHelp(ctx, c.Name) - return nerr - } + set, err := c.parseFlags(ctx.Args().Tail()) context := NewContext(ctx.App, set, ctx) context.Command = c @@ -205,60 +170,65 @@ func (c Command) Run(ctx *Context) (err error) { return err } -func getIndexes(ctx *Context) (int, int) { - 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 - } +func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { + set, err := flagSet(c.Name, c.Flags) + if err != nil { + return nil, err } - if len(ctx.Args()) > 0 && !strings.HasPrefix(ctx.Args()[0], "-") && firstFlagIndex == -1 { - return -1, -1 + set.SetOutput(ioutil.Discard) + + if c.SkipFlagParsing { + return set, set.Parse(append([]string{c.Name, "--"}, args...)) } - return firstFlagIndex, terminatorIndex + if c.UseShortOptionHandling { + args = translateShortOptions(args) + } -} + if !c.SkipArgReorder { + args = reorderArgs(args) + } -// copyStringslice takes a string slice and copies it -func copyStringSlice(slice []string, start, end int) []string { - newSlice := make([]string, end-start) - copy(newSlice, slice[start:end]) - return newSlice -} + err = set.Parse(args) + if err != nil { + return nil, err + } -// getAllArgs extracts and returns two string slices representing -// regularArgs and flagArgs -func getAllArgs(args []string, firstFlagIndex, terminatorIndex int) ([]string, []string) { - var regularArgs []string - // if there are no options, the we set the index to 1 manually - if firstFlagIndex == -1 { - firstFlagIndex = 1 - regularArgs = copyStringSlice(args, 0, len(args)) - } else { - regularArgs = copyStringSlice(args, 1, firstFlagIndex) + err = normalizeFlags(c.Flags, set) + if err != nil { + return nil, err } - var flagArgs []string - // a flag terminatorIndex was found in the input. we need to collect - // flagArgs based on it. - if terminatorIndex > -1 { - flagArgs = copyStringSlice(args, firstFlagIndex, terminatorIndex) - additionalRegularArgs := copyStringSlice(args, terminatorIndex, len(args)) - regularArgs = append(regularArgs, additionalRegularArgs...) - for _, i := range additionalRegularArgs { - regularArgs = append(regularArgs, i) + + return set, nil +} + +// reorderArgs moves all flags before arguments as this is what flag expects +func reorderArgs(args []string) []string { + var nonflags, flags []string + + readFlagValue := false + for i, arg := range args { + if arg == "--" { + nonflags = append(nonflags, args[i:]...) + break + } + + if readFlagValue { + readFlagValue = false + flags = append(flags, arg) + continue + } + + if arg != "-" && strings.HasPrefix(arg, "-") { + flags = append(flags, arg) + + readFlagValue = !strings.Contains(arg, "=") + } else { + nonflags = append(nonflags, arg) } - } else { - flagArgs = args[firstFlagIndex:] } - return flagArgs, regularArgs + + return append(flags, nonflags...) } func translateShortOptions(flagArgs Args) []string { From 0671b166dcacb3dc1215ba65bf986dab194581dc Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Mon, 4 Dec 2017 09:23:40 -0800 Subject: [PATCH 07/10] Add tests for flag reordering --- command_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/command_test.go b/command_test.go index e69750a..c235e4a 100644 --- a/command_test.go +++ b/command_test.go @@ -243,3 +243,41 @@ func TestCommand_Run_SubcommandsCanUseErrWriter(t *testing.T) { t.Fatal(err) } } + +func TestCommandFlagReordering(t *testing.T) { + cases := []struct { + testArgs []string + expectedValue string + expectedArgs []string + expectedErr error + }{ + {[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, "foo", []string{"some-arg"}, nil}, + {[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, "foo", []string{"some-arg"}, nil}, + {[]string{"some-exec", "some-command", "--flag=foo", "some-arg"}, "foo", []string{"some-arg"}, nil}, + } + + for _, c := range cases { + value := "" + args := []string{} + app := &App{ + Commands: []Command{ + { + Name: "some-command", + Flags: []Flag{ + StringFlag{Name: "flag"}, + }, + Action: func(c *Context) { + fmt.Printf("%+v\n", c.String("flag")) + value = c.String("flag") + args = c.Args() + }, + }, + }, + } + + err := app.Run(c.testArgs) + expect(t, err, c.expectedErr) + expect(t, value, c.expectedValue) + expect(t, args, c.expectedArgs) + } +} From e38e4ae2d05acf5b5164c160a67fb7048e1358b0 Mon Sep 17 00:00:00 2001 From: Jesse Szwedko Date: Fri, 29 Dec 2017 13:38:18 -0500 Subject: [PATCH 08/10] Fix regression of SkipFlagParsing behavior Introduced by df562bf1a8626f2d16f91fcbf7230a5bdca3d592 Was mistakenly prepending the command name. --- command.go | 2 +- command_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index 66a58b5..ed4a81a 100644 --- a/command.go +++ b/command.go @@ -178,7 +178,7 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { set.SetOutput(ioutil.Discard) if c.SkipFlagParsing { - return set, set.Parse(append([]string{c.Name, "--"}, args...)) + return set, set.Parse(append([]string{"--"}, args...)) } if c.UseShortOptionHandling { diff --git a/command_test.go b/command_test.go index c235e4a..c84b762 100644 --- a/command_test.go +++ b/command_test.go @@ -281,3 +281,39 @@ func TestCommandFlagReordering(t *testing.T) { expect(t, args, c.expectedArgs) } } + +func TestCommandSkipFlagParsing(t *testing.T) { + cases := []struct { + testArgs []string + expectedArgs []string + expectedErr error + }{ + {[]string{"some-exec", "some-command", "some-arg", "--flag", "foo"}, []string{"some-arg", "--flag", "foo"}, nil}, + {[]string{"some-exec", "some-command", "some-arg", "--flag=foo"}, []string{"some-arg", "--flag=foo"}, nil}, + } + + for _, c := range cases { + value := "" + args := []string{} + app := &App{ + Commands: []Command{ + { + SkipFlagParsing: true, + Name: "some-command", + Flags: []Flag{ + StringFlag{Name: "flag"}, + }, + Action: func(c *Context) { + fmt.Printf("%+v\n", c.String("flag")) + value = c.String("flag") + args = c.Args() + }, + }, + }, + } + + err := app.Run(c.testArgs) + expect(t, err, c.expectedErr) + expect(t, args, c.expectedArgs) + } +} From d7555e172994da8d058334aa1fe69533b1685924 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Olivier=20Mengu=C3=A9?= Date: Fri, 26 Jan 2018 21:14:34 +0100 Subject: [PATCH 09/10] Fix unnecessary uses of Sprintf - use strconv directly - use concatenation for "%s%s" --- flag.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/flag.go b/flag.go index d4a4d41..b0cffc0 100644 --- a/flag.go +++ b/flag.go @@ -636,7 +636,7 @@ func withEnvHint(envVar, str string) string { suffix = "%" sep = "%, %" } - envText = fmt.Sprintf(" [%s%s%s]", prefix, strings.Join(strings.Split(envVar, ","), sep), suffix) + envText = " [" + prefix + strings.Join(strings.Split(envVar, ","), sep) + suffix + "]" } return str + envText } @@ -709,13 +709,13 @@ func stringifyFlag(f Flag) string { placeholder = defaultPlaceholder } - usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultValueString)) + usageWithDefault := strings.TrimSpace(usage + defaultValueString) return FlagFileHinter( fv.FieldByName("FilePath").String(), FlagEnvHinter( fv.FieldByName("EnvVar").String(), - fmt.Sprintf("%s\t%s", FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder), usageWithDefault), + FlagNamePrefixer(fv.FieldByName("Name").String(), placeholder)+"\t"+usageWithDefault, ), ) } @@ -724,7 +724,7 @@ func stringifyIntSliceFlag(f IntSliceFlag) string { defaultVals := []string{} if f.Value != nil && len(f.Value.Value()) > 0 { for _, i := range f.Value.Value() { - defaultVals = append(defaultVals, fmt.Sprintf("%d", i)) + defaultVals = append(defaultVals, strconv.Itoa(i)) } } @@ -735,7 +735,7 @@ func stringifyInt64SliceFlag(f Int64SliceFlag) string { defaultVals := []string{} if f.Value != nil && len(f.Value.Value()) > 0 { for _, i := range f.Value.Value() { - defaultVals = append(defaultVals, fmt.Sprintf("%d", i)) + defaultVals = append(defaultVals, strconv.FormatInt(i, 10)) } } @@ -747,7 +747,7 @@ func stringifyStringSliceFlag(f StringSliceFlag) string { if f.Value != nil && len(f.Value.Value()) > 0 { for _, s := range f.Value.Value() { if len(s) > 0 { - defaultVals = append(defaultVals, fmt.Sprintf("%q", s)) + defaultVals = append(defaultVals, strconv.Quote(s)) } } } @@ -766,8 +766,8 @@ func stringifySliceFlag(usage, name string, defaultVals []string) string { defaultVal = fmt.Sprintf(" (default: %s)", strings.Join(defaultVals, ", ")) } - usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal)) - return fmt.Sprintf("%s\t%s", FlagNamePrefixer(name, placeholder), usageWithDefault) + usageWithDefault := strings.TrimSpace(usage + defaultVal) + return FlagNamePrefixer(name, placeholder) + "\t" + usageWithDefault } func flagFromFileEnv(filePath, envName string) (val string, ok bool) { From 3a87b13b01ac8628694f1e1b20bdb452cc0f54d2 Mon Sep 17 00:00:00 2001 From: Nico Windler Date: Sat, 10 Feb 2018 13:35:23 +0100 Subject: [PATCH 10/10] Fix args reordering when bool flags are present --- app_test.go | 33 +++++++++++++++++++++++++++++++++ command.go | 3 ++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app_test.go b/app_test.go index 5db9882..42fb149 100644 --- a/app_test.go +++ b/app_test.go @@ -329,6 +329,39 @@ func TestApp_CommandWithArgBeforeFlags(t *testing.T) { expect(t, firstArg, "my-arg") } +func TestApp_CommandWithArgBeforeBoolFlags(t *testing.T) { + var parsedOption, parsedSecondOption, firstArg string + var parsedBool, parsedSecondBool bool + + app := NewApp() + command := Command{ + Name: "cmd", + Flags: []Flag{ + StringFlag{Name: "option", Value: "", Usage: "some option"}, + StringFlag{Name: "secondOption", Value: "", Usage: "another option"}, + BoolFlag{Name: "boolflag", Usage: "some bool"}, + BoolFlag{Name: "b", Usage: "another bool"}, + }, + Action: func(c *Context) error { + parsedOption = c.String("option") + parsedSecondOption = c.String("secondOption") + parsedBool = c.Bool("boolflag") + parsedSecondBool = c.Bool("b") + firstArg = c.Args().First() + return nil + }, + } + app.Commands = []Command{command} + + app.Run([]string{"", "cmd", "my-arg", "--boolflag", "--option", "my-option", "-b", "--secondOption", "fancy-option"}) + + expect(t, parsedOption, "my-option") + expect(t, parsedSecondOption, "fancy-option") + expect(t, parsedBool, true) + expect(t, parsedSecondBool, true) + expect(t, firstArg, "my-arg") +} + func TestApp_RunAsSubcommandParseFlags(t *testing.T) { var context *Context diff --git a/command.go b/command.go index ed4a81a..2acb976 100644 --- a/command.go +++ b/command.go @@ -213,11 +213,12 @@ func reorderArgs(args []string) []string { break } - if readFlagValue { + if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { readFlagValue = false flags = append(flags, arg) continue } + readFlagValue = false if arg != "-" && strings.HasPrefix(arg, "-") { flags = append(flags, arg)