Improved show output to be valid JSON and added showNamed

[lefou]

Currently, show invoked with more than one target returns invalid JSON, as it is only concatenating multiple JSON values.

Example:

$ mill show __.artifactMetdata
[1/1] show
{
    "group": "com.acme",
    "id": "pkg1",
    "version": "1.0.0"
}
{
    "group": "com.acme",
    "id": "pkg2",
    "version": "1.0.0"
}

This PR makes its output valid JSON by outputting a proper JSON array when there is more than one task result.

Example:

$ mill show __.artifactMetdata
[1/1] show
[
  {
    "group": "com.acme",
    "id": "pkg1",
    "version": "1.0.0"
  },
  {
    "group": "com.acme",
    "id": "pkg2",
    "version": "1.0.0"
  }
]

Also it introduces a new showNamed command, which puts each task result in a JSON dictionary. The keys are reflecting the task names.

Example:

 mill showNamed __.artifactMetadata
[1/1] show
{
  "pkg1.artifactMetadata":
  {
      "group": "com.acme",
      "id": "pkg1",
      "version": "1.0.0"
  },
  "pkg2.artifctMetadata":
  {
      "group": "com.acme",
      "id": "pkg2",
      "version": "1.0.0"
  }
}

This makes the output of show and showNamed much more usable. For humans it's easier to recognize, which task outputted what, and for external tools, as they now can process the mill show output as-is.

Fix #1763

To implement this, I needed to enlarge the result values of some
evaluate methods to also contain the task name, so I added new methods.
These now also transport the actual result value in addition to the watched paths.
I was uncreative and just added a "1" to the existing methods names.
Any suggestions for better names are much appreciated.

I also took the opportunity to let show and showNamed return their result.

[lolgab]

Can you add a test that checks the newly generated JSON structure?

[lolgab]

You could also assign names in the mapand avoid p._1 and p._2

[lihaoyi]

@lefou could you update the PR description to give an example before and after of what the show output looks like?

[lefou]

@lefou could you update the PR description to give an example before and after of what the show output looks like?

Done

My concerns are addressed but didn't review it thoroughly yet.

[daddykotex]

Great work, thanks for resolving this so quickly

[lihaoyi]

I think this looks reasonable to me for the multi-target use case. One thing that worries me is that for running show on a single target, which is probably the majority of use cases, this adds some extra indirection for anyone trying to programmatically fish data out of the result JSON. e.g. if I have a Python script parsing the show output, now I need to remember the target name and plug it back into the JSON dictionary to get the actual data I want, or I need to use json.values()[0] to get the first result. Both are kind of clunky.

If we assume that show is most often called on a single target, with the multi-target use case being less common, could it be worth special-casing the single-target use case to skip the wrapping dictionary? Alternatively, we could leave show as is and introduce a new showAll/showMulti/showNamed command to satisfy the multi-target use case. Either alternative would preserve the conciseness of the common single-target use case while still providing a solution for those calling show on multiple targets at once

[daddykotex]

in my use case, I have no need for the task(s) name in the json, as I said I can on target on multiple module, so I know what's coming out of the json

but if you can send multiple targets like mill show t1 t2 then it might be interesting to disambiguate

personally, I would just run mill show t1 > t1.json and mill show t2.json if i wanted that behaviour

[lefou]

I think this looks reasonable to me for the multi-target use case. One thing that worries me is that for running show on a single target, which is probably the majority of use cases, this adds some extra indirection for anyone trying to programmatically fish data out of the result JSON. e.g. if I have a Python script parsing the show output, now I need to remember the target name and plug it back into the JSON dictionary to get the actual data I want, or I need to use json.values()[0] to get the first result. Both are kind of clunky.

If we assume that show is most often called on a single target, with the multi-target use case being less common, could it be worth special-casing the single-target use case to skip the wrapping dictionary? Alternatively, we could leave show as is and introduce a new showAll/showMulti/showNamed command to satisfy the multi-target use case. Either alternative would preserve the conciseness of the common single-target use case while still providing a solution for those calling show on multiple targets at once

Ok, yeah, looks like I hit a nerve here. (The thing with assumptions is, well, ... Here is some funny pic about someone using your product: https://twitter.com/WdeB/status/1482798979511373828, haha)

I can imagine lots of different ways to address. Yet, I'd like to have a rather small solution, which is also backwards compatible, so here is a compromise:

1. show task behaves as before for single-entry results
2. show task will put the results in a JSON array when there is more than one result
3. a new showNamed task will always print (and return) a JSON dictionary. Task names are the keys, and results as JSON are the values

WDYT?

[lefou]

If none of us already has "some python script" that does parse the output of show, I'd love to have some more stable show output though. Meaning: Always return an array. If you're interested in the first/single task result, just use the first element.

[lihaoyi]

@lefou what you propose sounds good to me. I'd prefer single results appear alone, but I can see the arguments either way.

[lefou]

I now implemented it and update the PR description. I will add it to the documentation later, too. Anyone who has a better suggestion to the new def names?

[lefou]

I'm done with editing, unless you have another objection.

[daddykotex]

Perfect for me