-
-
Notifications
You must be signed in to change notification settings - Fork 624
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
Aliases #879
Aliases #879
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.
Awesome work! 👏
I didn't test it yet, but the code is almost perfect. I did a couple of comments only.
Thanks a lot for your help 🙌
docs/docs/changelog.md
Outdated
@@ -5,6 +5,10 @@ sidebar_position: 6 | |||
|
|||
# Changelog | |||
|
|||
## Unreleased | |||
|
|||
- Add ability to set `aliases` for tasks and namespaces (#879). |
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.
- This may be confusing, but the right flow at the moment is to write changes to
/CHANGELOG.md
only, and on release I calltask docs:changelog
which will actually update this file according to this one. - Unfortunately linking issues/PR with just
#num
won't always work (on the website, for example). We need to make a proper link as([#num](url))
as the others below. - Mention Task identifier shorthand notation / aliases #268 and Feature: aliases #340 as well?
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.
Whoops, yeah that makes sense. I missed that there are two copies of the file. Should be fixed now
docs/docs/usage.md
Outdated
aliases: | ||
- gen |
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.
What about using one line arrays for aliases
mentions on documentation? This would be consistent with deps
and is shorter.
aliases: | |
- gen | |
aliases: [gen] |
docs/docs/usage.md
Outdated
@@ -936,6 +950,27 @@ If the task does not have a summary or a description, a warning is printed. | |||
|
|||
Please note: *showing the summary will not execute the command*. | |||
|
|||
## Task aliases | |||
|
|||
Aliases are alternative names for tasks. They can be used to make it easier and quicker to run tasks with long or hard-to-type names. You can use them on the command line, when [calling sub-tasks](#calling-another-task) in your Taskfile and when [including tasks](#including-other-taskfiles) with aliases from another Taskfile. They can also be used together with [namespace aliases](#namespace-aliases). |
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.
As a convention, we try to wrap paragraphs in 80 chars when possible.
help.go
Outdated
taskNames := append([]string{task.Name()}, task.Aliases...) | ||
fmt.Fprintf(w, "* %s: \t%s\n", strings.Join(taskNames, "|"), task.Desc) |
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 believe this may possible break the auto-completion scripts that use --list
to parse the list of available tasks.
Because of this, I think we may need to move the somewhere after. Not sure about the format, but maybe something like this?:
task: Available tasks for this project:
* foo: prints foo (aliases: f)
* included:bar prints bar (aliases: included:b, inc:b)
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.
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 end up changing that myself, taking your suggestion of printing in a separate column, and colored.
internal/summary/summary.go
Outdated
l.Outf(logger.Default, "aliases:") | ||
for _, alias := range t.Aliases { | ||
l.Outf(logger.Default, " - %s", alias) | ||
} |
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 liked the format choice this this 👍
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've updated the PR and removed the alternative syntax in favour of this. I've also set the color of aliases to Cyan since the colored output is now merged. Wdyt?
task.go
Outdated
} | ||
if t.Internal { | ||
// If the task is internal |
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 don't think this comment is needed. The code is self-descriptive.
// If the task is internal |
task.go
Outdated
// Search for a matching task | ||
matchingTask, ok := e.Taskfile.Tasks[call.Task] | ||
// If didn't find one, search for a task with a matching alias | ||
if !ok { |
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.
Early returns are preferred. Check for if ok { return }
and move the current block to after it.
if ok {
return matchingTask, nil
}
// Rest of the code
I like this a lot 👍 One thing I feel is missing though is aliases across namespaces. In my particular use case (a mono repo), I for instance have various tools under the version: "3"
includes:
tools: ./tools
tasks:
foobar:
alias: tools:foobar:run Could something like this fit within the alias concept somehow? |
@postlund "flattening" included files is something I considered, but I decided against it for a few reasons:
I would suggest adding to the conversation in #273 if you're interested in moving this idea forwards. |
feat: add aliases to --list and --list-all flags feat: add aliases to --summary feat: enable aliases for included tasks tests: added alias unit tests
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.
That is cool, I do often like to have 1-2 letter aliases, like with git.
Awesome work @pd93! |
Task
aliases
have been discussed a few times in #268, #340, #675. These discussions have ranged in scope and detail, but no design has yet to be established. This PR attempts to implement the simplest possible version ofaliases
as a proof of concept.Aliases are alternative names for tasks. They can be used to make it easier and quicker to run tasks with long or hard-to-type names. You can use them on the command line, when calling sub-tasks in your Taskfile and when including tasks with aliases from another Taskfile.
There are 3 parts to this PR:
--list
/--list-all
/--summary
Task Aliases
Task aliases allow users to set a list of alternative strings that can be used to call a task. In the example below, we have 2 tasks:
foo
andbar
. We can call either of these tasks in the normal way withtask foo
ortask bar
respectively, but thanks to aliases, we are now also able to calltask f
andtask b
with the same behaviour.Both tasks also have
x
aliased. If we were to runtask x
, Task will return an error as aliases are not allowed to collide.Note that
foo
callsbar
and that it is able to do this by usingbar
's alias:b
.All of the above also works with included taskfiles. In the below example we can call
task f
andtask included:b
from the command-line with the same behaviour as the previous example.Namespace Aliases
Namespace aliases are a way to create alternative names for the namespace of an included taskfile. In the example below, we have adjusted the taskfile so that the included taskfile:
included
is now aliased toinc
. This means that we can now calltask inc:b
and it will behave the same astask included:b
:When a task with aliases is merged into a file with a namespace alias, these aliases form a matrix and they are all merged into the task. This means that all of the following are callable:
task foo
task f
task included:bar
task included:b
task inc:bar
task inc:b
--list
/--list-all
/--summary
These flags all print information about one or many tasks. Now that we have aliases, these need to be updated. Below are some examples of the new output for each flag when run against the taskfiles in the Namespace Aliases section above:
task --list
/task --list-all
:Aliases are displayed after the main task name and separated by
|
characters. I'm slightly concerned about how long some of these lines might get with the matrix merging, so we may need to find a better way of displaying this.task foo --summary
:The output above show two options for how we could display aliases in the summary text. Either by having a
|
separated list in brackets next to the task name (similar to the--list
/--list-all
flags), or having a dedicated section with a bullet list like the commands - I think I prefer the latter.Other noteworthy changes
FIXME
in the codebase about deep copying the tasks when merging an included taskfile. This caused some issues when implementing this change, so I have also added a couple of new utility functions intaskfile/copy.go
calleddeepCopySlice
anddeepCopyMap
. These leverage generics to create a deep copy of slices and maps respectively. These utils are then used in 3 newDeepCopy
methods on theTask
,Vars
andIncludedTaskfile
structs to create a full deep copy of a task when merging. Hopefully this will help to reduce bugs in the future.golang.org/x/exp/slices
package to do generic "slice contains element" lookups. This also means I was able to remove the non-generic version of this function intaskfile/slice.go
. This package is expected to become a part of the standard library in the next few Go releases.usage.md
andapi_reference.md
docs.label
), so I added these too.changelog.md
doc.