peterbourgon / ff

Flags-first package for configuration
Apache License 2.0
1.34k stars 59 forks source link

v4.0.0-alpha.1 #113

Closed peterbourgon closed 10 months ago

peterbourgon commented 12 months ago

This PR introduces v4 of package ff, which includes several enhancements and breaking changes.

What are the major changes?

Why move away from flag.FlagSet?

Basically, it was too limited.

While I'm not necessarily sold on all of getopts(3), I think allowing flags to have long (--foo) and short (-f) names is an important feature, and a basic expectation for nearly all users. The fact that the stdlib flag set doesn't support this feature proved to be a problem worth fixing.

Also, using a concrete type kind of paints the module into a corner, in terms of capability and extensibility. I don't think it would have been possible to provide the most-requested features (hierarchical flags, support for different flag set implementations, etc.) without switching to some kind of abstraction.

Why adapt a flag.FlagSet to a bespoke CoreFlags instead of using a separate type?

To take advantage of CoreFlags features like Reset and SetParent, without needing to re-implement them. (I'm not totally convinced this is the right approach, but it's where I've landed for the moment.)

Why model hierarchy as parent flag sets instead of multiple flag sets in a command?

You always have a single set of args provided by the user. If you want to apply those args to multiple flag sets, then you need to find a way for the Parse method of each flag set to consume only the args that map to flags known to the flag set, and yield the args that map to flags that aren't known to the flag set, while also returning non-flag args separately. The command parser would then need to call Parse for each flag set in sequence, passing down the leftover args after each stage.

This would require changes to the Parse method in the Flags interface, which is currently extremely good and intuitive: Parse(args []string) error. Complicating it would be a very tall order, and a large burden for implementations. And after a lot of experimentation, I concluded that it was infeasible to reliably distinguish "a flag that we don't know about" from "a non-flag argument" when parsing args.

Consequently, the design keeps the simpler Parse signature in the Flags interface, and moves all of the logic related to parent flags into the implementation(s). The GetFlags method in the Flag interface was added in part so that callers can know in which flag set a flag was defined.

mfridman commented 10 months ago

More of a drive-by. Are you looking for feedback on this /v4?

I've used the /v3 fairly extensively in a few projects, so looking forward to whatever iteration comes next.

peterbourgon commented 10 months ago

Feedback is very much appreciated!

peterbourgon commented 10 months ago

I'm going to go ahead and release this as v4.0.0-alpha.1 so it can be imported. Feedback still appreciated.