peterbourgon / ff

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

[Feature Request] A way to specify required parameters else error #128

Open philwinder opened 5 months ago

philwinder commented 5 months ago

Hi Peter, Great work as always. I find myself having to duplicate boilerplate to check for empty parameters.

Can I propose that ff.NewFlagSet has some way of specifying that it should error if no parameter has been provided?

And ideally the error message should include a nice user friendly description?

Thanks, Phil

peterbourgon commented 5 months ago

Cheers! Not sure exactly what you mean by "if no parameter has been provided" — can you say a bit more?

Maybe you mean if name == ""? Alternatively, can you paste an example of the boilerplate that you're having to repeat?

peterbourgon commented 5 months ago

@philwinder Friendly ping :)

shidil commented 4 months ago

I think @philwinder is referring to marking certain flags as required, and ff.Parse handling the validation checks while printing help text explaining that flag --xx is required(but no value was provided).

Example https://cobra.dev/#required-flags

mfridman commented 3 months ago

Assuming this issue is asking for required flags, then +1.

It'd be neat if the Flag interface could expanded to include a setter like SetRequired (or maybe there's a better implementation), and then all the error states and help text would behave consistently.

akupila commented 3 months ago

+1 for having a way to specify required flags.

For now, i've used this to get similar behavior, but it would be nice if there was a more "native" way to do it:

func checkRequired(cmd *ff.Command, flag ...ff.Flag) {
    missing := make([]ff.Flag, 0, len(flag))
    for _, f := range flag {
        if !f.IsSet() {
            missing = append(missing, f)
        }
    }
    if len(missing) > 0 {
        fmt.Fprintln(os.Stderr, ffhelp.Command(cmd))
        fmt.Fprintln(os.Stderr, "Required flags were not provided:")
        for _, f := range missing {
            fmt.Fprintln(os.Stderr, ffhelp.FormatFlag(f, "  %+s"))
        }
        os.Exit(2)
    }
}
peterbourgon commented 3 months ago

Just so I understand: is the idea here that Parse should fail if a flag marked as required (however that might work) was not explicitly specified (as reported by IsSet)?

mfridman commented 3 months ago

I believe so. I wanted a bag of reusable but required flags (defined once) across multiple commands and so ended up with a global 😬 like this (in case you're interested to see how it's used in an OSS project).

https://github.com/pressly/goose/pull/741/files#diff-30f30550c03f971732cfeb7b92d7d64e64447acf969bfa5a55832429cf18cd01R50-R71

ps. Ideally, I could pass these down from the root command as a parent but I didn't want all parent flags to be inherited (but that's outside the scope of this issue). I'm still figuring out the best way to compose this, so don't mind the mess in that PR for now.

philwinder commented 3 months ago

Sorry for the delay, I missed the pings. Yes, you are both correct.

I would like some way of specifying that a particular flag is required and the program will error out if it is not provided.