Closed ssbarnea closed 1 year ago
Hi @ssbarnea, thanks for opening an issue for this!
I'd love to get some help with this.
Regarding task status, we already have a --status
flag that can be used to know that for a given task.
About the JSON output, I think I would not attach that to the --list
flag, but instead have a flag specifically for this (--json
?).
Previous discussion: #753
The effective command counts as an less important implementation detail, what I was mostly interested was to get an ack from the maintainer that this feature is desired to be included.
@ssbarnea To be clear: yes, I see this as a very useful feature to support integrations (as code extensions, etc).
And I'm also open to have an official VSCode extension if we have enough people interested in maintaining that, as I said in the other discussion.
I'd like to give it a go. And since I know TS I'll see if I can come up with an extension.
@ilewin We already have an extension, we only need to make taskfile produce json, so it would more future proof.
@ssbarnea Great! I'll do my best with JSON output then.
As said on https://github.com/go-task/task/discussions/753, we actually have two extensions maintained by the community.
Once we have this JSON output working, perhaps we should have an official one to give it more tracking and avoid confusion on which one is preferred. For this, I'd like to have volunteers, though, as my plan is to focus my time on Task itself.
@ilewin are you working on this? if not I might give it a try...
draft changes available for review as #884
this PR so far only addresses the output of the --list
and --list-all
flags.
if it needs to also address all output from the Executor I can revisit.
Just a side note, but since Taskfiles are just yaml, you can use yq
to parse and get info from specific tasks.
I see this as a big advantage over eg. Makefiles
yq eval '.tasks.default.cmds[]' Taskfile.yaml
echo "{{.GREETING}}"
I forgot that we already have a schema at https://json.schemastore.org/taskfile.json - maybe we would like to move it inside taskfile itself, to have it much easier to update. I did the same transition on 3 projects (ansible-lint, ansible-navigator and molecule) as I can extend/change schema atomic, no longer needing to remember to also update the schema after a change that affects it.
As a note, I do think that the output JSON should be compatible (very close) to config file, but it does not have to be the same config. I would expect that the JSON output would produce an exploded/rendered/resolved output, for example it should never have "includes". The idea is to return the in-memory representation of the config.
PS. I know how to validate a JSON using schema using Python and JavaScript/NPM.
Hey @davidalpert, thanks for working on that. I plan to review your changes once possible.
@ssbarnea Having the ability to generate the schema automatically would be interesting. I just don't know how easy or viable that it.
It also makes sense what you said about the expected output being the resolved form and not the raw one (should have variables already interpolated, etc). Implementation-wise, if to get the best result we'd need to declare separated Go structs I think that's fine. Seems that @davidalpert used the existing schema initially. Once I have time to review more carefully I may have more insights about the implementation.
Generating a schema dynamically would be overkill unless you are already using a loading library that supports this. On python world there is pydantic that can be used for that. Still, I am not using it and I "handcraft" them, which is perfect for any project that does not add/remove/change a lot of these.
Keep in mind that JSON Schemas are very good source of information, in fact they could be used to generate documentation. I plan to look deeply too, once I get less work, we have a release in two weeks, so after.
this is my first crack at contributing to this code base. feedback is very welcome. as well, feel free to throw it out and start again if a different direction is more appropriate.
a pattern I have used on other projects, adopted from the kubectl
codebase if I recall, is to abstract the writing to STDOUT and introduce some helpers such that a client might request different --output
formats (e.g. --output json
or --output yaml
would that be of use to consider here also?
I see that task
already has a --output
flag with these options:
so would another related flag, like --output-format=json|yaml
make sense?
I see the choice to format output as text|json|yaml
is orthogonal to the existing output options, meaning that it applies to all output independent of the other --output
choices. is that correct? or maybe chosing json
format is in conflict with --output prefix
as a prefix makes more sense as another custom tag in json output.
@andreynering if you are open to it we might schedule a zoom call to discuss the feature and align on a direction. I am happy to contribute effort but want to make sure that the usage semantics are what you expect before I drive too deep into implementation.
I'll try to write some thoughts about the design, and joining the discussion from #884 into this issue.
I think this will be simple, and a call is not necessarily needed.
My proposal:
1) Yes, let's start with an MVP and we can expand later. That's a great idea to discuss the design first and improve later.
2) The existing --output
flag is for a different purpose so I think we really need a new flag here. I think I'd keep --json
3) Let's add a new package/directory to hold this. Could be internal/editors
(WIP name, open to ideas)
4) Let's have structs describing the JSON schema we want. Something like this:
struct Output {
Tasks []Task `json:"tasks"`
}
struct Task {
Name string `json:"name"`
Desc string `json:"desc"`
Summary string `json:"summary"`
// "up-to-date" vs. "out-of-date"? Alternatively could be a "UpToDate bool"
Status string `json:"status"`
// These could be added on the future. Don't need to be on the MVP
Env map[string]string `json:"env"`
Vars map[string]string `json:"vars"`
}
5) And then a function to map this:
func ToOutput(tasks/etc) *Output {
return &Output{
// ...
}
}
5) Then on cmd/task/task.go
it's a matter of getting this struct, convert to JSON and print (if --json
was given).
6) Regarding filtering, there's a --filter
flag in discussion: #878. So we shouldn't worry about this now, and in the future we will integration that into this feature as well. For now we can print all tasks.
@davidalpert Does this sound simple enough? If you have further questions and happen to prefer to discuss on Discord, we could have a channel for this as well.
And thanks again for your work on this 🙂
I will get started on this.
I do wonder if it would be useful to print task execution output as JSON also 🤔
I don't think so. With --json
we won't be executing the tasks, just outputting JSON information about the available tasks.
In theory we could also allow --output=json
if that happens to be useful to extension maintainers, but let's push that to the future.
@andreynering here is a second draft; let me know if this is more aligned with your expectations.
3 unit tests are failing though when I printed out more context about each failure I wonder if the expectation that we print nothing to the console is correct.
in one case, for instance, that expectation appears to hide context from the following error:
task.multipleTasksWithAliasError{aliasName:"x", taskNames:[]string{"bar", "foo"}
in another that expectation seems to hide context from this error:
task.taskNotFoundError{taskName:"included:task-3", didYouMean:""}
In both cases we now print
task: No tasks with description available. Try --list-all to list all tasks
instead of an empty string but I'm thinking that those errors contain information which might be useful fo rather than an empty string or the "no tasks with description available" we might prefer something like
that alias was ambiguous; it may refer to the following tasks: "bar", "foo"
or
task "included:task-3" was not found.
or if there are didYouMean
suggestions in the error it could read
task "included:task-3" was not found; did you mean "included:task-300"?
Hi @davidalpert!
Looks great to me! Feel free to ask me a review once it's ready (or close to it) so I can look in depth.
And you're right, improving these error messages would be nice. Feel free to postpone that to another PR if you prefer.
@andreynering ready for review.
I resolved the code suggestions and converted the additional debug info to some enriched error responses which made the tests happy.
An initial version was just merged to master.
Contributions are welcome to add more fields to this output.
When integrating with other tools (like vscode extension, or when trying to script exporting tasks to github actions) we really want to avoid having to parse the free-form output, especially as this will likely change and break integration, even between minor versions.
If we would output as JSON, we can make the parsing far more reliable and less likely to break between versions, especially as if we add extra data, it would not render the original format obsolete.
Things to be included in JSON output: