Add only missing persistent flags of parents in mergePersistentFlags

As persistent flags of parents can only be added, we don't need to always
check them every time, so make updateParentsPflags return only added flags.

Performance improvement:
benchmark                     old ns/op     new ns/op     delta
BenchmarkInheritedFlags-4     5595          4412          -21.14%
BenchmarkLocalFlags-4         3235          2667          -17.56%

benchmark                     old allocs     new allocs     delta
BenchmarkInheritedFlags-4     39             24             -38.46%
BenchmarkLocalFlags-4         21             15             -28.57%

benchmark                     old bytes     new bytes     delta
BenchmarkInheritedFlags-4     1000          600           -40.00%
BenchmarkLocalFlags-4         544           408           -25.00%
This commit is contained in:
Albert Nigmatzianov 2017-04-06 15:28:17 +02:00
parent 6202b5942b
commit 3d89ed4908
2 changed files with 32 additions and 19 deletions

View file

@ -675,7 +675,7 @@ func TestPersistentFlags(t *testing.T) {
fullSetupTest("echo times -s again -c -p test here") fullSetupTest("echo times -s again -c -p test here")
if strings.Join(tt, " ") != "test here" { if strings.Join(tt, " ") != "test here" {
t.Errorf("flags didn't leave proper args remaining..%s given", tt) t.Errorf("flags didn't leave proper args remaining. %s given", tt)
} }
if flags1 != "again" { if flags1 != "again" {

View file

@ -772,6 +772,7 @@ func (c *Command) initHelpCmd() {
func (c *Command) ResetCommands() { func (c *Command) ResetCommands() {
c.commands = nil c.commands = nil
c.helpCommand = nil c.helpCommand = nil
c.parentsPflags = nil
} }
// Sorts commands by their names. // Sorts commands by their names.
@ -1083,7 +1084,6 @@ func (c *Command) LocalNonPersistentFlags() *flag.FlagSet {
// LocalFlags returns the local FlagSet specifically set in the current command. // LocalFlags returns the local FlagSet specifically set in the current command.
func (c *Command) LocalFlags() *flag.FlagSet { func (c *Command) LocalFlags() *flag.FlagSet {
c.mergePersistentFlags() c.mergePersistentFlags()
c.updateParentsPersistentFlags()
if c.lflags == nil { if c.lflags == nil {
c.lflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) c.lflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError)
@ -1106,7 +1106,6 @@ func (c *Command) LocalFlags() *flag.FlagSet {
// InheritedFlags returns all flags which were inherited from parents commands. // InheritedFlags returns all flags which were inherited from parents commands.
func (c *Command) InheritedFlags() *flag.FlagSet { func (c *Command) InheritedFlags() *flag.FlagSet {
c.mergePersistentFlags() c.mergePersistentFlags()
c.updateParentsPersistentFlags()
if c.iflags == nil { if c.iflags == nil {
c.iflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) c.iflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError)
@ -1226,10 +1225,37 @@ func (c *Command) Parent() *Command {
return c.parent return c.parent
} }
func (c *Command) updateParentsPersistentFlags() { // mergePersistentFlags merges c.PersistentFlags() to c.Flags()
// and adds missing persistent flags of all parents.
func (c *Command) mergePersistentFlags() {
if c.HasPersistentFlags() {
c.PersistentFlags().VisitAll(func(f *flag.Flag) {
if c.Flags().Lookup(f.Name) == nil {
c.Flags().AddFlag(f)
}
})
}
added := c.updateParentsPflags()
if len(added) > 0 {
for _, f := range added {
if c.Flags().Lookup(f.Name) == nil {
c.Flags().AddFlag(f)
}
}
}
}
// updateParentsPflags updates c.parentsPflags by adding
// new persistent flags of all parents and returns added flags.
// If c.parentsPflags == nil, it makes new.
//
// This function must be used ONLY in mergePersistentFlags.
func (c *Command) updateParentsPflags() (added []*flag.Flag) {
if c.parentsPflags == nil { if c.parentsPflags == nil {
c.parentsPflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError) c.parentsPflags = flag.NewFlagSet(c.Name(), flag.ContinueOnError)
c.parentsPflags.SetOutput(c.OutOrStderr()) c.parentsPflags.SetOutput(c.OutOrStderr())
c.parentsPflags.SortFlags = false
} }
c.VisitParents(func(x *Command) { c.VisitParents(func(x *Command) {
@ -1237,24 +1263,11 @@ func (c *Command) updateParentsPersistentFlags() {
x.PersistentFlags().VisitAll(func(f *flag.Flag) { x.PersistentFlags().VisitAll(func(f *flag.Flag) {
if c.parentsPflags.Lookup(f.Name) == nil { if c.parentsPflags.Lookup(f.Name) == nil {
c.parentsPflags.AddFlag(f) c.parentsPflags.AddFlag(f)
added = append(added, f)
} }
}) })
} }
}) })
}
func (c *Command) mergePersistentFlags() { return added
flags := c.Flags()
merge := func(x *Command) {
if x.HasPersistentFlags() {
x.PersistentFlags().VisitAll(func(f *flag.Flag) {
if flags.Lookup(f.Name) == nil {
flags.AddFlag(f)
}
})
}
}
merge(c)
c.VisitParents(merge)
} }