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

Add App.InvalidFlagAccessHandler #1446

Merged
merged 3 commits into from Aug 30, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions app.go
Expand Up @@ -78,6 +78,8 @@ type App struct {
CommandNotFound CommandNotFoundFunc
// Execute this function if a usage error occurs
OnUsageError OnUsageErrorFunc
// Execute this function when an invalid flag is accessed from the context
InvalidFlagAccessHandler InvalidFlagAccessFunc
// Compilation date
Compiled time.Time
// List of all authors who contributed
Expand Down
15 changes: 14 additions & 1 deletion context.go
Expand Up @@ -46,6 +46,9 @@ func (cCtx *Context) NumFlags() int {

// Set sets a context flag to a value.
func (cCtx *Context) Set(name, value string) error {
if cCtx.flagSet.Lookup(name) == nil {
cCtx.onInvalidFlag(name)
}
return cCtx.flagSet.Set(name, value)
}

Expand Down Expand Up @@ -158,7 +161,7 @@ func (cCtx *Context) lookupFlagSet(name string) *flag.FlagSet {
return c.flagSet
}
}

cCtx.onInvalidFlag(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if you move this call into Context.Value() . Thats the direct calling place from user so its better to be closer to there. lookupFlagSet is an internal function and so we shouldnt expect the user to get notified is we lookup something internally

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work because the other getter functions don't call Context.Value internally. https://github.com/urfave/cli/blob/v2.11.2/flag_uint.go#L75

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dearchap any ideas on a better place to do the check?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me look at it.

return nil
}

Expand Down Expand Up @@ -192,6 +195,16 @@ func (cCtx *Context) checkRequiredFlags(flags []Flag) requiredFlagsErr {
return nil
}

func (cCtx *Context) onInvalidFlag(name string) {
for cCtx != nil {
if cCtx.App != nil && cCtx.App.InvalidFlagAccessHandler != nil {
cCtx.App.InvalidFlagAccessHandler(cCtx, name)
break
}
cCtx = cCtx.parentContext
}
}

func makeFlagNameVisitor(names *[]string) func(*flag.Flag) {
return func(f *flag.Flag) {
nameParts := strings.Split(f.Name, ",")
Expand Down
38 changes: 38 additions & 0 deletions context_test.go
Expand Up @@ -150,6 +150,31 @@ func TestContext_Value(t *testing.T) {
expect(t, c.Value("unknown-flag"), nil)
}

func TestContext_Value_InvalidFlagAccessHandler(t *testing.T) {
icholy marked this conversation as resolved.
Show resolved Hide resolved
var flagName string
app := &App{
InvalidFlagAccessHandler: func(_ *Context, name string) {
flagName = name
},
Commands: []*Command{
{
Name: "command",
Subcommands: []*Command{
{
Name: "subcommand",
Action: func(ctx *Context) error {
ctx.Value("missing")
return nil
},
},
},
},
},
}
expect(t, app.Run([]string{"run", "command", "subcommand"}), nil)
expect(t, flagName, "missing")
}

func TestContext_Args(t *testing.T) {
set := flag.NewFlagSet("test", 0)
set.Bool("myflag", false, "doc")
Expand Down Expand Up @@ -258,6 +283,19 @@ func TestContext_Set(t *testing.T) {
expect(t, c.IsSet("int"), true)
}

func TestContext_Set_InvalidFlagAccessHandler(t *testing.T) {
set := flag.NewFlagSet("test", 0)
var flagName string
app := &App{
InvalidFlagAccessHandler: func(_ *Context, name string) {
flagName = name
},
}
c := NewContext(app, set, nil)
c.Set("missing", "")
expect(t, flagName, "missing")
}

func TestContext_LocalFlagNames(t *testing.T) {
set := flag.NewFlagSet("test", 0)
set.Bool("one-flag", false, "doc")
Expand Down
3 changes: 3 additions & 0 deletions funcs.go
Expand Up @@ -23,6 +23,9 @@ type CommandNotFoundFunc func(*Context, string)
// is displayed and the execution is interrupted.
type OnUsageErrorFunc func(cCtx *Context, err error, isSubcommand bool) error

// InvalidFlagAccessFunc is executed when an invalid flag is accessed from the context.
type InvalidFlagAccessFunc func(*Context, string)

// ExitErrHandlerFunc is executed if provided in order to handle exitError values
// returned by Actions and Before/After functions.
type ExitErrHandlerFunc func(cCtx *Context, err error)
Expand Down