Merge pull request #1742 from dearchap/issue_1644

Fix:(issue_1644) Run persistent flag action

app.go

@@ -227,7 +227,7 @@ func (a *App) Setup() {
 	for _, fl := range a.Flags {
 		if cf, ok := fl.(CategorizableFlag); ok {
 			if cf.GetCategory() != "" {
-				a.flagCategories.AddFlag(cf.GetCategory(), cf)
+				a.flagCategories.AddFlag(cf.GetCategory(), fl)
 			}
 		}
 	}

app_test.go

@@ -3080,7 +3080,7 @@ func TestFlagAction(t *testing.T) {
 		{
 			name: "mixture",
 			args: []string{"app", "--f_string=app", "--f_uint=1", "--f_int_slice=1,2,3", "--f_duration=1h30m20s", "c1", "--f_string=c1", "sub1", "--f_string=sub1"},
-			exp:  "app 1h30m20s [1 2 3] 1 c1 sub1 ",
+			exp:  "app 1h30m20s [1 2 3] 1 c1 c1 c1 sub1 sub1 ",
 		},
 		{
 			name: "flag_string_map",
@@ -3116,13 +3116,18 @@ func TestPersistentFlag(t *testing.T) {
 	var appOverrideCmdInt int64
 	var appSliceFloat64 []float64
 	var persistentAppSliceInt []int64
+	var persistentFlagActionCount int64
 
 	a := &App{
 		Flags: []Flag{
 			&StringFlag{
 				Name:        "persistentAppFlag",
 				Persistent:  true,
 				Destination: &appFlag,
+				Action: func(ctx *Context, s string) error {
+					persistentFlagActionCount++
+					return nil
+				},
 			},
 			&Int64SliceFlag{
 				Name:        "persistentAppSliceFlag",
@@ -3235,6 +3240,9 @@ func TestPersistentFlag(t *testing.T) {
 		t.Errorf("Expected %f got %f", expectedFloat, appSliceFloat64)
 	}
 
+	if persistentFlagActionCount != 2 {
+		t.Errorf("Expected persistent flag action to be called 2 times instead called %d", persistentFlagActionCount)
+	}
 }
 
 func TestFlagDuplicates(t *testing.T) {

category.go

@@ -103,7 +103,7 @@ func newFlagCategoriesFromFlags(fs []Flag) FlagCategories {
 	for _, fl := range fs {
 		if cf, ok := fl.(CategorizableFlag); ok {
 			if cf.GetCategory() != "" {
-				fc.AddFlag(cf.GetCategory(), cf)
+				fc.AddFlag(cf.GetCategory(), fl)
 			}
 		}
 	}
@@ -140,7 +140,7 @@ type VisibleFlagCategory interface {
 	// Name returns the category name string
 	Name() string
 	// Flags returns a slice of VisibleFlag sorted by name
-	Flags() []VisibleFlag
+	Flags() []Flag
 }
 
 type defaultVisibleFlagCategory struct {
@@ -152,7 +152,7 @@ func (fc *defaultVisibleFlagCategory) Name() string {
 	return fc.name
 }
 
-func (fc *defaultVisibleFlagCategory) Flags() []VisibleFlag {
+func (fc *defaultVisibleFlagCategory) Flags() []Flag {
 	vfNames := []string{}
 	for flName, fl := range fc.m {
 		if vf, ok := fl.(VisibleFlag); ok {
@@ -164,9 +164,9 @@ func (fc *defaultVisibleFlagCategory) Flags() []VisibleFlag {
 
 	sort.Strings(vfNames)
 
-	ret := make([]VisibleFlag, len(vfNames))
+	ret := make([]Flag, len(vfNames))
 	for i, flName := range vfNames {
-		ret[i] = fc.m[flName].(VisibleFlag)
+		ret[i] = fc.m[flName]
 	}
 
 	return ret

command.go

@@ -75,6 +75,9 @@ type Command struct {
 
 	// Flag exclusion group
 	MutuallyExclusiveFlags []MutuallyExclusiveFlags
+
+	// flags that have been applied in current parse
+	appliedFlags []Flag
 }
 
 type Commands []*Command
@@ -231,7 +234,7 @@ func (c *Command) Run(cCtx *Context, arguments ...string) (err error) {
 		}
 	}
 
-	if err = runFlagActions(cCtx, c.Flags); err != nil {
+	if err = runFlagActions(cCtx, c.appliedFlags); err != nil {
 		return err
 	}
 
@@ -293,6 +296,7 @@ func (c *Command) Run(cCtx *Context, arguments ...string) (err error) {
 }
 
 func (c *Command) newFlagSet() (*flag.FlagSet, error) {
+	c.appliedFlags = append(c.appliedFlags, c.allFlags()...)
 	return flagSet(c.Name, c.allFlags())
 }
 
@@ -374,6 +378,8 @@ func (c *Command) parseFlags(args Args, ctx *Context) (*flag.FlagSet, error) {
 			if err := fl.Apply(set); err != nil {
 				return nil, err
 			}
+
+			c.appliedFlags = append(c.appliedFlags, fl)
 		}
 	}
 

docs.go

@@ -204,7 +204,7 @@ func prepareFlags(
 		}
 		modifiedArg := opener
 
-		for _, s := range flag.Names() {
+		for _, s := range f.Names() {
 			trimmed := strings.TrimSpace(s)
 			if len(modifiedArg) > len(opener) {
 				modifiedArg += sep
@@ -368,7 +368,7 @@ func (tt tabularTemplate) PrepareFlags(flags []Flag) []cliTabularFlagTemplate {
 			f.Default = strconv.FormatBool(boolFlag.Value)
 		}
 
-		for i, name := range flag.Names() {
+		for i, name := range appFlag.Names() {
 			name = strings.TrimSpace(name)
 
 			if i == 0 {

flag.go

@@ -92,7 +92,6 @@ func (f FlagsByName) Swap(i, j int) {
 
 // ActionableFlag is an interface that wraps Flag interface and RunAction operation.
 type ActionableFlag interface {
-	Flag
 	RunAction(*Context) error
 }
 
@@ -115,16 +114,12 @@ type Flag interface {
 // RequiredFlag is an interface that allows us to mark flags as required
 // it allows flags required flags to be backwards compatible with the Flag interface
 type RequiredFlag interface {
-	Flag
-
 	// whether the flag is a required flag or not
 	IsRequired() bool
 }
 
 // DocGenerationFlag is an interface that allows documentation generation for the flag
 type DocGenerationFlag interface {
-	Flag
-
 	// TakesValue returns true if the flag takes a value, otherwise false
 	TakesValue() bool
 
@@ -158,17 +153,13 @@ type Countable interface {
 
 // VisibleFlag is an interface that allows to check if a flag is visible
 type VisibleFlag interface {
-	Flag
-
 	// IsVisible returns true if the flag is not hidden, otherwise false
 	IsVisible() bool
 }
 
 // CategorizableFlag is an interface that allows us to potentially
 // use a flag in a categorized representation.
 type CategorizableFlag interface {
-	VisibleFlag
-
 	// Returns the category of the flag
 	GetCategory() string
 }
@@ -357,7 +348,7 @@ func stringifyFlag(f Flag) string {
 
 	usageWithDefault := strings.TrimSpace(usage + defaultValueString)
 
-	pn := prefixedNames(df.Names(), placeholder)
+	pn := prefixedNames(f.Names(), placeholder)
 	sliceFlag, ok := f.(DocGenerationMultiValueFlag)
 	if ok && sliceFlag.IsMultiValueFlag() {
 		pn = pn + " [ " + pn + " ]"

flag_test.go

@@ -197,8 +197,8 @@ func TestFlagsFromEnv(t *testing.T) {
 				if !reflect.DeepEqual(ctx.Value(test.flag.Names()[0]), test.output) {
 					t.Errorf("ex:%01d expected %q to be parsed as %#v, instead was %#v", i, test.input, test.output, ctx.Value(test.flag.Names()[0]))
 				}
-				if !f.IsSet() {
-					t.Errorf("Flag %s not set", f.Names()[0])
+				if !test.flag.IsSet() {
+					t.Errorf("Flag %s not set", test.flag.Names()[0])
 				}
 
 				// check that flag names are returned when set via env as well

godoc-current.txt

@@ -307,7 +307,6 @@ type ActionFunc func(*Context) error
     ActionFunc is the action to execute when no subcommands are specified
 
 type ActionableFlag interface {
-	Flag
 	RunAction(*Context) error
 }
     ActionableFlag is an interface that wraps Flag interface and RunAction
@@ -528,8 +527,6 @@ func (s *BoolWithInverseFlag) String() string
 func (s *BoolWithInverseFlag) Value() bool
 
 type CategorizableFlag interface {
-	VisibleFlag
-
 	// Returns the category of the flag
 	GetCategory() string
 }
@@ -595,6 +592,7 @@ type Command struct {
 
 	// Flag exclusion group
 	MutuallyExclusiveFlags []MutuallyExclusiveFlags
+
 	// Has unexported fields.
 }
     Command is a subcommand for a cli.App.
@@ -762,8 +760,6 @@ type Countable interface {
     repetitive flags
 
 type DocGenerationFlag interface {
-	Flag
-
 	// TakesValue returns true if the flag takes a value, otherwise false
 	TakesValue() bool
 
@@ -1101,8 +1097,6 @@ type PersistentFlag interface {
     persistent through subcommands
 
 type RequiredFlag interface {
-	Flag
-
 	// whether the flag is a required flag or not
 	IsRequired() bool
 }
@@ -1220,8 +1214,6 @@ func FilePaths(paths ...string) ValueSources
 func (v ValueSources) Get() (string, string, bool)
 
 type VisibleFlag interface {
-	Flag
-
 	// IsVisible returns true if the flag is not hidden, otherwise false
 	IsVisible() bool
 }
@@ -1231,7 +1223,7 @@ type VisibleFlagCategory interface {
 	// Name returns the category name string
 	Name() string
 	// Flags returns a slice of VisibleFlag sorted by name
-	Flags() []VisibleFlag
+	Flags() []Flag
 }
     VisibleFlagCategory is a category containing flags.
 

testdata/godoc-v3.x.txt

@@ -307,7 +307,6 @@ type ActionFunc func(*Context) error
     ActionFunc is the action to execute when no subcommands are specified
 
 type ActionableFlag interface {
-	Flag
 	RunAction(*Context) error
 }
     ActionableFlag is an interface that wraps Flag interface and RunAction
@@ -528,8 +527,6 @@ func (s *BoolWithInverseFlag) String() string
 func (s *BoolWithInverseFlag) Value() bool
 
 type CategorizableFlag interface {
-	VisibleFlag
-
 	// Returns the category of the flag
 	GetCategory() string
 }
@@ -595,6 +592,7 @@ type Command struct {
 
 	// Flag exclusion group
 	MutuallyExclusiveFlags []MutuallyExclusiveFlags
+
 	// Has unexported fields.
 }
     Command is a subcommand for a cli.App.
@@ -762,8 +760,6 @@ type Countable interface {
     repetitive flags
 
 type DocGenerationFlag interface {
-	Flag
-
 	// TakesValue returns true if the flag takes a value, otherwise false
 	TakesValue() bool
 
@@ -1101,8 +1097,6 @@ type PersistentFlag interface {
     persistent through subcommands
 
 type RequiredFlag interface {
-	Flag
-
 	// whether the flag is a required flag or not
 	IsRequired() bool
 }
@@ -1220,8 +1214,6 @@ func FilePaths(paths ...string) ValueSources
 func (v ValueSources) Get() (string, string, bool)
 
 type VisibleFlag interface {
-	Flag
-
 	// IsVisible returns true if the flag is not hidden, otherwise false
 	IsVisible() bool
 }
@@ -1231,7 +1223,7 @@ type VisibleFlagCategory interface {
 	// Name returns the category name string
 	Name() string
 	// Flags returns a slice of VisibleFlag sorted by name
-	Flags() []VisibleFlag
+	Flags() []Flag
 }
     VisibleFlagCategory is a category containing flags.