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 ~~stealing~~mimicking 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

[coilysiren]

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!

[coilysiren]

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.

[coilysiren]

\\ 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.

[skelouse]

I made it here when trying to implement flags where the following commands are valid with a flag `local

$ tau local is not set, thus we prompt the user

$ tau --local local is set as true

$ tau --local=false local is set as false

I decided on an alternative where I take a flag, say local

$ tau --local local is set as true

$ tau --no-local local is set as false

I did so in the following implementation which in the above example takes cli.BoolFlag{Name: "local"}, and transforms it to two flags with localandno-local`. This could be implemented on this package as a cli.BoolFlagWithInverse{}, and the error handled when parsing the flags.

package flags

import (
	"fmt"

	"github.com/urfave/cli/v2"
)

type boolWithInverse struct {
	flag    *cli.BoolFlag
	inverse *cli.BoolFlag
}

func (s *boolWithInverse) flags() []cli.Flag {
	return []cli.Flag{s.flag, s.inverse}
}

func (s *boolWithInverse) bothFlagsSet(ctx *cli.Context) error {
	set := ctx.IsSet(s.flag.Name)
	inverseSet := ctx.IsSet(s.inverse.Name)
	if set && inverseSet {
		return fmt.Errorf("cannot set both flags `--%s` and `--%s`", s.flag.Name, s.inverse.Name)
	}

	return nil
}

func (s *boolWithInverse) IsSet(ctx *cli.Context) bool {
	return ctx.IsSet(s.flag.Name) || ctx.IsSet(s.inverse.Name)
}

func (s *boolWithInverse) Value(ctx *cli.Context) bool {
	return ctx.Bool(s.flag.Name)
}

// TODO move to utils
func clonePtrValue[T any](value *T) *T {
	if value == nil {
		return nil
	}

	t := *value
	return &t
}

// NewBool returns a BoolFlagHandler, one with the bool flag-provided and the other with no-flag-provided
func NewBoolWithInverse(flag cli.BoolFlag) BoolWithInverse {
	special := &boolWithInverse{}

	if flag.Action == nil {
		flag.Action = checkDoubleFlagSet(special)
	} else {
		flag.Action = func(ctx *cli.Context, v bool) error {
			err := checkDoubleFlagSet(special)(ctx, v)
			if err != nil {
				return err
			}

			return flag.Action(ctx, v)
		}
	}

	special.flag = &flag

	// Append `no-` to each alias
	var inverseAliases []string
	if len(flag.Aliases) > 0 {
		inverseAliases = make([]string, len(flag.Aliases))
		for idx, alias := range flag.Aliases {
			inverseAliases[idx] = "no-" + alias
		}
	}

	special.inverse = &cli.BoolFlag{
		Name:        "no-" + flag.Name,
		Category:    flag.Category,
		DefaultText: flag.DefaultText,
		FilePath:    flag.FilePath,
		Usage:       flag.Usage,
		Required:    flag.Required,
		Hidden:      flag.Hidden,
		HasBeenSet:  flag.HasBeenSet,
		Value:       flag.Value,
		Destination: clonePtrValue(flag.Destination),
		Aliases:     inverseAliases,
		Count:       clonePtrValue(flag.Count),
		Action:      flag.Action,
	}

	if len(flag.EnvVars) > 0 {
		// TODO we need to append to the action to reverse the value of the env vars
		special.inverse.EnvVars = append([]string{}, flag.EnvVars...)
	}

	return special
}

func checkDoubleFlagSet(flag *boolWithInverse) func(ctx *cli.Context, v bool) error {
	return func(ctx *cli.Context, v bool) error {
		return flag.bothFlagsSet(ctx)
	}
}

And in the action of a command:

// Instantiate the flag before the app is ran
var local = NewBoolWithInverse(cli.BoolFlag{
    Name: "local"
})

// In the action
var ctx *cli.Context

flagSet, err := local.IsSet(ctx)
if err != nil {
	return false, err
}

if flagSet == true {
	return local.Value(ctx)
}

// Prompt...

[dearchap]

@skelouse Excellent work. Can you submit a PR for this ? We can get it included in mainstream v3