mow.cli icon indicating copy to clipboard operation
mow.cli copied to clipboard

Request: parse expected flags with flag.ContinueOnError

Open mark-kubacki opened this issue 9 years ago • 4 comments

I'd like my app to ignore any options that I didn't specify in advance still parsing as much of the expected ones as possible.

Given this:

package main

import (
        "flag"
        "fmt"
        "github.com/jawher/mow.cli"
        "os"
)

func main() {
        app := cli.App("cli", "mow.cli test")
        app.ErrorHandling = flag.ContinueOnError
        s := app.String(cli.StringOpt{
                Name:   "s string",
                Value:  "",
                EnvVar: "NEEDED_S",
                Desc:   "somestring",
        })
        app.Action = func() {
                fmt.Println(*s)
        }
        app.Run(os.Args)
}

Currently this fails to read the option -s due to an unexpected option -t having been provided:

$ go run cli.go -s DISPLAYME -t
Error…

This is what I'd expect:

$ go run cli.go -s DISPLAYME -t
Error…
DISPLAYME

mark-kubacki avatar Feb 09 '16 16:02 mark-kubacki

There are 2 things going on here:

  • The ErrorHandling field is not fully documented in mow.cli. My bad. I'll fix it
  • flag.ContinueOnError is not a very good or self-explanatory name

What flag.ContinueOnError means is: stop parsing the args, don't run the target command code and instead return an error. The alternative is to either exit with a non-zero status or to panic.

That's how it is implemented in mow.cli. It does not mean parse as many flags and args as possible, and don't fail even if usage errors (unknown flags for example) are encountered.

The behaviour above is not easily implementable. Take this case for example:

myap -s IDSPLAYNAME  -t SOMETHING

Where -s is a declared flag and -t is not.

Should SOMETHING be treated as the value of the -t flag ? Or should it instead be considered to be a positional argument ? There is no way to know, since -t is not declared and hence wether it's a bool opt (takes no value) or not is unknown.

jawher avatar Feb 09 '16 20:02 jawher

Good points. I believe we have some greenfield freedom here.

I feel that, given your example, -s should be read. If and what everything else is can be ignored. Even if the order were a different one. Except, of course, -s were declared something different than StringOpt. In other words, what -t and rest is should not matter.

If it were myap -t SOMETHING -s IDSPLAYNAME we could argue that an error is justified, or warnings with a flag cli.BestEffortParse (I made it just up) could be displayed (or nothing, or an list with stray items) while still reading everything else.

mark-kubacki avatar Feb 09 '16 20:02 mark-kubacki

@wmark: Just to be sure I'm understanding correctly: you're suggesting that parsing should stop as soon as something erroneous is encountered, right ?

If that's the case, this approach is going to break the backtracking in mow.cli, which is central to correctly parse user input for moderately complex specs. For example:

cmd.Spec="-s | -s -- ARG*"

Which means accept just a -s flag, or a -s flag and consider whatever follow as a positional argument, even if it starts with a - or --.

If a softer mode was to be added, and with the following input:

-s SVALUE -t TVALUE

The parser would start with the first branch -s, match -s SVALUE and then encounter -t, which it considers an error. Usually, mow.cli would backtrack and try the second branch -s -- ARG*, which would succeed (by considering -t as a positional arg).

What are your thoughts about this ?

jawher avatar Feb 10 '16 08:02 jawher

On a side note, I've been thinking (from a while ago) of a way to handle arbitrary flags (not necessarily declared before-hand). Something like with the head or tail commands which accept the number of lines to show directly, e.g. head -3.

One possible way to achieve this would be:

cmd.Opt(CatchAll {
  // help message to be defined
  Accept: func(name, value string) bool { return true }
})

And whenever an unknown option -x is encountered, the catch all Accept function would be called to decide whether to accept or reject -x.

I think this would (indirectly) solve your issue, while also being a more general and flexible approach.

jawher avatar Feb 10 '16 08:02 jawher