Skip to content

Commit

Permalink
Merge pull request #1827 from dearchap/issue_1826
Browse files Browse the repository at this point in the history
Fix:(issue_1826) Make IsSet work with persistent flags
  • Loading branch information
dearchap authored Dec 4, 2023
2 parents 6e7906d + cafdd41 commit 281e398
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 6 deletions.
27 changes: 21 additions & 6 deletions command.go
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ func (cmd *Command) Run(ctx context.Context, osArgs []string) (deferErr error) {

tracef("running flag actions (cmd=%[1]q)", cmd.Name)

if err := runFlagActions(ctx, cmd, cmd.appliedFlags); err != nil {
if err := cmd.runFlagActions(ctx); err != nil {
return err
}

Expand Down Expand Up @@ -1014,19 +1014,34 @@ func hasCommand(commands []*Command, command *Command) bool {
return false
}

func runFlagActions(ctx context.Context, cmd *Command, flags []Flag) error {
for _, fl := range flags {
func (cmd *Command) runFlagActions(ctx context.Context) error {

for _, fl := range cmd.appliedFlags {
isSet := false

// check only local flagset for running local flag actions
for _, name := range fl.Names() {
if cmd.IsSet(name) {
isSet = true
cmd.flagSet.Visit(func(f *flag.Flag) {
if f.Name == name {
isSet = true
}
})
if isSet {
break
}
}

// If the flag hasnt been set on cmd line then we need to further
// check if it has been set via other means. If however it has
// been set by other means but it is persistent(and not set via current cmd)
// do not run the flag action
if !isSet {
continue
if !fl.IsSet() {
continue
}
if pf, ok := fl.(PersistentFlag); ok && pf.IsPersistent() {
continue
}
}

if af, ok := fl.(ActionableFlag); ok {
Expand Down
40 changes: 40 additions & 0 deletions command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2588,6 +2588,8 @@ func TestWhenExitSubCommandWithCodeThenCommandQuitUnexpectedly(t *testing.T) {
}

func buildMinimalTestCommand() *Command {
// reset the help flag because tests may have set it
HelpFlag.(*BoolFlag).hasBeenSet = false
return &Command{Writer: io.Discard}
}

Expand Down Expand Up @@ -3055,6 +3057,44 @@ func TestPersistentFlag(t *testing.T) {
}
}

func TestPersistentFlagIsSet(t *testing.T) {

result := ""
resultIsSet := false

app := &Command{
Name: "root",
Flags: []Flag{
&StringFlag{
Name: "result",
Persistent: true,
},
},
Commands: []*Command{
{
Name: "sub",
Action: func(_ context.Context, cmd *Command) error {
result = cmd.String("result")
resultIsSet = cmd.IsSet("result")
return nil
},
},
},
}

r := require.New(t)

err := app.Run(context.Background(), []string{"root", "--result", "before", "sub"})
r.NoError(err)
r.Equal("before", result)
r.True(resultIsSet)

err = app.Run(context.Background(), []string{"root", "sub", "--result", "after"})
r.NoError(err)
r.Equal("after", result)
r.True(resultIsSet)
}

func TestFlagDuplicates(t *testing.T) {
tests := []struct {
name string
Expand Down
1 change: 1 addition & 0 deletions flag_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func (f *FlagBase[T, C, V]) Apply(set *flag.FlagSet) error {
if err := f.value.Set(val); err != nil {
return err
}
f.hasBeenSet = true
if f.Validator != nil {
if v, ok := f.value.Get().(T); !ok {
return &typeError[T]{
Expand Down

0 comments on commit 281e398

Please sign in to comment.