ffcli: extend Command.Parse for custom validation

[marco-m]

Hello @peterbourgon,

Imagine a program that wants to use both Command.Parse and Command.Run, in order to distinguish the status code:

if err := rootCommand.Parse(os.Args[1:]); err != nil {
    fmt.Fprintln(os.Stderr, "xprog: error:", err)
    os.Exit(2)
}

if err := rootCommand.Run(context.Background()); err != nil {
    fmt.Fprintln(os.Stderr, "xprog: error:", err)
    os.Exit(1)
}

This doesn't really work, because almost all subcommands do additional custom validation of the CLI arguments, and so we end up exiting 1 also for CLI parse errors. The alternatives I can imagine are:

1. Abandon the idea and just call ParseAndRun and return 1.
2. Just call ParseAndRun and distinguish parse errors from other runtime errors via custom error types.

On the other hand, we could add another function field to ffcli.Command, let's call it Parse. If non nil, Parse of the root command would call it.

As an example, consider the repeat subcommand of textctl: https://github.com/peterbourgon/ff/blob/f5646899b17b9f61b7725a5191606484f56770f3/ffcli/examples/textctl/textctl.go#L30-L41

It could be split in two parts:

Parse: func(_ context.Context, args []string) error {
        if n := len(args); n != 1 {
            return fmt.Errorf("repeat requires exactly 1 argument, but you provided %d", n)
        }
    },
Exec: func(_ context.Context, args []string) error {
        if *verbose {
            fmt.Fprintf(os.Stderr, "repeat: will generate %dB of output\n", (*n)*len(args[0]))
        }
        for i := 0; i < *n; i++ {
            fmt.Fprintf(os.Stdout, "%s\n", args[0])
        }
        return nil
    },

If the function used by Parse was a method instead of an anonymous function (like the more complex example objectctl), it could also store state if needed.

[peterbourgon]

we could add another function field to ffcli.Command, let's call it Parse . . .

I don't think I want ffcli to understand user-supplied code in any more granularity than the current definition of the Exec func. Also, the Command struct is already far too large.

Your idea to

. . . distinguish parse errors from other runtime errors via custom error types

is absolutely the right way to go 👍

[marco-m]

Fair enough. Thanks.