Add App.EnableStrictLookup option

[icholy]

What type of PR is this?

• feature

What this PR does / why we need it:

The current behaviour returns a zero value when accessing an undefined flag. This hides programmer errors and has resulted in several bugs on my end.

Which issue(s) this PR fixes:

https://github.com/urfave/cli/issues/1408

Testing

I added tests which make sure Context.Set and Context.Value panic when accessing an undefined flag.

Release Notes

A new `App.EnableStrictLookup` option which causes undefined flag lookups to panic

[icholy]

Where would I return this error? The flag value getters don't return errors.

edit: Oh you mean for cli.Context.Set. That makes sense.

I'm thinking:

type FlagNotFoundError struct {
	Name string
}

func (e FlagNotFoundError) Error() string {
	return fmt.Sprintf("flag not found: %q", e.Name)
}

• Context.Set would return this error.
• Context.Get (and friends) will panic with this error.

[meatballhat]

@icholy yes! Sorry for not being clear about that distinction. What you propose LGTM 🤘🏼

[julian7]

I'm also wondering whether handling this event via a hook would be more powerful.

Like, instead of having App. EnableStrictLookup boolean, we could have an App.UnknownFlagHandler or something like that. Then, the implementer can decide whether the app should throw panic(), whether it should log it, or perhaps even change context on such an event.

[icholy]

@julian7 that's an interesting idea. Something like this?

type App struct {
    // ...
    UnknownFlagHandler func(ctx *Context, name string)
}

[meatballhat]

I'm also wondering whether handling this event via a hook would be more powerful.

Oooh I like that idea a lot @julian7