diff --git a/command.go b/command.go index fb0f7e055b..94d93eeecc 100644 --- a/command.go +++ b/command.go @@ -636,12 +636,6 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) { return err } - if err := cmd.checkPersistentRequiredFlags(); err != nil { - cmd.isInError = true - _ = ShowSubcommandHelp(cmd) - return err - } - if len(cmd.Arguments) > 0 { rargs := cmd.Args().Slice() tracef("calling argparse with %[1]v", rargs) @@ -768,8 +762,8 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { for _, fl := range pCmd.Flags { flNames := fl.Names() - pfl, ok := fl.(PersistentFlag) - if !ok || !pfl.IsPersistent() { + pfl, ok := fl.(LocalFlag) + if !ok || pfl.IsLocal() { tracef("skipping non-persistent flag %[1]q (cmd=%[2]q)", flNames, cmd.Name) continue } @@ -881,12 +875,12 @@ func (cmd *Command) appendFlag(fl Flag) { } } -// VisiblePersistentFlags returns a slice of [PersistentFlag] with Persistent=true and Hidden=false. +// VisiblePersistentFlags returns a slice of [LocalFlag] with Persistent=true and Hidden=false. func (cmd *Command) VisiblePersistentFlags() []Flag { var flags []Flag for _, fl := range cmd.Root().Flags { - pfl, ok := fl.(PersistentFlag) - if !ok || !pfl.IsPersistent() { + pfl, ok := fl.(LocalFlag) + if !ok || pfl.IsLocal() { continue } flags = append(flags, fl) @@ -994,48 +988,22 @@ func (cmd *Command) checkRequiredFlag(f Flag) (bool, string) { } func (cmd *Command) checkAllRequiredFlags() requiredFlagsErr { - if cmd.parent != nil { - if err := cmd.parent.checkRequiredFlags(); err != nil { + for pCmd := cmd; pCmd != nil; pCmd = pCmd.parent { + if err := pCmd.checkRequiredFlags(); err != nil { return err } } - return cmd.checkRequiredFlags() -} - -func (cmd *Command) checkRequiredFlags() requiredFlagsErr { - tracef("checking for required flags (cmd=%[1]q)", cmd.Name) - - missingFlags := []string{} - - for _, f := range cmd.Flags { - if pf, ok := f.(PersistentFlag); !ok || !pf.IsPersistent() { - if ok, name := cmd.checkRequiredFlag(f); !ok { - missingFlags = append(missingFlags, name) - } - } - } - - if len(missingFlags) != 0 { - tracef("found missing required flags %[1]q (cmd=%[2]q)", missingFlags, cmd.Name) - - return &errRequiredFlags{missingFlags: missingFlags} - } - - tracef("all required flags set (cmd=%[1]q)", cmd.Name) - return nil } -func (cmd *Command) checkPersistentRequiredFlags() requiredFlagsErr { +func (cmd *Command) checkRequiredFlags() requiredFlagsErr { tracef("checking for required flags (cmd=%[1]q)", cmd.Name) missingFlags := []string{} for _, f := range cmd.appliedFlags { - if pf, ok := f.(PersistentFlag); ok && pf.IsPersistent() { - if ok, name := cmd.checkRequiredFlag(f); !ok { - missingFlags = append(missingFlags, name) - } + if ok, name := cmd.checkRequiredFlag(f); !ok { + missingFlags = append(missingFlags, name) } } @@ -1233,7 +1201,7 @@ func (cmd *Command) runFlagActions(ctx context.Context) error { if !fl.IsSet() { continue } - if pf, ok := fl.(PersistentFlag); ok && pf.IsPersistent() { + if pf, ok := fl.(LocalFlag); ok && !pf.IsLocal() { continue } } diff --git a/command_test.go b/command_test.go index a2de06148d..780252c67c 100644 --- a/command_test.go +++ b/command_test.go @@ -2803,7 +2803,6 @@ func TestPersistentFlag(t *testing.T) { Flags: []Flag{ &StringFlag{ Name: "persistentCommandFlag", - Persistent: true, Destination: &appFlag, Action: func(context.Context, *Command, string) error { persistentFlagActionCount++ @@ -2812,22 +2811,18 @@ func TestPersistentFlag(t *testing.T) { }, &IntSliceFlag{ Name: "persistentCommandSliceFlag", - Persistent: true, Destination: &persistentCommandSliceInt, }, &FloatSliceFlag{ - Name: "persistentCommandFloatSliceFlag", - Persistent: true, - Value: []float64{11.3, 12.5}, + Name: "persistentCommandFloatSliceFlag", + Value: []float64{11.3, 12.5}, }, &IntFlag{ Name: "persistentCommandOverrideFlag", - Persistent: true, Destination: &appOverrideInt, }, &StringFlag{ Name: "persistentRequiredCommandFlag", - Persistent: true, Required: true, Destination: &appRequiredFlag, }, @@ -2839,16 +2834,17 @@ func TestPersistentFlag(t *testing.T) { &IntFlag{ Name: "cmdFlag", Destination: &topInt, + Local: true, }, &IntFlag{ Name: "cmdPersistentFlag", - Persistent: true, Destination: &topPersistentInt, }, &IntFlag{ Name: "paof", Aliases: []string{"persistentCommandOverrideFlag"}, Destination: &appOverrideCmdInt, + Local: true, }, }, Commands: []*Command{ @@ -2858,6 +2854,7 @@ func TestPersistentFlag(t *testing.T) { &IntFlag{ Name: "cmdFlag", Destination: &subCommandInt, + Local: true, }, }, Action: func(_ context.Context, cmd *Command) error { @@ -2914,8 +2911,7 @@ func TestPersistentFlagIsSet(t *testing.T) { Name: "root", Flags: []Flag{ &StringFlag{ - Name: "result", - Persistent: true, + Name: "result", }, }, Commands: []*Command{ @@ -3016,9 +3012,8 @@ func TestRequiredPersistentFlag(t *testing.T) { Name: "root", Flags: []Flag{ &StringFlag{ - Name: "result", - Persistent: true, - Required: true, + Name: "result", + Required: true, }, }, Commands: []*Command{ @@ -3418,10 +3413,10 @@ func TestCommand_IsSet_fromEnv(t *testing.T) { cmd := &Command{ Flags: []Flag{ - &FloatFlag{Name: "timeout", Aliases: []string{"t"}, Sources: EnvVars("APP_TIMEOUT_SECONDS")}, - &StringFlag{Name: "password", Aliases: []string{"p"}, Sources: EnvVars("APP_PASSWORD")}, - &FloatFlag{Name: "unparsable", Aliases: []string{"u"}, Sources: EnvVars("APP_UNPARSABLE")}, - &FloatFlag{Name: "no-env-var", Aliases: []string{"n"}}, + &FloatFlag{Name: "timeout", Aliases: []string{"t"}, Local: true, Sources: EnvVars("APP_TIMEOUT_SECONDS")}, + &StringFlag{Name: "password", Aliases: []string{"p"}, Local: true, Sources: EnvVars("APP_PASSWORD")}, + &FloatFlag{Name: "unparsable", Aliases: []string{"u"}, Local: true, Sources: EnvVars("APP_UNPARSABLE")}, + &FloatFlag{Name: "no-env-var", Aliases: []string{"n"}, Local: true}, }, Action: func(_ context.Context, cmd *Command) error { timeoutIsSet = cmd.IsSet("timeout") @@ -3772,18 +3767,15 @@ func TestCheckRequiredFlags(t *testing.T) { _ = os.Setenv(test.envVarInput[0], test.envVarInput[1]) } - set := flag.NewFlagSet("test", 0) - for _, flags := range test.flags { - _ = flags.Apply(set) - } - _ = set.Parse(test.parseInput) - cmd := &Command{ - Flags: test.flags, - flagSet: set, + Name: "foo", + Flags: test.flags, } + args := []string{"foo"} + args = append(args, test.parseInput...) + _ = cmd.Run(context.Background(), args) - err := cmd.checkRequiredFlags() + err := cmd.checkAllRequiredFlags() // assertions if test.expectedAnError { @@ -4041,7 +4033,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": "", "aliases": [ "sub-fl", @@ -4062,7 +4054,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": [ "s" @@ -4103,7 +4095,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": "", "aliases": [ "fl", @@ -4124,7 +4116,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": [ "b" @@ -4283,7 +4275,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": [ "s" @@ -4324,7 +4316,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": "", "aliases": [ "fl", @@ -4345,7 +4337,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": [ "b" @@ -4386,7 +4378,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": "value", "aliases": [ "s" @@ -4406,7 +4398,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": "", "aliases": [ "fl", @@ -4427,7 +4419,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": false, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": [ "b" @@ -4447,7 +4439,7 @@ func TestJSONExportCommand(t *testing.T) { "required": false, "hidden": true, "hideDefault": false, - "persistent": false, + "local": false, "defaultValue": false, "aliases": null, "takesFileArg": false, diff --git a/flag.go b/flag.go index 76a5d71669..420ea5e939 100644 --- a/flag.go +++ b/flag.go @@ -38,6 +38,7 @@ var VersionFlag Flag = &BoolFlag{ Aliases: []string{"v"}, Usage: "print the version", HideDefault: true, + Local: true, } // HelpFlag prints the help for all commands and subcommands. @@ -48,6 +49,7 @@ var HelpFlag Flag = &BoolFlag{ Aliases: []string{"h"}, Usage: "show help", HideDefault: true, + Local: true, } // FlagStringer converts a flag definition to a string. This is used by help @@ -172,10 +174,10 @@ type CategorizableFlag interface { SetCategory(string) } -// PersistentFlag is an interface to enable detection of flags which are persistent -// through subcommands -type PersistentFlag interface { - IsPersistent() bool +// LocalFlag is an interface to enable detection of flags which are local +// to current command +type LocalFlag interface { + IsLocal() bool } // IsDefaultVisible returns true if the flag is not hidden, otherwise false diff --git a/flag_bool_with_inverse.go b/flag_bool_with_inverse.go index 371064c5f7..00871d6143 100644 --- a/flag_bool_with_inverse.go +++ b/flag_bool_with_inverse.go @@ -90,7 +90,7 @@ func (parent *BoolWithInverseFlag) initialize() { Usage: child.Usage, Required: child.Required, Hidden: child.Hidden, - Persistent: child.Persistent, + Local: child.Local, Value: child.Value, Destination: parent.negDest, TakesFile: child.TakesFile, diff --git a/flag_bool_with_inverse_test.go b/flag_bool_with_inverse_test.go index 62266ff714..bdf64c90f2 100644 --- a/flag_bool_with_inverse_test.go +++ b/flag_bool_with_inverse_test.go @@ -199,6 +199,7 @@ func TestBoolWithInverseEnvVars(t *testing.T) { BoolFlag: &BoolFlag{ Name: "env", Sources: EnvVars("ENV"), + Local: true, }, } } diff --git a/flag_impl.go b/flag_impl.go index 610920d64b..77bc591860 100644 --- a/flag_impl.go +++ b/flag_impl.go @@ -77,7 +77,7 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct { Sources ValueSourceChain `json:"-"` // sources to load flag value from Required bool `json:"required"` // whether the flag is required or not Hidden bool `json:"hidden"` // whether to hide the flag in help output - Persistent bool `json:"persistent"` // whether the flag needs to be applied to subcommands as well + Local bool `json:"local"` // whether the flag needs to be applied to subcommands as well Value T `json:"defaultValue"` // default value for this flag if not set by from any source Destination *T `json:"-"` // destination pointer for value when set Aliases []string `json:"aliases"` // Aliases that are allowed for this flag @@ -107,13 +107,15 @@ 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 { + tracef("apply (flag=%[1]q)", f.Name) + // 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 { + if !f.applied || f.Local { newVal := f.Value if val, source, found := f.Sources.LookupWithSource(); found { @@ -278,7 +280,7 @@ func (f *FlagBase[T, C, VC]) IsMultiValueFlag() bool { return kind == reflect.Slice || kind == reflect.Map } -// IsPersistent returns true if flag needs to be persistent across subcommands -func (f *FlagBase[T, C, VC]) IsPersistent() bool { - return f.Persistent +// IsLocal returns false if flag needs to be persistent across subcommands +func (f *FlagBase[T, C, VC]) IsLocal() bool { + return f.Local } diff --git a/flag_test.go b/flag_test.go index ffc1fd2327..74b6d1d1bb 100644 --- a/flag_test.go +++ b/flag_test.go @@ -101,8 +101,8 @@ func TestBoolFlagCountFromCommand(t *testing.T) { }, } - bf := &BoolFlag{Name: "tf", Aliases: []string{"w", "huh"}} for _, bct := range boolCountTests { + bf := &BoolFlag{Name: "tf", Aliases: []string{"w", "huh"}} cmd := &Command{ Flags: []Flag{ bf, diff --git a/godoc-current.txt b/godoc-current.txt index eae97b42c7..2d120038ab 100644 --- a/godoc-current.txt +++ b/godoc-current.txt @@ -518,8 +518,8 @@ func (cmd *Command) VisibleFlags() []Flag VisibleFlags returns a slice of the Flags with Hidden=false func (cmd *Command) VisiblePersistentFlags() []Flag - VisiblePersistentFlags returns a slice of PersistentFlag with - Persistent=true and Hidden=false. + VisiblePersistentFlags returns a slice of LocalFlag with Persistent=true and + Hidden=false. type CommandCategories interface { // AddCommand adds a command to a category, creating a new category if necessary. @@ -633,6 +633,7 @@ var HelpFlag Flag = &BoolFlag{ Aliases: []string{"h"}, Usage: "show help", HideDefault: true, + Local: true, } HelpFlag prints the help for all commands and subcommands. Set to nil to disable the flag. The subcommand will still be added unless HideHelp or @@ -643,6 +644,7 @@ var VersionFlag Flag = &BoolFlag{ Aliases: []string{"v"}, Usage: "print the version", HideDefault: true, + Local: true, } VersionFlag prints the version for the application @@ -655,7 +657,7 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct { Sources ValueSourceChain `json:"-"` // sources to load flag value from Required bool `json:"required"` // whether the flag is required or not Hidden bool `json:"hidden"` // whether to hide the flag in help output - Persistent bool `json:"persistent"` // whether the flag needs to be applied to subcommands as well + Local bool `json:"local"` // whether the flag needs to be applied to subcommands as well Value T `json:"defaultValue"` // default value for this flag if not set by from any source Destination *T `json:"-"` // destination pointer for value when set Aliases []string `json:"aliases"` // Aliases that are allowed for this flag @@ -700,13 +702,13 @@ func (f *FlagBase[T, C, V]) GetValue() string func (f *FlagBase[T, C, V]) IsDefaultVisible() bool IsDefaultVisible returns true if the flag is not hidden, otherwise false +func (f *FlagBase[T, C, VC]) IsLocal() bool + IsLocal returns false if flag needs to be persistent across subcommands + func (f *FlagBase[T, C, VC]) IsMultiValueFlag() bool IsMultiValueFlag returns true if the value type T can take multiple values from cmd line. This is true for slice and map type flags -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 @@ -804,6 +806,12 @@ type InvalidFlagAccessFunc func(context.Context, *Command, string) InvalidFlagAccessFunc is executed when an invalid flag is accessed from the context. +type LocalFlag interface { + IsLocal() bool +} + LocalFlag is an interface to enable detection of flags which are local to + current command + type MapBase[T any, C any, VC ValueCreator[T, C]] struct { // Has unexported fields. } @@ -860,12 +868,6 @@ type OnUsageErrorFunc func(ctx context.Context, cmd *Command, err error, isSubco 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 { // whether the flag is a required flag or not IsRequired() bool diff --git a/help_test.go b/help_test.go index dd0228b0e2..b0fc94b33a 100644 --- a/help_test.go +++ b/help_test.go @@ -691,8 +691,7 @@ func TestShowSubcommandHelp_GlobalOptions(t *testing.T) { cmd := &Command{ Flags: []Flag{ &StringFlag{ - Name: "foo", - Persistent: true, + Name: "foo", }, }, Commands: []*Command{ @@ -700,7 +699,8 @@ func TestShowSubcommandHelp_GlobalOptions(t *testing.T) { Name: "frobbly", Flags: []Flag{ &StringFlag{ - Name: "bar", + Name: "bar", + Local: true, }, }, Action: func(context.Context, *Command) error { diff --git a/testdata/godoc-v3.x.txt b/testdata/godoc-v3.x.txt index 71e46b266e..2d120038ab 100644 --- a/testdata/godoc-v3.x.txt +++ b/testdata/godoc-v3.x.txt @@ -283,7 +283,7 @@ func (parent *BoolWithInverseFlag) RunAction(ctx context.Context, cmd *Command) func (parent *BoolWithInverseFlag) String() string String implements the standard Stringer interface. - Example for BoolFlag{Name: "env"} --[no-]env (default: false) + Example for BoolFlag{Name: "env"} --[no-]env (default: false) func (parent *BoolWithInverseFlag) Value() bool @@ -518,8 +518,8 @@ func (cmd *Command) VisibleFlags() []Flag VisibleFlags returns a slice of the Flags with Hidden=false func (cmd *Command) VisiblePersistentFlags() []Flag - VisiblePersistentFlags returns a slice of PersistentFlag with - Persistent=true and Hidden=false. + VisiblePersistentFlags returns a slice of LocalFlag with Persistent=true and + Hidden=false. type CommandCategories interface { // AddCommand adds a command to a category, creating a new category if necessary. @@ -633,6 +633,7 @@ var HelpFlag Flag = &BoolFlag{ Aliases: []string{"h"}, Usage: "show help", HideDefault: true, + Local: true, } HelpFlag prints the help for all commands and subcommands. Set to nil to disable the flag. The subcommand will still be added unless HideHelp or @@ -643,6 +644,7 @@ var VersionFlag Flag = &BoolFlag{ Aliases: []string{"v"}, Usage: "print the version", HideDefault: true, + Local: true, } VersionFlag prints the version for the application @@ -655,7 +657,7 @@ type FlagBase[T any, C any, VC ValueCreator[T, C]] struct { Sources ValueSourceChain `json:"-"` // sources to load flag value from Required bool `json:"required"` // whether the flag is required or not Hidden bool `json:"hidden"` // whether to hide the flag in help output - Persistent bool `json:"persistent"` // whether the flag needs to be applied to subcommands as well + Local bool `json:"local"` // whether the flag needs to be applied to subcommands as well Value T `json:"defaultValue"` // default value for this flag if not set by from any source Destination *T `json:"-"` // destination pointer for value when set Aliases []string `json:"aliases"` // Aliases that are allowed for this flag @@ -700,13 +702,13 @@ func (f *FlagBase[T, C, V]) GetValue() string func (f *FlagBase[T, C, V]) IsDefaultVisible() bool IsDefaultVisible returns true if the flag is not hidden, otherwise false +func (f *FlagBase[T, C, VC]) IsLocal() bool + IsLocal returns false if flag needs to be persistent across subcommands + func (f *FlagBase[T, C, VC]) IsMultiValueFlag() bool IsMultiValueFlag returns true if the value type T can take multiple values from cmd line. This is true for slice and map type flags -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 @@ -804,6 +806,12 @@ type InvalidFlagAccessFunc func(context.Context, *Command, string) InvalidFlagAccessFunc is executed when an invalid flag is accessed from the context. +type LocalFlag interface { + IsLocal() bool +} + LocalFlag is an interface to enable detection of flags which are local to + current command + type MapBase[T any, C any, VC ValueCreator[T, C]] struct { // Has unexported fields. } @@ -860,12 +868,6 @@ type OnUsageErrorFunc func(ctx context.Context, cmd *Command, err error, isSubco 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 { // whether the flag is a required flag or not IsRequired() bool