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

I guess arguments work now #326

Merged
merged 7 commits into from
Dec 29, 2020
Merged

I guess arguments work now #326

merged 7 commits into from
Dec 29, 2020

Conversation

natefinch
Copy link
Member

@natefinch natefinch commented Dec 23, 2020

200 (2)

So, this works now:

func Say(msg string) {
  fmt.Println(msg)
}

$ mage say hi
hi

Please test with

go get github.com/magefile/mage@nargs

Note: only supports int, string, bool, and time.Duration as arg types. Let me know if there are other types that are necessary.

@natefinch
Copy link
Member Author

Once I figured out that the best UX was also the easiest to program.... I just banged it out.
I had been thinking that I needed to do something like mage say -msg=hi and programming all the flags and crap was going to be a PITA...
But that was assuming that I had to make args distinguishable from targets... and I just don't think that's all that important in reality. Plus, it's a lot nicer to just type mage say hi ... and if you happen to have a target called hi that you meant to call, well, you'll figure it out and understand your own mistake.

So, yeah, that's the thing. Note it only support string, int, bool, and time.Duration.... is there really anything anyone else needs? Are people really gonna type out an ISO 8601 date or something? in theory I could support any number type, but like.. why?

@natefinch
Copy link
Member Author

Hmm, it occurs to me that I'll need to update mg.Deps to support targets with arguments... probably will do some reflection magic like

mg.Deps(mg.F(Say, "hi"))

I could probably do mg.Deps(Say, "hi") ... I'll have to think about it.

This hopefully would also fix the problem people have right now with trying to pass closures to mg.Deps.

@svengreb
Copy link

This is a awesome, early Christmas present 💙

[…] and if you happen to have a target called hi that you meant to call, well, you'll figure it out and understand your own mistake. […]

Most of the time the minimal solution is the way to go, sometimes we tend to “overthink“ problems 😄
If someone wants to pass to targets in one command it can simply be solved by creating a dedicated target that runs both or defines the first one as dependency.

Going to use the branch to play around with it in my projects.

@natefinch
Copy link
Member Author

To be clear, this does support multiple targets each with their distinct args being passed on the cli.

@natefinch
Copy link
Member Author

mg.Deps now accepts an mg.Fn interface, which can let you pass your own function - usually these will be created by mg.F(func, arg1, arg2...) but you could in theory implement your own mg.Fn (but hopefully you won't have to)

@natefinch
Copy link
Member Author

natefinch commented Dec 28, 2020

func Say(something string) {
    fmt.Println(something)
}

func Talk() {
    // this will Do The Right Thing
    // which is to say, it'll run Say("hi!") exactly once and Say("bye!") exactly once
    mg.Deps(mg.F(Say, "hi!"), mg.F(Say, "bye!"), mg.F(Say, "hi!"))
}

you can also do it this way, which might be more clear:

    hi := mg.F(Say, "hi!")
    bye := mg.F(Say, "bye!")
    mg.Deps(hi, bye, hi)

(of course, you'd never actually pass the same dependency twice to mg.Deps because it won't run the second one, by definition)

@natefinch natefinch merged commit aba3950 into master Dec 29, 2020
@hampgoodwin
Copy link

References #24

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.

3 participants