peterbourgon / ff

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

Support async flag definition #68

Closed moul closed 3 years ago

moul commented 4 years ago

I was thinking about various solutions to support my issue (#67), and one of them can be 100% managed thanks to ff, and I think it can also have other advantages for the ff users:

what do you think about supporting an alternative way to define flags, something like this maybe:

fooFs := flag.NewFlagSet()
fooFs.Var(...)
barFs := flag.NewFlagSet()
barFs.Var(...)
root := &ffcli.Command{
    Subcommands: []*ffcli.Command{
        {Name: "foo", FlagSet: fooFs, Exec: noopHandler},
        {Name: "bar", FlagSet: barFs, Exec: noopHandler},
    },
    Exec: noopHandler,
}

becomes

fooFsBuilder := func() (*flag.FlagSet, error) {
    fs := flag.NewFlagSet()
    fs.Var(...)
    return fs, nil
}
barFsBuilder := func() (*flag.FlagSet, error) {
    fs.NewFlagSet()
    fs.Var(...)
    return fs, nil
}
root := &ffcli.Command{
    Subcommands: []*ffcli.Command{
        {Name: "foo", FlagSetBuilder: fooFsBuilder, Exec: noopHandler},
        {Name: "bar", FlagSetBuilder: barFsBuilder, Exec: noopHandler},
    },
    Exec: noopHandler,
}

It can completely fix my issue and allow new workflows, i.e., if one of your flag default value requires some computing, i.e.

ipAddress, _ := retrieveInternetIPAddress() // something that can fail, take time, or even lock
fs.StringVar(&ipAddress, "addr", ipAddress, "")

Thanks to this, a program -h will always be fast, even if a subcommand requires some computing for initializing flags.

I understand that it has one major drawback -> having two different ways of doing the same thing, but I think that it's totally understandable for people discovering the package, for me it's like the current ParseAndBuild doing Parse, then Build

I can start making a PR, just tell me what you think before I start please :)

peterbourgon commented 3 years ago

if one of your flag default value requires some computing,

That seems like a design error to me.

The Command struct is already too large. If it will grow even more fields, they certainly shouldn't be almost-duplicates of existing fields for specific use cases. Sorry. I still want to solve your problem, but it's got to be in a different way.