From bef835d455a7351fc745869c5a9fd456f2bff4a4 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Mon, 16 May 2016 10:18:15 -0400 Subject: [PATCH 1/4] Remove all Context.Global* methods and change the behavior of the non-Global variants to always search up the context lineage. Closes #385 --- CHANGELOG.md | 11 ++ altsrc/helpers_test.go | 14 ++- context.go | 221 ++++++++++++++++------------------------- context_test.go | 159 ++++++----------------------- flag_test.go | 21 ++-- help.go | 8 +- 6 files changed, 155 insertions(+), 279 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d089c0..4a6b2f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,12 @@ ## 2.0.0 - (unreleased 2.x series) ### Added - `NewStringSlice` and `NewIntSlice` for creating their related types +- `Context.Lineage` to get all contexts from current up to global +- `Context.LocalFlagNames` to get the flag names from *only* the current context + +### Changed +- `Context.FlagNames` now returns all flags in the context lineage +- `Context.IsSet` now considers the full context lineage ### Removed - the ability to specify `&StringSlice{...string}` or `&IntSlice{...int}`. @@ -17,6 +23,11 @@ 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. +- All `Context.Global*` methods, as the non-global versions now traverse up + the context lineage automatically. + +### Fixed +- `Context.BoolT` now returns `true` when not found ## [Unreleased] - (1.x series) ### Added diff --git a/altsrc/helpers_test.go b/altsrc/helpers_test.go index 3b7f7e9..33e8a4b 100644 --- a/altsrc/helpers_test.go +++ b/altsrc/helpers_test.go @@ -1,13 +1,23 @@ package altsrc import ( + "os" "reflect" + "runtime" + "strings" "testing" ) +var ( + wd, _ = os.Getwd() +) + func expect(t *testing.T, a interface{}, b interface{}) { - if !reflect.DeepEqual(b, a) { - t.Errorf("Expected %#v (type %v) - Got %#v (type %v)", b, reflect.TypeOf(b), a, reflect.TypeOf(a)) + _, fn, line, _ := runtime.Caller(1) + fn = strings.Replace(fn, wd+"/", "", -1) + + if !reflect.DeepEqual(a, b) { + t.Errorf("(%s:%d) Expected %v (type %v) - Got %v (type %v)", fn, line, b, reflect.TypeOf(b), a, reflect.TypeOf(a)) } } diff --git a/context.go b/context.go index 87ad6d4..af80bdb 100644 --- a/context.go +++ b/context.go @@ -28,129 +28,76 @@ func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context { // Int looks up the value of a local int flag, returns 0 if no int flag exists func (c *Context) Int(name string) int { - return lookupInt(name, c.flagSet) + if fs := lookupFlagSet(name, c); fs != nil { + return lookupInt(name, fs) + } + return 0 } // Duration looks up the value of a local time.Duration flag, returns 0 if no // time.Duration flag exists func (c *Context) Duration(name string) time.Duration { - return lookupDuration(name, c.flagSet) + if fs := lookupFlagSet(name, c); fs != nil { + return lookupDuration(name, fs) + } + return 0 } // Float64 looks up the value of a local float64 flag, returns 0 if no float64 // flag exists func (c *Context) Float64(name string) float64 { - return lookupFloat64(name, c.flagSet) -} - -// Bool looks up the value of a local bool flag, returns false if no bool flag exists -func (c *Context) Bool(name string) bool { - return lookupBool(name, c.flagSet) -} - -// BoolT looks up the value of a local boolT flag, returns false if no bool flag exists -func (c *Context) BoolT(name string) bool { - return lookupBoolT(name, c.flagSet) -} - -// String looks up the value of a local string flag, returns "" if no string flag exists -func (c *Context) String(name string) string { - return lookupString(name, c.flagSet) -} - -// StringSlice looks up the value of a local string slice flag, returns nil if no -// string slice flag exists -func (c *Context) StringSlice(name string) []string { - return lookupStringSlice(name, c.flagSet) -} - -// IntSlice looks up the value of a local int slice flag, returns nil if no int -// slice flag exists -func (c *Context) IntSlice(name string) []int { - return lookupIntSlice(name, c.flagSet) -} - -// Generic looks up the value of a local generic flag, returns nil if no generic -// flag exists -func (c *Context) Generic(name string) interface{} { - return lookupGeneric(name, c.flagSet) -} - -// GlobalInt looks up the value of a global int flag, returns 0 if no int flag exists -func (c *Context) GlobalInt(name string) int { - if fs := lookupGlobalFlagSet(name, c); fs != nil { - return lookupInt(name, fs) - } - return 0 -} - -// GlobalFloat64 looks up the value of a global float64 flag, returns float64(0) -// if no float64 flag exists -func (c *Context) GlobalFloat64(name string) float64 { - if fs := lookupGlobalFlagSet(name, c); fs != nil { + if fs := lookupFlagSet(name, c); fs != nil { return lookupFloat64(name, fs) } - return float64(0) -} - -// GlobalDuration looks up the value of a global time.Duration flag, returns 0 -// if no time.Duration flag exists -func (c *Context) GlobalDuration(name string) time.Duration { - if fs := lookupGlobalFlagSet(name, c); fs != nil { - return lookupDuration(name, fs) - } return 0 } -// GlobalBool looks up the value of a global bool flag, returns false if no bool -// flag exists -func (c *Context) GlobalBool(name string) bool { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// Bool looks up the value of a local bool flag, returns false if no bool flag exists +func (c *Context) Bool(name string) bool { + if fs := lookupFlagSet(name, c); fs != nil { return lookupBool(name, fs) } return false } -// GlobalBoolT looks up the value of a global bool flag, returns true if no bool -// flag exists -func (c *Context) GlobalBoolT(name string) bool { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// BoolT looks up the value of a local boolT flag, returns false if no bool flag exists +func (c *Context) BoolT(name string) bool { + if fs := lookupFlagSet(name, c); fs != nil { return lookupBoolT(name, fs) } - return false + return true } -// GlobalString looks up the value of a global string flag, returns "" if no -// string flag exists -func (c *Context) GlobalString(name string) string { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// String looks up the value of a local string flag, returns "" if no string flag exists +func (c *Context) String(name string) string { + if fs := lookupFlagSet(name, c); fs != nil { return lookupString(name, fs) } return "" } -// GlobalStringSlice looks up the value of a global string slice flag, returns -// nil if no string slice flag exists -func (c *Context) GlobalStringSlice(name string) []string { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// StringSlice looks up the value of a local string slice flag, returns nil if no +// string slice flag exists +func (c *Context) StringSlice(name string) []string { + if fs := lookupFlagSet(name, c); fs != nil { return lookupStringSlice(name, fs) } return nil } -// GlobalIntSlice looks up the value of a global int slice flag, returns nil if -// no int slice flag exists -func (c *Context) GlobalIntSlice(name string) []int { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// IntSlice looks up the value of a local int slice flag, returns nil if no int +// slice flag exists +func (c *Context) IntSlice(name string) []int { + if fs := lookupFlagSet(name, c); fs != nil { return lookupIntSlice(name, fs) } return nil } -// GlobalGeneric looks up the value of a global generic flag, returns nil if no -// generic flag exists -func (c *Context) GlobalGeneric(name string) interface{} { - if fs := lookupGlobalFlagSet(name, c); fs != nil { +// Generic looks up the value of a local generic flag, returns nil if no generic +// flag exists +func (c *Context) Generic(name string) interface{} { + if fs := lookupFlagSet(name, c); fs != nil { return lookupGeneric(name, fs) } return nil @@ -166,61 +113,37 @@ func (c *Context) Set(name, value string) error { return c.flagSet.Set(name, value) } -// GlobalSet sets a context flag to a value on the global flagset -func (c *Context) GlobalSet(name, value string) error { - return globalContext(c).flagSet.Set(name, value) -} - // IsSet determines if the flag was actually set func (c *Context) IsSet(name string) bool { - if c.setFlags == nil { - c.setFlags = make(map[string]bool) + if fs := lookupFlagSet(name, c); fs != nil { + isSet := false c.flagSet.Visit(func(f *flag.Flag) { - c.setFlags[f.Name] = true + if f.Name == name { + isSet = true + } }) + return isSet } - return c.setFlags[name] == true + return false } -// GlobalIsSet determines if the global flag was actually set -func (c *Context) GlobalIsSet(name string) bool { - if c.globalSetFlags == nil { - c.globalSetFlags = make(map[string]bool) - ctx := c - if ctx.parentContext != nil { - ctx = ctx.parentContext - } - for ; ctx != nil && c.globalSetFlags[name] == false; ctx = ctx.parentContext { - ctx.flagSet.Visit(func(f *flag.Flag) { - c.globalSetFlags[f.Name] = true - }) - } - } - return c.globalSetFlags[name] +// LocalFlagNames returns a slice of flag names used in this context. +func (c *Context) LocalFlagNames() []string { + names := []string{} + c.flagSet.Visit(makeFlagNameVisitor(&names)) + return names } -// FlagNames returns a slice of flag names used in this context. -func (c *Context) FlagNames() (names []string) { - for _, flag := range c.Command.Flags { - name := strings.Split(flag.GetName(), ",")[0] - if name == "help" { - continue - } - names = append(names, name) - } - return -} +// FlagNames returns a slice of flag names used by the this context and all of +// its parent contexts. +func (c *Context) FlagNames() []string { + names := []string{} -// GlobalFlagNames returns a slice of global flag names used by the app. -func (c *Context) GlobalFlagNames() (names []string) { - for _, flag := range c.App.Flags { - name := strings.Split(flag.GetName(), ",")[0] - if name == "help" || name == "version" { - continue - } - names = append(names, name) + for _, ctx := range c.Lineage() { + ctx.flagSet.Visit(makeFlagNameVisitor(&names)) } - return + + return names } // Parent returns the parent context, if any @@ -228,6 +151,25 @@ func (c *Context) Parent() *Context { return c.parentContext } +// Lineage returns *this* context and all of its ancestor contexts in order from +// child to parent +func (c *Context) Lineage() []*Context { + lineage := []*Context{} + cur := c + + for { + lineage = append(lineage, cur) + + if cur.parentContext == nil { + break + } + + cur = cur.parentContext + } + + return lineage +} + // Args contains apps console arguments type Args []string @@ -291,15 +233,13 @@ func globalContext(ctx *Context) *Context { } } -func lookupGlobalFlagSet(name string, ctx *Context) *flag.FlagSet { - if ctx.parentContext != nil { - ctx = ctx.parentContext - } - for ; ctx != nil; ctx = ctx.parentContext { - if f := ctx.flagSet.Lookup(name); f != nil { - return ctx.flagSet +func lookupFlagSet(name string, ctx *Context) *flag.FlagSet { + for _, c := range ctx.Lineage() { + if f := c.flagSet.Lookup(name); f != nil { + return c.flagSet } } + return nil } @@ -401,7 +341,7 @@ func lookupBoolT(name string, set *flag.FlagSet) bool { return val } - return false + return true } func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) { @@ -445,3 +385,12 @@ func normalizeFlags(flags []Flag, set *flag.FlagSet) error { } return nil } + +func makeFlagNameVisitor(names *[]string) func(*flag.Flag) { + return func(f *flag.Flag) { + name := strings.Split(f.Name, ",")[0] + if name != "help" && name != "version" { + (*names) = append(*names, name) + } + } +} diff --git a/context_test.go b/context_test.go index 28d4884..e819dbe 100644 --- a/context_test.go +++ b/context_test.go @@ -2,6 +2,7 @@ package cli import ( "flag" + "sort" "testing" "time" ) @@ -19,8 +20,6 @@ func TestNewContext(t *testing.T) { c.Command = command expect(t, c.Int("myflag"), 12) expect(t, c.Float64("myflag64"), float64(17)) - expect(t, c.GlobalInt("myflag"), 42) - expect(t, c.GlobalFloat64("myflag64"), float64(47)) expect(t, c.Command.Name, "mycommand") } @@ -31,14 +30,6 @@ func TestContext_Int(t *testing.T) { expect(t, c.Int("myflag"), 12) } -func TestContext_GlobalInt(t *testing.T) { - set := flag.NewFlagSet("test", 0) - set.Int("myflag", 12, "doc") - c := NewContext(nil, set, nil) - expect(t, c.GlobalInt("myflag"), 12) - expect(t, c.GlobalInt("nope"), 0) -} - func TestContext_Float64(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Float64("myflag", float64(17), "doc") @@ -46,14 +37,6 @@ func TestContext_Float64(t *testing.T) { expect(t, c.Float64("myflag"), float64(17)) } -func TestContext_GlobalFloat64(t *testing.T) { - set := flag.NewFlagSet("test", 0) - set.Float64("myflag", float64(17), "doc") - c := NewContext(nil, set, nil) - expect(t, c.GlobalFloat64("myflag"), float64(17)) - expect(t, c.GlobalFloat64("nope"), float64(0)) -} - func TestContext_Duration(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Duration("myflag", time.Duration(12*time.Second), "doc") @@ -82,30 +65,6 @@ func TestContext_BoolT(t *testing.T) { expect(t, c.BoolT("myflag"), true) } -func TestContext_GlobalBool(t *testing.T) { - set := flag.NewFlagSet("test", 0) - - globalSet := flag.NewFlagSet("test-global", 0) - globalSet.Bool("myflag", false, "doc") - globalCtx := NewContext(nil, globalSet, nil) - - c := NewContext(nil, set, globalCtx) - expect(t, c.GlobalBool("myflag"), false) - expect(t, c.GlobalBool("nope"), false) -} - -func TestContext_GlobalBoolT(t *testing.T) { - set := flag.NewFlagSet("test", 0) - - globalSet := flag.NewFlagSet("test-global", 0) - globalSet.Bool("myflag", true, "doc") - globalCtx := NewContext(nil, globalSet, nil) - - c := NewContext(nil, set, globalCtx) - expect(t, c.GlobalBoolT("myflag"), true) - expect(t, c.GlobalBoolT("nope"), false) -} - func TestContext_Args(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") @@ -139,25 +98,6 @@ func TestContext_IsSet(t *testing.T) { expect(t, c.IsSet("myflagGlobal"), false) } -func TestContext_GlobalIsSet(t *testing.T) { - set := flag.NewFlagSet("test", 0) - set.Bool("myflag", false, "doc") - set.String("otherflag", "hello world", "doc") - globalSet := flag.NewFlagSet("test", 0) - globalSet.Bool("myflagGlobal", true, "doc") - globalSet.Bool("myflagGlobalUnset", true, "doc") - globalCtx := NewContext(nil, globalSet, nil) - c := NewContext(nil, set, globalCtx) - set.Parse([]string{"--myflag", "bat", "baz"}) - globalSet.Parse([]string{"--myflagGlobal", "bat", "baz"}) - expect(t, c.GlobalIsSet("myflag"), false) - expect(t, c.GlobalIsSet("otherflag"), false) - expect(t, c.GlobalIsSet("bogusflag"), false) - expect(t, c.GlobalIsSet("myflagGlobal"), true) - expect(t, c.GlobalIsSet("myflagGlobalUnset"), false) - expect(t, c.GlobalIsSet("bogusGlobal"), false) -} - func TestContext_NumFlags(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("myflag", false, "doc") @@ -171,62 +111,6 @@ func TestContext_NumFlags(t *testing.T) { expect(t, c.NumFlags(), 2) } -func TestContext_GlobalFlag(t *testing.T) { - var globalFlag string - var globalFlagSet bool - app := NewApp() - app.Flags = []Flag{ - StringFlag{Name: "global, g", Usage: "global"}, - } - app.Action = func(c *Context) error { - globalFlag = c.GlobalString("global") - globalFlagSet = c.GlobalIsSet("global") - return nil - } - app.Run([]string{"command", "-g", "foo"}) - expect(t, globalFlag, "foo") - expect(t, globalFlagSet, true) - -} - -func TestContext_GlobalFlagsInSubcommands(t *testing.T) { - subcommandRun := false - parentFlag := false - app := NewApp() - - app.Flags = []Flag{ - BoolFlag{Name: "debug, d", Usage: "Enable debugging"}, - } - - app.Commands = []Command{ - { - Name: "foo", - Flags: []Flag{ - BoolFlag{Name: "parent, p", Usage: "Parent flag"}, - }, - Subcommands: []Command{ - { - Name: "bar", - Action: func(c *Context) error { - if c.GlobalBool("debug") { - subcommandRun = true - } - if c.GlobalBool("parent") { - parentFlag = true - } - return nil - }, - }, - }, - }, - } - - app.Run([]string{"command", "-d", "foo", "-p", "bar"}) - - expect(t, subcommandRun, true) - expect(t, parentFlag, true) -} - func TestContext_Set(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Int("int", 5, "an int") @@ -236,21 +120,36 @@ func TestContext_Set(t *testing.T) { expect(t, c.Int("int"), 1) } -func TestContext_GlobalSet(t *testing.T) { - gSet := flag.NewFlagSet("test", 0) - gSet.Int("int", 5, "an int") +func TestContext_LocalFlagNames(t *testing.T) { + set := flag.NewFlagSet("test", 0) + set.Bool("one-flag", false, "doc") + set.String("two-flag", "hello world", "doc") + globalSet := flag.NewFlagSet("test", 0) + globalSet.Bool("top-flag", true, "doc") + globalCtx := NewContext(nil, globalSet, nil) + c := NewContext(nil, set, globalCtx) + set.Parse([]string{"--one-flag", "--two-flag=foo"}) + globalSet.Parse([]string{"--top-flag"}) - set := flag.NewFlagSet("sub", 0) - set.Int("int", 3, "an int") + actualFlags := c.LocalFlagNames() + sort.Strings(actualFlags) - pc := NewContext(nil, gSet, nil) - c := NewContext(nil, set, pc) + expect(t, actualFlags, []string{"one-flag", "two-flag"}) +} - c.Set("int", "1") - expect(t, c.Int("int"), 1) - expect(t, c.GlobalInt("int"), 5) +func TestContext_FlagNames(t *testing.T) { + set := flag.NewFlagSet("test", 0) + set.Bool("one-flag", false, "doc") + set.String("two-flag", "hello world", "doc") + globalSet := flag.NewFlagSet("test", 0) + globalSet.Bool("top-flag", true, "doc") + globalCtx := NewContext(nil, globalSet, nil) + c := NewContext(nil, set, globalCtx) + set.Parse([]string{"--one-flag", "--two-flag=foo"}) + globalSet.Parse([]string{"--top-flag"}) - c.GlobalSet("int", "1") - expect(t, c.Int("int"), 1) - expect(t, c.GlobalInt("int"), 1) + actualFlags := c.FlagNames() + sort.Strings(actualFlags) + + expect(t, actualFlags, []string{"one-flag", "top-flag", "two-flag"}) } diff --git a/flag_test.go b/flag_test.go index f914da3..f163468 100644 --- a/flag_test.go +++ b/flag_test.go @@ -391,7 +391,7 @@ func TestParseMultiStringSliceWithDefaults(t *testing.T) { Flags: []Flag{ StringSliceFlag{Name: "serve, s", Value: NewStringSlice("9", "2")}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { expected := []string{"10", "20"} if !reflect.DeepEqual(ctx.StringSlice("serve"), expected) { t.Errorf("main name not set: %v != %v", expected, ctx.StringSlice("serve")) @@ -399,6 +399,7 @@ func TestParseMultiStringSliceWithDefaults(t *testing.T) { if !reflect.DeepEqual(ctx.StringSlice("s"), expected) { t.Errorf("short name not set: %v != %v", expected, ctx.StringSlice("s")) } + return nil }, }).Run([]string{"run", "-s", "10", "-s", "20"}) } @@ -408,13 +409,14 @@ func TestParseMultiStringSliceWithDefaultsUnset(t *testing.T) { Flags: []Flag{ StringSliceFlag{Name: "serve, s", Value: NewStringSlice("9", "2")}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.StringSlice("serve"), []string{"9", "2"}) { t.Errorf("main name not set: %v", ctx.StringSlice("serve")) } if !reflect.DeepEqual(ctx.StringSlice("s"), []string{"9", "2"}) { t.Errorf("short name not set: %v", ctx.StringSlice("s")) } + return nil }, }).Run([]string{"run"}) } @@ -427,13 +429,14 @@ func TestParseMultiStringSliceFromEnv(t *testing.T) { Flags: []Flag{ StringSliceFlag{Name: "intervals, i", Value: NewStringSlice(), EnvVar: "APP_INTERVALS"}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.StringSlice("i"), []string{"20", "30", "40"}) { t.Errorf("short name not set from env") } + return nil }, }).Run([]string{"run"}) } @@ -466,13 +469,14 @@ func TestParseMultiStringSliceFromEnvCascade(t *testing.T) { Flags: []Flag{ StringSliceFlag{Name: "intervals, i", Value: NewStringSlice(), EnvVar: "COMPAT_INTERVALS,APP_INTERVALS"}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.StringSlice("intervals"), []string{"20", "30", "40"}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.StringSlice("i"), []string{"20", "30", "40"}) { t.Errorf("short name not set from env") } + return nil }, }).Run([]string{"run"}) } @@ -596,13 +600,14 @@ func TestParseMultiIntSliceWithDefaults(t *testing.T) { Flags: []Flag{ IntSliceFlag{Name: "serve, s", Value: NewIntSlice(9, 2)}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{10, 20}) { t.Errorf("main name not set") } if !reflect.DeepEqual(ctx.IntSlice("s"), []int{10, 20}) { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run", "-s", "10", "-s", "20"}) } @@ -612,13 +617,14 @@ func TestParseMultiIntSliceWithDefaultsUnset(t *testing.T) { Flags: []Flag{ IntSliceFlag{Name: "serve, s", Value: NewIntSlice(9, 2)}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("serve"), []int{9, 2}) { t.Errorf("main name not set") } if !reflect.DeepEqual(ctx.IntSlice("s"), []int{9, 2}) { t.Errorf("short name not set") } + return nil }, }).Run([]string{"run"}) } @@ -631,13 +637,14 @@ func TestParseMultiIntSliceFromEnv(t *testing.T) { Flags: []Flag{ IntSliceFlag{Name: "intervals, i", Value: NewIntSlice(), EnvVar: "APP_INTERVALS"}, }, - Action: func(ctx *Context) { + Action: func(ctx *Context) error { if !reflect.DeepEqual(ctx.IntSlice("intervals"), []int{20, 30, 40}) { t.Errorf("main name not set from env") } if !reflect.DeepEqual(ctx.IntSlice("i"), []int{20, 30, 40}) { t.Errorf("short name not set from env") } + return nil }, }).Run([]string{"run"}) } diff --git a/help.go b/help.go index a9e7327..893b1be 100644 --- a/help.go +++ b/help.go @@ -209,7 +209,7 @@ func checkVersion(c *Context) bool { found := false if VersionFlag.Name != "" { eachName(VersionFlag.Name, func(name string) { - if c.GlobalBool(name) || c.Bool(name) { + if c.Bool(name) { found = true } }) @@ -221,7 +221,7 @@ func checkHelp(c *Context) bool { found := false if HelpFlag.Name != "" { eachName(HelpFlag.Name, func(name string) { - if c.GlobalBool(name) || c.Bool(name) { + if c.Bool(name) { found = true } }) @@ -239,7 +239,7 @@ func checkCommandHelp(c *Context, name string) bool { } func checkSubcommandHelp(c *Context) bool { - if c.GlobalBool("h") || c.GlobalBool("help") { + if c.Bool("h") || c.Bool("help") { ShowSubcommandHelp(c) return true } @@ -248,7 +248,7 @@ func checkSubcommandHelp(c *Context) bool { } func checkCompletions(c *Context) bool { - if (c.GlobalBool(BashCompletionFlag.Name) || c.Bool(BashCompletionFlag.Name)) && c.App.EnableBashCompletion { + if c.Bool(BashCompletionFlag.Name) && c.App.EnableBashCompletion { ShowCompletions(c) return true } From c720f37e15082fbc5d89d0b7f836db0bc15c78d1 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 17 May 2016 03:16:26 -0400 Subject: [PATCH 2/4] Context tests and tweaks around lineage traversal per feedback on #410 --- context.go | 40 ++++------------------ context_test.go | 89 ++++++++++++++++++++++++++++++++++++------------- 2 files changed, 72 insertions(+), 57 deletions(-) diff --git a/context.go b/context.go index af80bdb..002fe6e 100644 --- a/context.go +++ b/context.go @@ -13,12 +13,11 @@ import ( // can be used to retrieve context-specific Args and // parsed command-line options. type Context struct { - App *App - Command Command - flagSet *flag.FlagSet - setFlags map[string]bool - globalSetFlags map[string]bool - parentContext *Context + App *App + Command Command + + flagSet *flag.FlagSet + parentContext *Context } // NewContext creates a new context. For use in when invoking an App or Command action. @@ -117,7 +116,7 @@ func (c *Context) Set(name, value string) error { func (c *Context) IsSet(name string) bool { if fs := lookupFlagSet(name, c); fs != nil { isSet := false - c.flagSet.Visit(func(f *flag.Flag) { + fs.Visit(func(f *flag.Flag) { if f.Name == name { isSet = true } @@ -146,25 +145,13 @@ func (c *Context) FlagNames() []string { return names } -// Parent returns the parent context, if any -func (c *Context) Parent() *Context { - return c.parentContext -} - // Lineage returns *this* context and all of its ancestor contexts in order from // child to parent func (c *Context) Lineage() []*Context { lineage := []*Context{} - cur := c - for { + for cur := c; cur != nil; cur = cur.parentContext { lineage = append(lineage, cur) - - if cur.parentContext == nil { - break - } - - cur = cur.parentContext } return lineage @@ -220,19 +207,6 @@ func (a Args) Swap(from, to int) error { return nil } -func globalContext(ctx *Context) *Context { - if ctx == nil { - return nil - } - - for { - if ctx.parentContext == nil { - return ctx - } - ctx = ctx.parentContext - } -} - func lookupFlagSet(name string, ctx *Context) *flag.FlagSet { for _, c := range ctx.Lineage() { if f := c.flagSet.Lookup(name); f != nil { diff --git a/context_test.go b/context_test.go index e819dbe..bdb77fd 100644 --- a/context_test.go +++ b/context_test.go @@ -84,18 +84,22 @@ func TestContext_NArg(t *testing.T) { func TestContext_IsSet(t *testing.T) { set := flag.NewFlagSet("test", 0) - set.Bool("myflag", false, "doc") - set.String("otherflag", "hello world", "doc") - globalSet := flag.NewFlagSet("test", 0) - globalSet.Bool("myflagGlobal", true, "doc") - globalCtx := NewContext(nil, globalSet, nil) - c := NewContext(nil, set, globalCtx) - set.Parse([]string{"--myflag", "bat", "baz"}) - globalSet.Parse([]string{"--myflagGlobal", "bat", "baz"}) - expect(t, c.IsSet("myflag"), true) - expect(t, c.IsSet("otherflag"), false) - expect(t, c.IsSet("bogusflag"), false) - expect(t, c.IsSet("myflagGlobal"), false) + set.Bool("one-flag", false, "doc") + set.Bool("two-flag", false, "doc") + set.String("three-flag", "hello world", "doc") + parentSet := flag.NewFlagSet("test", 0) + parentSet.Bool("top-flag", true, "doc") + parentCtx := NewContext(nil, parentSet, nil) + ctx := NewContext(nil, set, parentCtx) + + set.Parse([]string{"--one-flag", "--two-flag", "--three-flag", "frob"}) + parentSet.Parse([]string{"--top-flag"}) + + expect(t, ctx.IsSet("one-flag"), true) + expect(t, ctx.IsSet("two-flag"), true) + expect(t, ctx.IsSet("three-flag"), true) + expect(t, ctx.IsSet("top-flag"), true) + expect(t, ctx.IsSet("bogus"), false) } func TestContext_NumFlags(t *testing.T) { @@ -124,14 +128,14 @@ func TestContext_LocalFlagNames(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("one-flag", false, "doc") set.String("two-flag", "hello world", "doc") - globalSet := flag.NewFlagSet("test", 0) - globalSet.Bool("top-flag", true, "doc") - globalCtx := NewContext(nil, globalSet, nil) - c := NewContext(nil, set, globalCtx) + parentSet := flag.NewFlagSet("test", 0) + parentSet.Bool("top-flag", true, "doc") + parentCtx := NewContext(nil, parentSet, nil) + ctx := NewContext(nil, set, parentCtx) set.Parse([]string{"--one-flag", "--two-flag=foo"}) - globalSet.Parse([]string{"--top-flag"}) + parentSet.Parse([]string{"--top-flag"}) - actualFlags := c.LocalFlagNames() + actualFlags := ctx.LocalFlagNames() sort.Strings(actualFlags) expect(t, actualFlags, []string{"one-flag", "two-flag"}) @@ -141,15 +145,52 @@ func TestContext_FlagNames(t *testing.T) { set := flag.NewFlagSet("test", 0) set.Bool("one-flag", false, "doc") set.String("two-flag", "hello world", "doc") - globalSet := flag.NewFlagSet("test", 0) - globalSet.Bool("top-flag", true, "doc") - globalCtx := NewContext(nil, globalSet, nil) - c := NewContext(nil, set, globalCtx) + parentSet := flag.NewFlagSet("test", 0) + parentSet.Bool("top-flag", true, "doc") + parentCtx := NewContext(nil, parentSet, nil) + ctx := NewContext(nil, set, parentCtx) set.Parse([]string{"--one-flag", "--two-flag=foo"}) - globalSet.Parse([]string{"--top-flag"}) + parentSet.Parse([]string{"--top-flag"}) - actualFlags := c.FlagNames() + actualFlags := ctx.FlagNames() sort.Strings(actualFlags) expect(t, actualFlags, []string{"one-flag", "top-flag", "two-flag"}) } + +func TestContext_Lineage(t *testing.T) { + set := flag.NewFlagSet("test", 0) + set.Bool("local-flag", false, "doc") + parentSet := flag.NewFlagSet("test", 0) + parentSet.Bool("top-flag", true, "doc") + parentCtx := NewContext(nil, parentSet, nil) + ctx := NewContext(nil, set, parentCtx) + set.Parse([]string{"--local-flag"}) + parentSet.Parse([]string{"--top-flag"}) + + lineage := ctx.Lineage() + expect(t, len(lineage), 2) + expect(t, lineage[0], ctx) + expect(t, lineage[1], parentCtx) +} + +func TestContext_lookupFlagSet(t *testing.T) { + set := flag.NewFlagSet("test", 0) + set.Bool("local-flag", false, "doc") + parentSet := flag.NewFlagSet("test", 0) + parentSet.Bool("top-flag", true, "doc") + parentCtx := NewContext(nil, parentSet, nil) + ctx := NewContext(nil, set, parentCtx) + set.Parse([]string{"--local-flag"}) + parentSet.Parse([]string{"--top-flag"}) + + fs := lookupFlagSet("top-flag", ctx) + expect(t, fs, parentCtx.flagSet) + + fs = lookupFlagSet("local-flag", ctx) + expect(t, fs, ctx.flagSet) + + if fs := lookupFlagSet("frob", ctx); fs != nil { + t.Fail() + } +} From 7318e27528b3be6a024e85c16d1776342ac40d70 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 17 May 2016 03:46:16 -0400 Subject: [PATCH 3/4] Tidy up the flag name visitor func in Context --- context.go | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/context.go b/context.go index 002fe6e..03c61b2 100644 --- a/context.go +++ b/context.go @@ -137,11 +137,9 @@ func (c *Context) LocalFlagNames() []string { // its parent contexts. func (c *Context) FlagNames() []string { names := []string{} - for _, ctx := range c.Lineage() { ctx.flagSet.Visit(makeFlagNameVisitor(&names)) } - return names } @@ -362,8 +360,17 @@ func normalizeFlags(flags []Flag, set *flag.FlagSet) error { func makeFlagNameVisitor(names *[]string) func(*flag.Flag) { return func(f *flag.Flag) { - name := strings.Split(f.Name, ",")[0] - if name != "help" && name != "version" { + nameParts := strings.Split(f.Name, ",") + name := strings.TrimSpace(nameParts[0]) + + for _, part := range nameParts { + part = strings.TrimSpace(part) + if len(part) > len(name) { + name = part + } + } + + if name != "" { (*names) = append(*names, name) } } From a02aab6b4bd3faa000dd633de3862090fcc19e0e Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Tue, 17 May 2016 20:27:42 -0400 Subject: [PATCH 4/4] Add note about removal of `Context.Parent` --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3470ccd..1c94a17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ - deprecated `App.Author`, `App.Email`, and `Command.ShortName` fields - All `Context.Global*` methods, as the non-global versions now traverse up the context lineage automatically. +- `Context.Parent` method, as this is now available via `Context.Lineage` ### Fixed - `Context.BoolT` now returns `true` when not found