flag.Func error not raised if trigged via config file

[youngpm]

I've noticed differing behavior if I specify a flag via the command line vs if that flag comes from a config file.

Here is an example:

package main

import (
    "errors"
    "flag"
    "os"

    "github.com/peterbourgon/ff"
)

func main() {
    fs := flag.NewFlagSet("my-program", flag.ExitOnError)
    var (
        _ = fs.String("config", "", "config file (optional)")
    )

    fs.Func("aflag", "a flag that raises a error",
        func(flagValue string) error {
            return errors.New("bad flag")
        },
    )

    ff.Parse(fs, os.Args[1:],
        ff.WithEnvVarPrefix("MY_PROGRAM"),
        ff.WithConfigFileFlag("config"),
        ff.WithConfigFileParser(ff.PlainParser),
    )
}

If I run it by passing the flag explicitly, I get

./my-program -aflag bad
invalid value "bad" for flag -aflag: bad flag
Usage of my-program:
-aflag value
a flag that raises a error
-config string
config file (optional)

but running ./my-program -config config.txt where config.txt is aflag bad executes without error. I know fs.Func runs and the error is returned in both cases, but I am not sure why it doesn't trigger an an exit in the latter case.

Thanks!

[peterbourgon]

Huh! Cute!

From the docs...

ErrorHandling defines how FlagSet.Parse behaves if the parse fails.

Emphasis mine — it only affects Parse, and not e.g. Set. Of course ff.Parse does call fs.Parse, but right at the beginning of its run, and just with the args you provided. That's the only invocation — all other interactions with the FlagSet happen via other methods.

I'm not sure what would be the best solution here. I agree it is somewhat surprising that the behavior is different. But it is subtle. If we wanted to fix it, we'd have to invoke FlagSet.Parse multiple times in ff.Parse, and I'm not sure that's the play. I wonder if a docs clarification would be enough?

Another thing making me reluctant to address this is that you should really only ever use ContinueOnError, and always check the error returned by Parse. Only func main has the right to terminate the program.

[youngpm]

Yeah this makes sense; I'll PR a note to the docs.

Ashamedly I wasn't even aware of ContinueOnError, too much copypasta! That's a better way.