Cli: Add support for flags that also behave as switches

[purpleidea]

If there's a way to do this, it's not apparent to me or in the docs, but there doesn't seem to be an obvious way to distinguish between --someflag "" and no flag specified at all. Using a magic sentinel as the default value is _not_ a good approach for many cases.

/cc @ffrank

[ffrank]

In practice, though, requiring the user to specify --someflag "" is poor UX, anyway.

It would be nice to be able to add flags with polymorphic type, so that its value can either be boolean or string. This way, you could check for presence like

    if context.Bool("someflag") {
        // handle the case of --someflag
    } else if context.String("someflag") != "" {
        // handle the case of --someflag got-a-value
    } else {
        // flag was not passed at all
    }

Each possible type would take a zero value. The given value is considered to have the most fitting type by some rules (e.g. numbers before strings etc.). Any lookup function for another type yields the zero value.

For example, if a flag can be bool, string or int, the value 24 will return "" when looked up as a string. (Having such a flag seems foolhardy, but who am I to judge;)

The most likely occurrence will be to have (X,Bool) optional values. So I'd be happy with just getting that, and not allow arbitrary value type variations as outlined above.

[meatballhat]

@ffrank What you're proposing sounds to me like we'd either have to ditch wrapping stdlib flag (untested) and do our own flag parsing, or perhaps _only_ use the stdlib flag layer for the most basic parsing and treat everything as a generic... which sounds fun/awful/scary :smile_cat:. Thoughts?

[ffrank]

Oh, I only just realized that his _is_ in fact part of stdlib. In light of this, I guess I'd vote to close...work with what we have.

If you really _do_ want to expand upon the capabilities of the flag module, I suggest you adhere to KISS by stealingmimicking stdlib's code and perhaps adding *Optional derivatives (should the lookup values return both a bool and the type in question? I'm sure there are interesting possibilities.)

[purpleidea]

@ffrank I actually disagree with your "polymorphic types suggestion" I don't think it fits well with golang. Instead if you want this, then you pick a string type and parse it to an int on your own. I do however think you should be able to have string value (for the string type) and a boolean for each flag which is "IsSet" true/false. This wouldn't upset the golang types.

[jszwedko]

@purpleidea it feels like I'm missing something, but is this behavior not covered by context.IsSet()?

package main

import (
        "fmt"
        "os"

        "github.com/urfave/cli"
)

func main() {
        app := cli.NewApp()
        app.Name = "greet"
        app.Flags = []cli.Flag{
                cli.StringFlag{
                        Name:  "foo",
                        Value: "",
                },
        }
        app.Action = func(c *cli.Context) error {
                fmt.Println(c.IsSet("foo"))
                return nil
        }
        app.Run(os.Args)
}

$ go run /tmp/tmp.go --foo "" 
true

[jszwedko]

Polymorphic flags would be interesting, I can see the use case for that though it would require using a different flag parser (and may also be slightly confusing to the end-user depending if the command also takes arguments).

[purpleidea]

@jszwedko Actually, this is really helpful, thanks!

A bonus would be if there was some way that running:

go run /tmp/tmp.go --foo

Would not throw an error. Ideally this would be IsSet: true, c.String("foo") == ""

Cheers!

[jszwedko]

@purpleidea that is an interesting idea -- as mentioned before, it'd require implementing our own flag parser -- but I'll give that some more thought.

[purpleidea]

@jszwedko I'd appreciate it! Perhaps a better way to phrase it more generally (describe the API) would be that if some flag setting DefaultEmpty was true, then it would use the default when passed without an argument. Eg:

cli.StringFlag{
    Name:  "someflag",
    Value: "default",
    DefaultEmpty: true, // woo!
    Usage: "theFlag",
},

Please note that my naming of DefaultEmpty is terrible. I can never think of good variable/flag names.

Cheers

[jszwedko]

Updating title to be closer to what we arrived at after the discussion

[lynncyrin]

Given that this is from last year, I think I'm comfortable closing it 🙂 feel free to re-open / open a new issue / comment in support if there's still interest here!

[purpleidea]

@lynncyrin You should keep it open please. Thanks!

[lynncyrin]

Got it 👍

[stale[bot]]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[lynncyrin]

\ bump ^^ this one is still looking for a contributor 🙏

[stale[bot]]

This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant.

[stale[bot]]

This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else.

[stale[bot]]

Closing this as it has become stale.