-
-
Notifications
You must be signed in to change notification settings - Fork 677
feat: wildcard match aliases #2234
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR introduces support for wildcard matching of task aliases by refactoring task-matching logic into a single function, FindMatchingTasks, and updating related components.
- Updated task definitions in Taskfile.yml to include alias patterns
- Modified WildcardMatch in taskfile/ast/task.go to iterate over both task names and aliases
- Adjusted FindMatchingTasks and GetTask in task.go to handle conflicts when matching aliases
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
testdata/wildcards/Taskfile.yml | Added alias definitions to support wildcard alias matching |
taskfile/ast/task.go | Updated WildcardMatch to check both task names and aliases in one loop |
task_test.go | Added test case to verify alias based matching |
task.go | Refactored FindMatchingTasks and GetTask to return conflict errors for multiple alias matches |
Files not reviewed (1)
- website/docs/usage.mdx: Language not supported
names := append([]string{t.Task}, t.Aliases...) | ||
|
||
for _, taskName := range names { |
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.
[nitpick] Consider renaming 'names' to 'candidateNames' to improve clarity when iterating over both task names and aliases.
names := append([]string{t.Task}, t.Aliases...) | |
for _, taskName := range names { | |
candidateNames := append([]string{t.Task}, t.Aliases...) | |
for _, taskName := range candidateNames { |
Copilot uses AI. Check for mistakes.
func (e *Executor) FindMatchingTasks(call *Call) ([]*MatchingTask, error) { | ||
if call == nil { | ||
return nil | ||
return nil, nil |
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.
Returning nil for both matching tasks and error when 'call' is nil might be ambiguous for API consumers; consider returning an explicit error or adding documentation to clarify this behavior.
Copilot uses AI. Check for mistakes.
Summary
The wildcard did not match aliases.
I've done a bit of refactoring and moved some code into
FindMatchingTasks
, but its signature has changed. Is it OK for the package API, or should I consider another solution ?The goal was for
FindMatchingTasks
to also handle matching aliases (even without wildcards). It felt more logical to group all the task-matching logic in one place.Fixes #1900
NOTE: I modified the test, but I did not migrate it to the new system yet