-
-
Notifications
You must be signed in to change notification settings - Fork 625
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
feat: wildcard matching for task names #1489
Conversation
Thanks for this work. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! 🚀
I agree that a wildcard is probably enough. Regexp was suggested on the issue, but it'd be too complex IMO, specially when mixing in YAML.
docs/docs/usage.md
Outdated
If multiple matching tasks are found, the first one listed in the Taskfile will | ||
be used. If you are using included Taskfiles, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Would return an error be better if multiple matches are found?
- Reminder: the second sentence is still incomplete 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
I think that adding this restriction is fine for now. It keeps things simple. However, I can definitely see a world where someone with a complex setup asks for the current behaviour as it would allow you to do something like this:
version: 3 tasks: foo-*-*: .. foo-*: ..
This allows Task to do something different depending on if the user does
task foo-bar
vstask foo-bar-baz
. This would not be possible if we errored when multiple tasks match sincetask-bar-baz
matches both tasks in the file.Happy to add the error for now and revisit this later though.
-
Haha, I probably started writing this and then realised I didn't know what the behaviour with includes was, so went to check and never came back. I have completed the sentence now 😆
Fixes #836
Context
This PR is a proposal for a design that would allow basic wildcard matching for task names. At the moment, if users want to make a generic task that performs a common set of operations on many different services or directories (etc) they must either:
.CLI_ARGS
and the--
operatorBoth of these methods can be clunky to type into a CLI and are not very intuitive.
make
has the option to do wildcard matching (using%
). This PR is a proposal to do the equivalent in Task.Proposal
When a Taskfile has a task with a name that contains a wildcard (
*
) and the user callstask
with a task name that "matches" a wildcard task, that task will be run. Matches will occur by replacing any instances of*
with the(.*)
regex string and doing a regex match. The captured variables will be made available to the Task via theMATCH
variable in the form of an array. For example:> task start-foo Starting foo ...
Implementation
The first commit updates all usages of
ast.Call
to be passed by reference. Doing this allows methods that accept a call to update the call's variables. This means we can add any wildcard matches into thecall.Vars
without have to propagate them and change a bunch of function signatures.Other than this, the implementation is very simple, we are simply overriding the
omap.Get()
method by adding aGet()
method directly onast.Tasks
and then adding a simpleWildcardMatch()
function toast.Task
which will work out if a Task matches the call and retrieve the wildcard variablesNotes
I haven't written any docs yet since I'd like some feedback on the design/implementation. I'll add this once we've decided we're happy with it. What do people think about the naming? i.e.
*
andMATCH
. There were some other ideas discussed in #836, but I think starting with simple wildcard matching is probably a good start. I personally, would find this to be more than enough for my use cases.Final note. The
omap.Range()
function doesn't support yielding right now so there is a bit of a hack to get around this (seeTODO
in code). There is an interesting iterator proposal that will be introduced to Golang 1.22 as an experiment. This would be a really good solution to this, but we probably won't be able to add this until it is added to the language permanently (probably 1.23) and we drop support for 1.22 (so around a year from now).