mage icon indicating copy to clipboard operation
mage copied to clipboard

Add command line flag support

Open ryanfaerman opened this issue 5 years ago • 9 comments

This builds on the work that was recently merged, adding argument support. I've extended it a bit, adding support for optional flags in addition to positional arguments.

Given the following function in a magefile:

func Say(animal string, loud mg.BoolFlag, msgs mg.StringSliceFlag) {
        fmt.Println("what does the", animal, "say?")
	if loud {
		fmt.Println("GET LOUD")
	}

	for _, m := range msgs {
		if loud {
			m = strings.ToUpper(m)
		}
		fmt.Println(m)
	}
}

It can now be executed as any of the following:

mage say fox --loud --msgs="bark bark" --msgs="woof woof" --msgs="yip yip"
mage say fox --msgs="bark bark" --msgs="woof woof" --msgs="yip yip"
mage say --msgs="bark bark" --msgs="woof woof" --msgs="yip yip" fox
mage say fox --msgs="bark bark"

ryanfaerman avatar Dec 31 '20 04:12 ryanfaerman

So the flags become optional parameters that you need to use -- to specify? That's interesting... but I'm not 100% sure the extra complexity in the code and the docs is worth it. Lemme think about it.

natefinch avatar Sep 27 '21 04:09 natefinch

@natefinch FWIW, I do think this is worthwhile. having named arguments is often more clear to users because they're more explicit. I've also had need for optional parameters to a target that I had to use environment variables for, and that's less discoverable for users.

arschles avatar Oct 01 '21 21:10 arschles

I'll add a quick note to agree with @arschles. The one thing that I was really surprised at and actually made things initially more complicated in adopting mage was lack of standard flag handling. I'd be fine with param=1 mage do or sticking with standard mage do -filter "foo" as well. I have tried to avoid parameters for any tasks where possible because it seems I can't make them optional right now (maybe I'm wrong there), and it's confusing to type parameters as if they are commands.

If possible, I'd wish to support single dash flags since that's so common in Go, but double dash is also great.

sheldonhull avatar Oct 02 '21 03:10 sheldonhull

@ryanfaerman, thank you so much for this.

@natefinch 👋 , can we please consider to add this? I've read through different threads (e.g. https://github.com/magefile/mage/issues/24) and to me it feels as this is the right approach, given that Golang doesn't support overloading methods or optional arguments.

At the time of writing I have to do this:

func Test() error {
	mg.Deps(Prep)
	pkgNames := os.Getenv("PKG")
	filterRegex := os.Getenv("FILTER")
	args := []string{"test", "-v"}
	if len(pkgNames) != 0 {
		for _, pkgName := range strings.Split(pkgNames, ",") {
			args = append(args, "./"+pkgName)
		}
	} else {
		args = append(args, "./...")
	}

	if len(filterRegex) != 0 {
		args = append(args, "-run", filterRegex)
	}

	return sh.Run("go", args...)
}

And then I can invoke the following ways:

// All cases
mage test

// Single package
PKG="util" mage test

// Multiple packages
PKG="util,crypt" mage test

// Multiple packages with filter
PKG="util,crypt" FILTER="Test*" mage test

Doesn't feel as intuitive or perhaps it's just me doing it wrong.

giladreich avatar Oct 31 '21 19:10 giladreich

Hello, just stopping by to say that I would be interested by this PR! The main advantage to me is the discoverability of target options/flags and better support for Windows (where SOMETHING=asd mage ... is not valid).

I definitely see the value in this, but I think I want to go a slightly different way.

I think we could dispense with mg.BoolFlag and just use pointers (*bool etc) instead. I think that's obvious enough to a go developer that *bool might be true, false, or unset.

natefinch avatar May 12 '22 19:05 natefinch

What's the status on this? We build internal tooling using mage, and we have had to use a plethora of ENV vars in place of flags, which can be very convoluted at times :/

c0nfleis avatar Jan 26 '24 19:01 c0nfleis

I'll spend some time on it this weekend, see if I can knock it out. I know it would be very useful.

natefinch avatar Jan 26 '24 19:01 natefinch