Skip to content

Conversation

davidalpert
Copy link
Contributor

resolves #764

in preparation for adding a third list option,
let's first collect the existing two list options
into a struct to avoid expanding parameter lists

this also lets us simplify the multiple ListTask*
functions into a single one which accepts options
and switches internally; this is more consistent
with the tell, don't ask pattern of pushing
implementation below the level where clients care.
@ssbarnea
Copy link
Contributor

ssbarnea commented Oct 5, 2022

Can you please also update the docs to give an example of output file?

If you want I can easily write a JSON Schema specification once I have the example, I happened that I authored quite a few in the recent years.

@davidalpert
Copy link
Contributor Author

davidalpert commented Oct 5, 2022

@ssbarnea let's first validate that this output is what you expect, then I am happy to update the docs.

--list-all --silent --json

Given

go run ./cmd/task/task.go -t ./testdata/env/Taskfile.yml --list-all --silent --json | jq .

I get

[
  "default",
  "global",
  "local"
]

--list-all --json

Given

go run ./cmd/task/task.go -t ./testdata/env/Taskfile.yml --list-all --json | jq .

I get

[
  {
    "task": "default",
    "cmds": [
      {
        "cmd": "",
        "silent": false,
        "task": "local",
        "vars": null,
        "ignore_error": false,
        "defer": false
      },
      {
        "cmd": "",
        "silent": false,
        "task": "global",
        "vars": null,
        "ignore_error": false,
        "defer": false
      }
    ],
    "deps": null,
    "label": "",
    "desc": "",
    "summary": "",
    "sources": null,
    "generates": null,
    "status": null,
    "preconditions": null,
    "dir": "",
    "vars": null,
    "env": null,
    "silent": false,
    "interactive": false,
    "internal": false,
    "method": "",
    "prefix": "",
    "ignore_error": false,
    "run": "",
    "include_vars": null,
    "included_taskfile_vars": null,
    "included_taskfile": null
  },
  {
    "task": "global",
    "cmds": [
      {
        "cmd": "echo \"FOO='$FOO' BAR='$BAR' BAZ='$BAZ'\" > global.txt",
        "silent": false,
        "task": "",
        "vars": null,
        "ignore_error": false,
        "defer": false
      }
    ],
    "deps": null,
    "label": "",
    "desc": "",
    "summary": "",
    "sources": null,
    "generates": null,
    "status": null,
    "preconditions": null,
    "dir": "",
    "vars": null,
    "env": {
      "keys": [
        "BAR"
      ],
      "mapping": {
        "BAR": {
          "static": "overriden",
          "live": null,
          "sh": "",
          "dir": ""
        }
      }
    },
    "silent": false,
    "interactive": false,
    "internal": false,
    "method": "",
    "prefix": "",
    "ignore_error": false,
    "run": "",
    "include_vars": null,
    "included_taskfile_vars": null,
    "included_taskfile": null
  },
  {
    "task": "local",
    "cmds": [
      {
        "cmd": "echo \"GOOS='$GOOS' GOARCH='$GOARCH' CGO_ENABLED='$CGO_ENABLED'\" > local.txt",
        "silent": false,
        "task": "",
        "vars": null,
        "ignore_error": false,
        "defer": false
      }
    ],
    "deps": null,
    "label": "",
    "desc": "",
    "summary": "",
    "sources": null,
    "generates": null,
    "status": null,
    "preconditions": null,
    "dir": "",
    "vars": {
      "keys": [
        "AMD64"
      ],
      "mapping": {
        "AMD64": {
          "static": "amd64",
          "live": null,
          "sh": "",
          "dir": ""
        }
      }
    },
    "env": {
      "keys": [
        "GOOS",
        "GOARCH",
        "CGO_ENABLED"
      ],
      "mapping": {
        "CGO_ENABLED": {
          "static": "",
          "live": null,
          "sh": "echo '0'",
          "dir": ""
        },
        "GOARCH": {
          "static": "{{.AMD64}}",
          "live": null,
          "sh": "",
          "dir": ""
        },
        "GOOS": {
          "static": "linux",
          "live": null,
          "sh": "",
          "dir": ""
        }
      }
    },
    "silent": false,
    "interactive": false,
    "internal": false,
    "method": "",
    "prefix": "",
    "ignore_error": false,
    "run": "",
    "include_vars": null,
    "included_taskfile_vars": null,
    "included_taskfile": null
  }
]

Notes:

  • this PR only supports --json output on the --list and --list-all flags (with and without --silent)
  • this PR does not include task status, as the --list and --list-all flags do not include task status
  • including task status seems to require a larger refactoring or alternative design optimized for the use cases you are trying to support (or a generalized version of them)
  • in order to support JSON output more widely (i.e. for all subcommands) I would recommend introducing an abstraction over output (e.g. in tools like kubectl I have seen a global(ish) --output format flag) something like go-printers which I put together adapting a few ideas I've seen around the community.

@pd93
Copy link
Member

pd93 commented Oct 5, 2022

@davidalpert can we add JSON tags to the Cmd struct so that casing is consistent everywhere - and are we happy with snake_case? The alternative would be camelCase? I'm indifferent, but worth considering now.

@davidalpert
Copy link
Contributor Author

I can add struct tags to Cmd; I must have missed that one.

JSON key formatting is easy to switch up if camelCase is preferred; I tend to default to snake_case but I am also indifferent here.

@davidalpert
Copy link
Contributor Author

updated the sample JSON output

@andreynering
Copy link
Member

Hi @davidalpert, thanks for opening this PR.

I think this will need to be more advanced, though. Instead of just dumping tasks in JSON, I would like to have a separated package to handle this, with handcrafted structs with just the things that are relevant to code editors. It'll need to have variables in its computed state, for example.

Also, we'll probably need to ability to output only for a specific task as well, because always outputting the whole thing may be a performance issue on big Taskfiles, specially when dynamic variables are in place.

Let me know if you would still like to continue working on this. I'm open to give you the direction needed. If you don't want or won't have the time for that, just let us know so someone else will be free to work on it without having a conflict with your work.

@ssbarnea
Copy link
Contributor

Bit of feedback unrelated to proposed implementation:

  • Keep JSON schema inside task project itself (separated means more maintenance work and it will almost always lag behind)
  • Don't try to get it perfect from start, in fact I would suggest an MVP approach, like only outputting the rendered tasks and their names for start. Set additionalProperties=true to allow later extension without breaking the older schema.
  • Avoid over-engineering by trying to build schema dynamically. It might give you the impression that it would be easier but in long run it would be quite costly. Doing it by hand is fine as long you have positive/negative tests to validate it.

Over the last 3 years, I wrote 10+ JSON schemas for various projects, including big ones like Ansible. I made many mistakes, but I think I did learn from them (these bullet points being part of conclusions).

@davidalpert
Copy link
Contributor Author

Hi @andreynering

Thank you for the feedback. I initially picked #764 during Hactoberfest as a way into this code base but I am happy to continue the effort to refine this PR (or rewrite it from the ground up) so that it can align with your expectations. Being new to the project I wasn't sure that this was the right way to approach this request, so I appreciate the guidance.

Specifically, you mentioned:

  • a separate package with handcrafted structs - do you have a package name or a list of what code editors need? shall we take @ssbarnea's suggestion and start with an MVP and then iteratively add more detail? Sorin originally requested this set of details:
    • all available tasks as keys inside a mapping
    • each task's dependencies
    • task description and summary
    • task status (built or not)
  • ability to output for a specific task - can you jot down some notes about the CLI experience you have in mind, in terms of the various combinations of flags and arguments you envision?

Please jot down whatever level of detail you have in mind in terms of what you would like to see this PR turn into. As I mentioned, this was an initial spike into the code for me, so I am happy to let this one go and start over in whatever direction you prefer.

@davidalpert
Copy link
Contributor Author

davidalpert commented Nov 13, 2022

in fact, looking at the code changes in October I would like to close this PR, take the lessons learned and the discussion, and start fresh with your guidance as detailed as you would like to give it.

Let's move the discussion back to #764 to align on the approach.

@andreynering
Copy link
Member

Closing in favor of #936.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add flag to output JSON info to be used by editor extensions, etc
4 participants