jawher / mow.cli

A versatile library for building CLI applications in Go
MIT License
871 stars 55 forks source link

Request: parse expected flags with flag.ContinueOnError #30

Open mark-kubacki opened 8 years ago

mark-kubacki commented 8 years ago

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
jawher commented 8 years ago

There are 2 things going on here:

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.

mark-kubacki commented 8 years ago

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.

jawher commented 8 years ago

@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 commented 8 years ago

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.