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

Attempt at extra args #491

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

perrito666
Copy link
Contributor

@perrito666 perrito666 commented Nov 18, 2023

Proposal

So, this is an idea mostly, i have not been able to test this as running mage just gets the mg from upstream, perhaps i should not use upstream?

The idea is to support this:

//go:build mage
// +build mage

package main

import (
	"fmt"

	mg "go/github.com/magefile/mage/mg"
)

func TargetOne(extra mg.ExtraArgs) {
	fmt.Printf("%#v\n", extra)
}

where ideally extra would become a string slice []string{"-baz", "blah"} after calling mage targetone foo bar -- -baz blah

Implementation

I manged to implement this by having the ExtraArgs be handled out of band with the actual Args, they are accounted by mage and the underlying generated mage file and both handle them correctly.

Ideally users of this will use this as sampled in the various targets in the tests (which is to say, in any way they want) and they will always receive the list of arguments.

The caller should then be the one in charge of processing these, we do no processing whatsoever except removing the leading -- if the user's code implements a flag parser they can pass ExtraArgs as is and get it parsed. If they want to just give this to a sub-process they can append to the arg list adding again --.

This does not solve the issue of people wanting to pass arbitrary arguments to targets, but solves the way to at least being able to pass some form of arbitrary arguments (useful,again, for invoking sub-processes that take external modifyers by commandline)

Note: every target implementing ExtraArgs will receive the same copy of the slice, it is their responsibility to process them.

@perrito666 perrito666 marked this pull request as draft November 18, 2023 23:14
Now extra args are treated as separate but merged to call the function.
Extra testing is needing to try them in conjunction with other tests.

Every function receives the same extra args
@perrito666 perrito666 changed the title Draft attempt at extra args Attempt at extra args Nov 19, 2023
@perrito666 perrito666 marked this pull request as ready for review November 19, 2023 07:32
Remove increase in x index count when iterating args sin mage file.
@perrito666
Copy link
Contributor Author

Ok @natefinch this works now as I intended, let me know if this is something we want to support.

@natefinch
Copy link
Member

I think we need a test where we run mage.ParseAndRun() and pass in a raw args list and see that it does the right thing. Right now, the code in mage/main.go in the Parse function is never tested, and that's pretty key.

@perrito666
Copy link
Contributor Author

@natefinch done, added the same test cases, which i consider to be relevant but also using ParseAndRun

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

Successfully merging this pull request may close these issues.

None yet

2 participants