magefile / mage

a Make/rake-like dev tool using Go
https://magefile.org
Apache License 2.0
4.02k stars 250 forks source link

How to get variadic arguments #340

Open mondy opened 3 years ago

mondy commented 3 years ago

I read the Arguments section of the https://magefile.org/targets/. I wanted to do something like the following, but I couldn't.

func Run(arguments ...string) {
    args := append([]string{"run", "."}, arguments...)
    sh.RunV(mg.GoCmd(), args...)
}

When executed, the output will be as follows

Unknown target specified: "run"

Is there a way to get variadic arguments?

natefinch commented 3 years ago

Variadics aren't currently supported.... But in theory they could be. Right now you'd have to do something like

mage run "foo bar baz" And then do a strings.Split() by hand in the code.

mondy commented 3 years ago

Thank you for your reply. I will try this.

I hope variadic to be supported.

danbrakeley commented 3 years ago

@mondy I don't think variadics play nice with specifying multiple build targets.

For example, if your mage file had func Foo(arg1, arg2 string) and func Bar(), then you can do mage foo value1 value2 bar, which would call both Foo("value1", "value2") and Bar(). But if instead your Foo was defined as func Foo(args ...string), then mage foo value1 value2 bar would call Foo("value1", "value2", "bar").

I feel like this ambiguity in behavior would cause a lot of confusion.

avarabyeu commented 1 year ago

Still, the feature is quite useful. Consider wrapping some docker/compose call:

func (Docker) Compose(args []string) error {
    compose := sh.RunCmd("docker", "compose")
    return compose(args...)
}

it could be an option to make unknown args/targets available over context, for instance:

func (Docker) Compose(ctx context.Context) error {
    compose := sh.RunCmd("docker", "compose")
    argsArr := mg.CtxArgs(ctx) 
    return compose(argsArr...)
}
flowchartsman commented 1 year ago

@mondy I don't think variadics play nice with specifying multiple build targets.

For example, if your mage file had func Foo(arg1, arg2 string) and func Bar(), then you can do mage foo value1 value2 bar, which would call both Foo("value1", "value2") and Bar(). But if instead your Foo was defined as func Foo(args ...string), then mage foo value1 value2 bar would call Foo("value1", "value2", "bar").

I feel like this ambiguity in behavior would cause a lot of confusion.

Just randomly browsing issues and wanted to throw an idea in on this. What you could do is allow variadic targets to be separated with with -- and use a heuristic to throw "ambiguous argument" errors in the case where an argument to a variadic shares the name with a valid target unless there is a terminal --.

So, for example, given targets

func Run(arguments ...string) error {/**/}
func Stuff() error {/**/}

The behavior would be as follows:

mage run my things
# execute target "run" with args ["my", "things"]

mage run my stuff
Ambiguous argument to "run": "stuff" is also a target name, please use `--` to terminate the arguments to target "run"

mage run my -- stuff
# execute target "run" with args ["my"]
# execute target "stuff" with args []

mage run my stuff --
# execute target "run" with args ["my", "stuff"]
spockz commented 1 year ago

@mondy I don't think variadics play nice with specifying multiple build targets.

IMHO, that is why variadic arguments typically have to be the last argument in a function. We can translate this that a target with variadic arguments can only be called if it is the last target. Or combine with the suggestion from @flowchartsman and add separators that are only needed when needing to fix ambiguity.

thorntonrose commented 9 months ago

I think it would be nice if Mage offered a flag for key/value pairs, like this:

mage -v test -p pkg=./foo -p test="TestFoo*"

The key/value pairs could be accessed via an implicit variable or a function, like this:

func Test() {
   pkg := mg.props["pkg"] // default value would be ""
   // ...
}

Or this:

func Test() {
   pkg := mg.Property("pkg", "./...") // second param is default value
   // ...
}

The -p flag, or something similar, is used by many build tools.

spockz commented 9 months ago

I saw #402 adds variadic support, although the documentation seems missing. I'll try the feature for our use case.

perj commented 9 months ago

Sorry about the missing documentation. Could you point me to it so I can amend it?

spockz commented 9 months ago

Okay. It seems I jumped the gun in my enthousiasm. :) It adds the support to mg.F, but it seems that doesn't automatically translate in variadic support for targets. Sorry for claiming missing documentation for a feature that is not there!

//go:build mage

package main

// "github.com/magefile/mage/sh"

// Runs go mod download and then installs the binary.
func Build(foo ...string) error {
    for i := 0; i < len(foo); i++ {
        println(foo[i])
    }
    return nil
}

Running mage (1.15) gives me:

❯ mage build 1
Unknown target specified: "build"
mloskot commented 7 months ago

Would this idea address support for optional arguments too?