Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to get variadic arguments #340

Open
mondy opened this issue Mar 14, 2021 · 11 comments
Open

How to get variadic arguments #340

mondy opened this issue Mar 14, 2021 · 11 comments

Comments

@mondy
Copy link

mondy commented Mar 14, 2021

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
Copy link
Member

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
Copy link
Author

mondy commented Mar 15, 2021

Thank you for your reply.
I will try this.

I hope variadic to be supported.

@danbrakeley
Copy link
Contributor

@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
Copy link

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
Copy link

flowchartsman commented Dec 27, 2022

@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
Copy link

spockz commented May 10, 2023

@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
Copy link

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
Copy link

spockz commented Oct 4, 2023

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

@perj
Copy link
Contributor

perj commented Oct 4, 2023

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

@spockz
Copy link

spockz commented Oct 4, 2023

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
Copy link

mloskot commented Nov 17, 2023

Would this idea address support for optional arguments too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants