Skip to content
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

Add json format to list command #1155

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alexlafroscia
Copy link

@alexlafroscia alexlafroscia commented Feb 5, 2022

After finding #550 and the work that @musikid did toward resolving it, I decided to take a crack at getting that code working and merged.

Since the work from @musikid was a few years ago, all this PR is (so far) is getting that code into a mergable state and updating the tests so that they pass. At this point...

  • Build completes without errors
  • Tests all pass
  • No clippy warnings

I'm going to iterate on this a bit, but figured I would open a PR so the the work being done is at least visible (and to put a little pressure on myself to actually finish this!)


The current output of the command, in JSON format, looks like this (taken from my own machine, so it reflects what I have installed locally):

{
  "runtimes": [
    {
      "source": "Default",
      "version": "16.13.2"
    }
  ]
},{
  "packages": [
    {
      "Default": {
        "details": {
          "name": "@fsouza/prettierd",
          "version": "0.18.1"
        },
        "node": "16.13.2",
        "tools": [
          "prettierd"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "@lifeart/ember-language-server",
          "version": "2.7.0"
        },
        "node": "16.13.2",
        "tools": [
          "ember-language-server"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "@tailwindcss/language-server",
          "version": "0.0.7"
        },
        "node": "16.13.2",
        "tools": [
          "tailwindcss-language-server"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "eslint_d",
          "version": "11.1.1"
        },
        "node": "16.13.2",
        "tools": [
          "eslint_d"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "svelte-language-server",
          "version": "0.14.18"
        },
        "node": "16.13.2",
        "tools": [
          "svelteserver"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "typescript",
          "version": "4.5.4"
        },
        "node": "16.13.2",
        "tools": [
          "tsc",
          "tsserver"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "typescript-language-server",
          "version": "0.9.4"
        },
        "node": "16.13.2",
        "tools": [
          "typescript-language-server"
        ]
      }
    },
    {
      "Default": {
        "details": {
          "name": "yarn-deduplicate",
          "version": "3.1.0"
        },
        "node": "16.13.2",
        "tools": [
          "yarn-deduplicate"
        ]
      }
    }
  ]
}

I'd like to alter this format somewhat, as there are a few things I think could be improved:

  1. This mirrors the internal representation exactly, which I'm not sure is necessarily desirable. While all of the information you might want to consume is there, the structure isn't particularly self-descriptive; many objects with just a "Default" key in them pointing to a package doesn't really translate into something that's easily understood by whoever is reading the output. What else could be in that location? Why an array of objects with just one key? I think these are questions a user of this format would ask themselves
  2. Returning multiple JSON objects, comma-separated, is going to be tough to parse. I think a single object with multiple top-level keys would make more sense. For example, if you try to pipe this into jq you end up with a parsing error right off the bat, because the comma after the runtimes object is not expected

Closes #550

musikid and others added 2 commits February 5, 2022 10:35
# Conflicts:
#	Cargo.lock
#	Cargo.toml
#	src/command/list/mod.rs
@alexlafroscia alexlafroscia marked this pull request as draft February 5, 2022 15:46
@chriskrycho
Copy link
Contributor

@alexlafroscia still interested in getting this across the line? If not, no worries. If so, I agree that the output should be valid JSON, so should probably be similar in spirit to the human and plain formats—it might warrant a bit of design discussion! Happy to help steward that forward if you want to tackle it, but again, no worries if not. Sorry this sat without reply for so long!

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

Successfully merging this pull request may close these issues.

want volta list --format=json
3 participants