Skip to content

Commit

Permalink
Merge pull request #1568 from dearchap/persistent_flags
Browse files Browse the repository at this point in the history
  • Loading branch information
dearchap committed Nov 27, 2022
2 parents aef9126 + e0d5817 commit 04736d1
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 32 deletions.
132 changes: 132 additions & 0 deletions app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,10 @@ func TestApp_AfterFunc(t *testing.T) {
reset
*/
counts = &opCounts{}
// reset the flags since they are set previously
app.Flags = []Flag{
&StringFlag{Name: "opt"},
}

// run with none args
err = app.Run([]string{"command"})
Expand Down Expand Up @@ -3053,3 +3057,131 @@ func TestFlagAction(t *testing.T) {
})
}
}

func TestPersistentFlag(t *testing.T) {

var topInt, topPersistentInt, subCommandInt, appOverrideInt int
var appFlag string
var appOverrideCmdInt int64
var appSliceFloat64 []float64
var persistentAppSliceInt []int64

a := &App{
Flags: []Flag{
&StringFlag{
Name: "persistentAppFlag",
Persistent: true,
Destination: &appFlag,
},
&Int64SliceFlag{
Name: "persistentAppSliceFlag",
Persistent: true,
Destination: &persistentAppSliceInt,
},
&Float64SliceFlag{
Name: "persistentAppFloatSliceFlag",
Persistent: true,
Value: []float64{11.3, 12.5},
},
&IntFlag{
Name: "persistentAppOverrideFlag",
Persistent: true,
Destination: &appOverrideInt,
},
},
Commands: []*Command{
{
Name: "cmd",
Flags: []Flag{
&IntFlag{
Name: "cmdFlag",
Destination: &topInt,
},
&IntFlag{
Name: "cmdPersistentFlag",
Persistent: true,
Destination: &topPersistentInt,
},
&Int64Flag{
Name: "paof",
Aliases: []string{"persistentAppOverrideFlag"},
Destination: &appOverrideCmdInt,
},
},
Subcommands: []*Command{
{
Name: "subcmd",
Flags: []Flag{
&IntFlag{
Name: "cmdFlag",
Destination: &subCommandInt,
},
},
Action: func(ctx *Context) error {
appSliceFloat64 = ctx.Float64Slice("persistentAppFloatSliceFlag")
return nil
},
},
},
},
},
}

err := a.Run([]string{"app",
"--persistentAppFlag", "hello",
"--persistentAppSliceFlag", "100",
"--persistentAppOverrideFlag", "102",
"cmd",
"--cmdFlag", "12",
"--persistentAppSliceFlag", "102",
"--persistentAppFloatSliceFlag", "102.455",
"--paof", "105",
"subcmd",
"--cmdPersistentFlag", "20",
"--cmdFlag", "11",
"--persistentAppFlag", "bar",
"--persistentAppSliceFlag", "130",
"--persistentAppFloatSliceFlag", "3.1445",
})

if err != nil {
t.Fatal(err)
}

if appFlag != "bar" {
t.Errorf("Expected 'bar' got %s", appFlag)
}

if topInt != 12 {
t.Errorf("Expected 12 got %d", topInt)
}

if topPersistentInt != 20 {
t.Errorf("Expected 20 got %d", topPersistentInt)
}

// this should be changed from app since
// cmd overrides it
if appOverrideInt != 102 {
t.Errorf("Expected 102 got %d", appOverrideInt)
}

if subCommandInt != 11 {
t.Errorf("Expected 11 got %d", subCommandInt)
}

if appOverrideCmdInt != 105 {
t.Errorf("Expected 105 got %d", appOverrideCmdInt)
}

expectedInt := []int64{100, 102, 130}
if !reflect.DeepEqual(persistentAppSliceInt, expectedInt) {
t.Errorf("Expected %v got %d", expectedInt, persistentAppSliceInt)
}

expectedFloat := []float64{102.455, 3.1445}
if !reflect.DeepEqual(appSliceFloat64, expectedFloat) {
t.Errorf("Expected %f got %f", expectedFloat, appSliceFloat64)
}

}
41 changes: 35 additions & 6 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ func (c *Command) Run(cCtx *Context, arguments ...string) (err error) {
}

a := args(arguments)
set, err := c.parseFlags(&a, cCtx.shellComplete)
set, err := c.parseFlags(&a, cCtx)
cCtx.flagSet = set

if c.isRoot {
Expand Down Expand Up @@ -308,7 +308,7 @@ func (c *Command) suggestFlagFromError(err error, command string) (string, error
return fmt.Sprintf(SuggestDidYouMeanTemplate, suggestion) + "\n\n", nil
}

func (c *Command) parseFlags(args Args, shellComplete bool) (*flag.FlagSet, error) {
func (c *Command) parseFlags(args Args, ctx *Context) (*flag.FlagSet, error) {
set, err := c.newFlagSet()
if err != nil {
return nil, err
Expand All @@ -318,13 +318,42 @@ func (c *Command) parseFlags(args Args, shellComplete bool) (*flag.FlagSet, erro
return set, set.Parse(append([]string{"--"}, args.Tail()...))
}

err = parseIter(set, c, args.Tail(), shellComplete)
if err != nil {
for pCtx := ctx.parentContext; pCtx != nil; pCtx = pCtx.parentContext {
if pCtx.Command == nil {
continue
}

for _, fl := range pCtx.Command.Flags {
pfl, ok := fl.(PersistentFlag)
if !ok || !pfl.IsPersistent() {
continue
}

applyPersistentFlag := true
set.VisitAll(func(f *flag.Flag) {
for _, name := range fl.Names() {
if name == f.Name {
applyPersistentFlag = false
break
}
}
})

if !applyPersistentFlag {
continue
}

if err := fl.Apply(set); err != nil {
return nil, err
}
}
}

if err := parseIter(set, c, args.Tail(), ctx.shellComplete); err != nil {
return nil, err
}

err = normalizeFlags(c.Flags, set)
if err != nil {
if err := normalizeFlags(c.Flags, set); err != nil {
return nil, err
}

Expand Down
6 changes: 6 additions & 0 deletions flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,12 @@ type CategorizableFlag interface {
GetCategory() string
}

// PersistentFlag is an interface to enable detection of flags which are persistent
// through subcommands
type PersistentFlag interface {
IsPersistent() bool
}

func flagSet(name string, flags []Flag) (*flag.FlagSet, error) {
set := flag.NewFlagSet(name, flag.ContinueOnError)

Expand Down
61 changes: 39 additions & 22 deletions flag_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct {
FilePath string // file path to load value from
Usage string // usage string for help output

Required bool // whether the flag is required or not
Hidden bool // whether to hide the flag in help output
Required bool // whether the flag is required or not
Hidden bool // whether to hide the flag in help output
Persistent bool // whether the flag needs to be applied to subcommands as well

Value T // default value for this flag if not set by from any source
Destination *T // destination pointer for value when set
Expand All @@ -58,6 +59,7 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct {

// unexported fields for internal use
hasBeenSet bool // whether the flag has been set from env or file
applied bool // whether the flag has been applied to a flag set already
creator VC // value creator for this flag type
value Value // value representing this flag's value
}
Expand All @@ -73,35 +75,44 @@ func (f *FlagBase[T, C, V]) GetValue() string {

// Apply populates the flag given the flag set and environment
func (f *FlagBase[T, C, V]) Apply(set *flag.FlagSet) error {
newVal := f.Value

if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
tmpVal := f.creator.Create(f.Value, new(T), f.Config)
if val != "" || reflect.TypeOf(f.Value).Kind() == reflect.String {
if err := tmpVal.Set(val); err != nil {
return fmt.Errorf("could not parse %q as %T value from %s for flag %s: %s", val, f.Value, source, f.Name, err)
}
} else if val == "" && reflect.TypeOf(f.Value).Kind() == reflect.Bool {
val = "false"
if err := tmpVal.Set(val); err != nil {
return fmt.Errorf("could not parse %q as %T value from %s for flag %s: %s", val, f.Value, source, f.Name, err)
// TODO move this phase into a separate flag initialization function
// if flag has been applied previously then it would have already been set
// from env or file. So no need to apply the env set again. However
// lots of units tests prior to persistent flags assumed that the
// flag can be applied to different flag sets multiple times while still
// keeping the env set.
if !f.applied || !f.Persistent {
newVal := f.Value

if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {
tmpVal := f.creator.Create(f.Value, new(T), f.Config)
if val != "" || reflect.TypeOf(f.Value).Kind() == reflect.String {
if err := tmpVal.Set(val); err != nil {
return fmt.Errorf("could not parse %q as %T value from %s for flag %s: %s", val, f.Value, source, f.Name, err)
}
} else if val == "" && reflect.TypeOf(f.Value).Kind() == reflect.Bool {
val = "false"
if err := tmpVal.Set(val); err != nil {
return fmt.Errorf("could not parse %q as %T value from %s for flag %s: %s", val, f.Value, source, f.Name, err)
}
}
}

newVal = tmpVal.Get().(T)
f.hasBeenSet = true
}
newVal = tmpVal.Get().(T)
f.hasBeenSet = true
}

if f.Destination == nil {
f.value = f.creator.Create(newVal, new(T), f.Config)
} else {
f.value = f.creator.Create(newVal, f.Destination, f.Config)
if f.Destination == nil {
f.value = f.creator.Create(newVal, new(T), f.Config)
} else {
f.value = f.creator.Create(newVal, f.Destination, f.Config)
}
}

for _, name := range f.Names() {
set.Var(f.value, name, f.Usage)
}

f.applied = true
return nil
}

Expand Down Expand Up @@ -178,7 +189,13 @@ func (f *FlagBase[T, C, V]) RunAction(ctx *Context) error {
return nil
}

// IsSliceFlag returns true if the value type T is of kind slice
func (f *FlagBase[T, C, VC]) IsSliceFlag() bool {
// TBD how to specify
return reflect.TypeOf(f.Value).Kind() == reflect.Slice
}

// IsPersistent returns true if flag needs to be persistent across subcommands
func (f *FlagBase[T, C, VC]) IsPersistent() bool {
return f.Persistent
}
15 changes: 13 additions & 2 deletions godoc-current.txt
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,9 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct {
FilePath string // file path to load value from
Usage string // usage string for help output

Required bool // whether the flag is required or not
Hidden bool // whether to hide the flag in help output
Required bool // whether the flag is required or not
Hidden bool // whether to hide the flag in help output
Persistent bool // whether the flag needs to be applied to subcommands as well

Value T // default value for this flag if not set by from any source
Destination *T // destination pointer for value when set
Expand Down Expand Up @@ -826,13 +827,17 @@ func (f *FlagBase[T, C, V]) GetValue() string
GetValue returns the flags value as string representation and an empty
string if the flag takes no value at all.

func (f *FlagBase[T, C, VC]) IsPersistent() bool
IsPersistent returns true if flag needs to be persistent across subcommands

func (f *FlagBase[T, C, V]) IsRequired() bool
IsRequired returns whether or not the flag is required

func (f *FlagBase[T, C, V]) IsSet() bool
IsSet returns whether or not the flag has been set through env or file

func (f *FlagBase[T, C, VC]) IsSliceFlag() bool
IsSliceFlag returns true if the value type T is of kind slice

func (f *FlagBase[T, C, V]) IsVisible() bool
IsVisible returns true if the flag is not hidden, otherwise false
Expand Down Expand Up @@ -940,6 +945,12 @@ type OnUsageErrorFunc func(cCtx *Context, err error, isSubcommand bool) error
the original error messages. If this function is not set, the "Incorrect
usage" is displayed and the execution is interrupted.

type PersistentFlag interface {
IsPersistent() bool
}
PersistentFlag is an interface to enable detection of flags which are
persistent through subcommands

type RequiredFlag interface {
Flag

Expand Down

0 comments on commit 04736d1

Please sign in to comment.