Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Add support for persistent flags #1568

Merged
merged 2 commits into from
Nov 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {
dearchap marked this conversation as resolved.
Show resolved Hide resolved
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
Loading