From f98cd70c9c344c7ffbc66845d43dbb9b724a7eab Mon Sep 17 00:00:00 2001 From: Brendan Burns Date: Fri, 13 Mar 2015 14:21:42 -0700 Subject: [PATCH] Update the cobra and pflag libraries with upstream fixes. --- Godeps/Godeps.json | 4 +- .../src/github.com/spf13/cobra/cobra_test.go | 48 ++++- .../src/github.com/spf13/cobra/command.go | 202 +++++++++++------- .../src/github.com/spf13/pflag/bool_test.go | 2 +- .../github.com/spf13/pflag/example_test.go | 2 +- .../src/github.com/spf13/pflag/flag.go | 4 + 6 files changed, 173 insertions(+), 89 deletions(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 9e0ec1fb8bf..415e6d74a7e 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -357,11 +357,11 @@ }, { "ImportPath": "github.com/spf13/cobra", - "Rev": "f8e1ec56bdd7494d309c69681267859a6bfb7549" + "Rev": "9e7273d5469dd5e04a35fd8823ba510117448c0b" }, { "ImportPath": "github.com/spf13/pflag", - "Rev": "370c3171201099fa6b466db45c8a032cbce33d8d" + "Rev": "11b7cf8387a31f278486eaad758162830eca8c73" }, { "ImportPath": "github.com/stretchr/objx", diff --git a/Godeps/_workspace/src/github.com/spf13/cobra/cobra_test.go b/Godeps/_workspace/src/github.com/spf13/cobra/cobra_test.go index 629865c2108..fa1192277c9 100644 --- a/Godeps/_workspace/src/github.com/spf13/cobra/cobra_test.go +++ b/Godeps/_workspace/src/github.com/spf13/cobra/cobra_test.go @@ -11,11 +11,14 @@ var _ = fmt.Println var tp, te, tt, t1 []string var flagb1, flagb2, flagb3, flagbr bool -var flags1, flags2, flags3 string +var flags1, flags2a, flags2b, flags3 string var flagi1, flagi2, flagi3, flagir int var globalFlag1 bool var flagEcho, rootcalled bool +const strtwoParentHelp = "help message for parent flag strtwo" +const strtwoChildHelp = "help message for child flag strtwo" + var cmdPrint = &Command{ Use: "print [string to print]", Short: "Print anything to the screen", @@ -72,11 +75,12 @@ func flagInit() { cmdRootNoRun.ResetFlags() cmdRootSameName.ResetFlags() cmdRootWithRun.ResetFlags() + cmdRootNoRun.PersistentFlags().StringVarP(&flags2a, "strtwo", "t", "two", strtwoParentHelp) cmdEcho.Flags().IntVarP(&flagi1, "intone", "i", 123, "help message for flag intone") cmdTimes.Flags().IntVarP(&flagi2, "inttwo", "j", 234, "help message for flag inttwo") cmdPrint.Flags().IntVarP(&flagi3, "intthree", "i", 345, "help message for flag intthree") cmdEcho.PersistentFlags().StringVarP(&flags1, "strone", "s", "one", "help message for flag strone") - cmdTimes.PersistentFlags().StringVarP(&flags2, "strtwo", "t", "two", "help message for flag strtwo") + cmdTimes.PersistentFlags().StringVarP(&flags2b, "strtwo", "t", "2", strtwoChildHelp) cmdPrint.PersistentFlags().StringVarP(&flags3, "strthree", "s", "three", "help message for flag strthree") cmdEcho.Flags().BoolVarP(&flagb1, "boolone", "b", true, "help message for flag boolone") cmdTimes.Flags().BoolVarP(&flagb2, "booltwo", "c", false, "help message for flag booltwo") @@ -377,10 +381,21 @@ func TestChildCommandFlags(t *testing.T) { t.Errorf("invalid flag should generate error") } - if !strings.Contains(r.Output, "intone=123") { + if !strings.Contains(r.Output, "unknown shorthand flag") { t.Errorf("Wrong error message displayed, \n %s", r.Output) } + // Testing with persistent flag overwritten by child + noRRSetupTest("echo times --strtwo=child one two") + + if flags2b != "child" { + t.Errorf("flag value should be child, %s given", flags2b) + } + + if flags2a != "two" { + t.Errorf("unset flag should have default value, expecting two, given %s", flags2a) + } + // Testing flag with invalid input r = noRRSetupTest("echo -i10E") @@ -437,6 +452,13 @@ func TestHelpCommand(t *testing.T) { checkResultContains(t, r, cmdTimes.Long) } +func TestChildCommandHelp(t *testing.T) { + c := noRRSetupTest("print --help") + checkResultContains(t, c, strtwoParentHelp) + r := noRRSetupTest("echo times --help") + checkResultContains(t, r, strtwoChildHelp) +} + func TestRunnableRootCommand(t *testing.T) { fullSetupTest("") @@ -486,6 +508,26 @@ func TestRootHelp(t *testing.T) { } +func TestFlagAccess(t *testing.T) { + initialize() + + local := cmdTimes.LocalFlags() + inherited := cmdTimes.InheritedFlags() + + for _, f := range []string{"inttwo", "strtwo", "booltwo"} { + if local.Lookup(f) == nil { + t.Errorf("LocalFlags expected to contain %s, Got: nil", f) + } + } + if inherited.Lookup("strone") == nil { + t.Errorf("InheritedFlags expected to contain strone, Got: nil") + } + if inherited.Lookup("strtwo") != nil { + t.Errorf("InheritedFlags shouldn not contain overwritten flag strtwo") + + } +} + func TestRootNoCommandHelp(t *testing.T) { x := rootOnlySetupTest("--help") diff --git a/Godeps/_workspace/src/github.com/spf13/cobra/command.go b/Godeps/_workspace/src/github.com/spf13/cobra/command.go index 6ac3ea70c55..82368e2673c 100644 --- a/Godeps/_workspace/src/github.com/spf13/cobra/command.go +++ b/Godeps/_workspace/src/github.com/spf13/cobra/command.go @@ -46,6 +46,8 @@ type Command struct { flags *flag.FlagSet // Set of flags childrens of this command will inherit pflags *flag.FlagSet + // Flags that are declared specifically by this command (not inherited). + lflags *flag.FlagSet // Run runs the command. // The args are the arguments after the command name. Run func(cmd *Command, args []string) @@ -218,8 +220,8 @@ Available Commands: {{range .Commands}}{{if .Runnable}} {{end}} {{ if .HasLocalFlags}}Flags: {{.LocalFlags.FlagUsages}}{{end}} -{{ if .HasAnyPersistentFlags}}Global Flags: -{{.AllPersistentFlags.FlagUsages}}{{end}}{{if .HasParent}}{{if and (gt .Commands 0) (gt .Parent.Commands 1) }} +{{ if .HasInheritedFlags}}Global Flags: +{{.InheritedFlags.FlagUsages}}{{end}}{{if .HasParent}}{{if and (gt .Commands 0) (gt .Parent.Commands 1) }} Additional help topics: {{if gt .Commands 0 }}{{range .Commands}}{{if not .Runnable}} {{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{if gt .Parent.Commands 1 }}{{range .Parent.Commands}}{{if .Runnable}}{{if not (eq .Name $cmd.Name) }}{{end}} {{rpad .CommandPath .CommandPathPadding}} {{.Short}}{{end}}{{end}}{{end}}{{end}} {{end}}{{ if .HasSubCommands }} @@ -249,7 +251,27 @@ func (c *Command) resetChildrensParents() { } } -func stripFlags(args []string) []string { +// Test if the named flag is a boolean flag. +func isBooleanFlag(name string, f *flag.FlagSet) bool { + flag := f.Lookup(name) + if flag == nil { + return false + } + return flag.Value.Type() == "bool" +} + +// Test if the named flag is a boolean flag. +func isBooleanShortFlag(name string, f *flag.FlagSet) bool { + result := false + f.VisitAll(func(f *flag.Flag) { + if f.Shorthand == name && f.Value.Type() == "bool" { + result = true + } + }) + return result +} + +func stripFlags(args []string, c *Command) []string { if len(args) < 1 { return args } @@ -257,6 +279,7 @@ func stripFlags(args []string) []string { commands := []string{} inQuote := false + inFlag := false for _, y := range args { if !inQuote { switch { @@ -264,8 +287,16 @@ func stripFlags(args []string) []string { inQuote = true case strings.Contains(y, "=\""): inQuote = true + case strings.HasPrefix(y, "--") && !strings.Contains(y, "="): + // TODO: this isn't quite right, we should really check ahead for 'true' or 'false' + inFlag = !isBooleanFlag(y[2:], c.Flags()) + case strings.HasPrefix(y, "-") && !strings.Contains(y, "=") && len(y) == 2 && !isBooleanShortFlag(y[1:], c.Flags()): + inFlag = true + case inFlag: + inFlag = false case !strings.HasPrefix(y, "-"): commands = append(commands, y) + inFlag = false } } @@ -303,7 +334,7 @@ func (c *Command) Find(arrs []string) (*Command, []string, error) { innerfind = func(c *Command, args []string) (*Command, []string) { if len(args) > 0 && c.HasSubCommands() { - argsWOflags := stripFlags(args) + argsWOflags := stripFlags(args, c) if len(argsWOflags) > 0 { matches := make([]*Command, 0) for _, cmd := range c.commands { @@ -372,7 +403,10 @@ func (c *Command) execute(a []string) (err error) { } err = c.ParseFlags(a) - + if err == flag.ErrHelp { + c.Help() + return nil + } if err != nil { // We're writing subcommand usage to root command's error buffer to have it displayed to the user r := c.Root() @@ -460,14 +494,15 @@ func (c *Command) Execute() (err error) { if e != nil { // Flags parsing had an error. // If an error happens here, we have to report it to the user - c.Println(c.errorMsgFromParse()) + c.Println(e.Error()) // If an error happens search also for subcommand info about that if c.cmdErrorBuf != nil && c.cmdErrorBuf.Len() > 0 { c.Println(c.cmdErrorBuf.String()) } else { c.Usage() } - return e + err = e + return } else { // If help is called, regardless of other flags, we print that if c.helpFlagVal { @@ -491,9 +526,13 @@ func (c *Command) Execute() (err error) { } if err != nil { - c.Println("Error:", err.Error()) - c.Printf("%v: invalid command %#q\n", c.Root().Name(), os.Args[1:]) - c.Printf("Run '%v help' for usage\n", c.Root().Name()) + if err == flag.ErrHelp { + c.Help() + } else { + c.Println("Error:", err.Error()) + c.Printf("%v: invalid command %#q\n", c.Root().Name(), os.Args[1:]) + c.Printf("Run '%v help' for usage\n", c.Root().Name()) + } } return @@ -553,6 +592,40 @@ func (c *Command) AddCommand(cmds ...*Command) { } } +// AddCommand removes one or more commands from a parent command. +func (c *Command) RemoveCommand(cmds ...*Command) { + commands := []*Command{} +main: + for _, command := range c.commands { + for _, cmd := range cmds { + if command == cmd { + command.parent = nil + continue main + } + } + commands = append(commands, command) + } + c.commands = commands + // recompute all lengths + c.commandsMaxUseLen = 0 + c.commandsMaxCommandPathLen = 0 + c.commandsMaxNameLen = 0 + for _, command := range c.commands { + usageLen := len(command.Use) + if usageLen > c.commandsMaxUseLen { + c.commandsMaxUseLen = usageLen + } + commandPathLen := len(command.CommandPath()) + if commandPathLen > c.commandsMaxCommandPathLen { + c.commandsMaxCommandPathLen = commandPathLen + } + nameLen := len(command.Name()) + if nameLen > c.commandsMaxNameLen { + c.commandsMaxNameLen = nameLen + } + } +} + // Convenience method to Print to the defined output func (c *Command) Print(i ...interface{}) { fmt.Fprint(c.Out(), i...) @@ -726,14 +799,9 @@ func (c *Command) LocalFlags() *flag.FlagSet { c.mergePersistentFlags() local := flag.NewFlagSet(c.Name(), flag.ContinueOnError) - allPersistent := c.AllPersistentFlags() - - c.Flags().VisitAll(func(f *flag.Flag) { - if allPersistent.Lookup(f.Name) == nil { - local.AddFlag(f) - } + c.lflags.VisitAll(func(f *flag.Flag) { + local.AddFlag(f) }) - return local } @@ -741,44 +809,34 @@ func (c *Command) LocalFlags() *flag.FlagSet { func (c *Command) InheritedFlags() *flag.FlagSet { c.mergePersistentFlags() - local := flag.NewFlagSet(c.Name(), flag.ContinueOnError) + inherited := flag.NewFlagSet(c.Name(), flag.ContinueOnError) + local := c.LocalFlags() - var rmerge func(x *Command) + var rmerge func(x *Command) - rmerge = func(x *Command) { - if x.HasPersistentFlags() { - x.PersistentFlags().VisitAll(func(f *flag.Flag) { - if local.Lookup(f.Name) == nil { - local.AddFlag(f) - } - }) - } - if x.HasParent() { - rmerge(x.parent) - } - } + rmerge = func(x *Command) { + if x.HasPersistentFlags() { + x.PersistentFlags().VisitAll(func(f *flag.Flag) { + if inherited.Lookup(f.Name) == nil && local.Lookup(f.Name) == nil { + inherited.AddFlag(f) + } + }) + } + if x.HasParent() { + rmerge(x.parent) + } + } if c.HasParent() { rmerge(c.parent) } - return local + return inherited } // All Flags which were not inherited from parent commands func (c *Command) NonInheritedFlags() *flag.FlagSet { - c.mergePersistentFlags() - - local := flag.NewFlagSet(c.Name(), flag.ContinueOnError) - inheritedFlags := c.InheritedFlags() - - c.Flags().VisitAll(func(f *flag.Flag) { - if inheritedFlags.Lookup(f.Name) == nil { - local.AddFlag(f) - } - }) - - return local + return c.LocalFlags() } // Get the Persistent FlagSet specifically set in the current command @@ -793,29 +851,6 @@ func (c *Command) PersistentFlags() *flag.FlagSet { return c.pflags } -// Get the Persistent FlagSet traversing the Command hierarchy -func (c *Command) AllPersistentFlags() *flag.FlagSet { - allPersistent := flag.NewFlagSet(c.Name(), flag.ContinueOnError) - - var visit func(x *Command) - visit = func(x *Command) { - if x.HasPersistentFlags() { - x.PersistentFlags().VisitAll(func(f *flag.Flag) { - if allPersistent.Lookup(f.Name) == nil { - allPersistent.AddFlag(f) - } - }) - } - if x.HasParent() { - visit(x.parent) - } - } - - visit(c) - - return allPersistent -} - // For use in testing func (c *Command) ResetFlags() { c.flagErrorBuf = new(bytes.Buffer) @@ -836,16 +871,15 @@ func (c *Command) HasPersistentFlags() bool { return c.PersistentFlags().HasFlags() } -// Does the command hierarchy contain persistent flags -func (c *Command) HasAnyPersistentFlags() bool { - return c.AllPersistentFlags().HasFlags() -} - // Does the command has flags specifically declared locally func (c *Command) HasLocalFlags() bool { return c.LocalFlags().HasFlags() } +func (c *Command) HasInheritedFlags() bool { + return c.InheritedFlags().HasFlags() +} + // Climbs up the command tree looking for matching flag func (c *Command) Flag(name string) (flag *flag.Flag) { flag = c.Flags().Lookup(name) @@ -873,16 +907,7 @@ func (c *Command) persistentFlag(name string) (flag *flag.Flag) { func (c *Command) ParseFlags(args []string) (err error) { c.mergePersistentFlags() err = c.Flags().Parse(args) - - // The upstream library adds spaces to the error - // response regardless of success. - // Handling it here until fixing upstream - if len(strings.TrimSpace(c.flagErrorBuf.String())) > 1 { - return fmt.Errorf("%s", c.flagErrorBuf.String()) - } - - //always return nil because upstream library is inconsistent & we always check the error buffer anyway - return nil + return } func (c *Command) Parent() *Command { @@ -892,6 +917,19 @@ func (c *Command) Parent() *Command { func (c *Command) mergePersistentFlags() { var rmerge func(x *Command) + // Save the set of local flags + if c.lflags == nil { + c.lflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) + if c.flagErrorBuf == nil { + c.flagErrorBuf = new(bytes.Buffer) + } + c.lflags.SetOutput(c.flagErrorBuf) + addtolocal := func(f *flag.Flag) { + c.lflags.AddFlag(f) + } + c.Flags().VisitAll(addtolocal) + c.PersistentFlags().VisitAll(addtolocal) + } rmerge = func(x *Command) { if x.HasPersistentFlags() { x.PersistentFlags().VisitAll(func(f *flag.Flag) { diff --git a/Godeps/_workspace/src/github.com/spf13/pflag/bool_test.go b/Godeps/_workspace/src/github.com/spf13/pflag/bool_test.go index 72a12beca98..fe45b8d6e6a 100644 --- a/Godeps/_workspace/src/github.com/spf13/pflag/bool_test.go +++ b/Godeps/_workspace/src/github.com/spf13/pflag/bool_test.go @@ -9,7 +9,7 @@ import ( "strconv" "testing" - . "github.com/ogier/pflag" + . "github.com/spf13/pflag" ) // This value can be a boolean ("true", "false") or "maybe" diff --git a/Godeps/_workspace/src/github.com/spf13/pflag/example_test.go b/Godeps/_workspace/src/github.com/spf13/pflag/example_test.go index 6aaed3c8082..9be7a49f26f 100644 --- a/Godeps/_workspace/src/github.com/spf13/pflag/example_test.go +++ b/Godeps/_workspace/src/github.com/spf13/pflag/example_test.go @@ -11,7 +11,7 @@ import ( "strings" "time" - flag "github.com/ogier/pflag" + flag "github.com/spf13/pflag" ) // Example 1: A single string flag called "species" with default value "gopher". diff --git a/Godeps/_workspace/src/github.com/spf13/pflag/flag.go b/Godeps/_workspace/src/github.com/spf13/pflag/flag.go index ad65ddad2df..72165f6c8a7 100644 --- a/Godeps/_workspace/src/github.com/spf13/pflag/flag.go +++ b/Godeps/_workspace/src/github.com/spf13/pflag/flag.go @@ -498,6 +498,7 @@ func (f *FlagSet) parseShortArg(s string, args []string) (a []string, err error) if len(args) == 0 { return } + return } if alreadythere { if bv, ok := flag.Value.(boolFlag); ok && bv.IsBoolFlag() { @@ -551,6 +552,9 @@ func (f *FlagSet) parseArgs(args []string) (err error) { } else { args, err = f.parseShortArg(s, args) } + if err != nil { + return + } } return }