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

feat(cli): add parse() with descriptive schema #4362

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

Conversation

timreichen
Copy link
Contributor

@timreichen timreichen commented Feb 20, 2024

ref: #4272

This PR adds parse() that takes a descriptive schema for parsing args.
This is inspired by rusts clap, yargs and commanderjs.

This needs more tests for edge cases and some discussions about features like

  • Option.value.accepted?: T[] for allowed values as a shortcut for
    `Option.fn: (value) => { if (!accepted.includes(value)) { throw new Error("...")}
  • Option.value.min?: number for a minimum of arguments taken
  • Option.value.max?: number for a maximum of arguments taken

The type generics also need some rework to imply correct types based on Option.default and Option.type for returned result and inside Option.fn(value: T)
Feedback and suggestions are very welcome. @iuioiua @ngdangtu-vn

@github-actions github-actions bot added the cli label Feb 20, 2024
@iuioiua iuioiua added the feedback welcome We want community's feedback on this issue or PR label Feb 20, 2024
@kt3k
Copy link
Member

kt3k commented Feb 24, 2024

This is the 2nd set of API for parsing cli arguments. It feels confusing to have 2 different sets of APIs for the same purpose.

I'd recommend you develop this tool as a 3rd party tool. If that tool got popularity and adoption in the ecosystem, we would be able to consider adopting it in std

@timreichen
Copy link
Contributor Author

timreichen commented Feb 24, 2024

This is the 2nd set of API for parsing cli arguments. It feels confusing to have 2 different sets of APIs for the same purpose.

I agree, two wouldn't make too much sense, though the one currently implemented is based on minimist and is not capable of some use cases, especially comparing it to other poplar tools like cliffy, nor compatible with such a schema based approach (as pointed out in #4272)

I'd recommend you develop this tool as a 3rd party tool. If that tool got popularity and adoption in the ecosystem, we would be able to consider adopting it in std

There are plenty of popular 3rd party tools out there (commander, yargs, cliffy, etc.) that have this declarative approach implemented and are far more popular than minimist. So I think publishing another 3rd party tool wouldn't make much sense to prove that point.

The declarative approach this implements avoids the function call chains and replaces them with native objects and arrays to make it less framework-ish, but is similar to the named modules.
Example:

...
.option("--foo")
.option("--bar")

=>

{ ...
  options: [
    { name: "foo", },
    { name: "bar", },
  ]
}

@andrewthauer
Copy link
Contributor

Deno is fantastic tool for writing scripts and CLIs in general. I've been using it a lot lately and find that I can get a lot of mileage using just Deno + @std. The only other libraries I typically reach for are dax & cliffy.

For parsing args, the @std parseArgs works in a pinch for very basic usage. However, I don't find the API overly intuitive and quite simplistic for anything but very basic scripts where I don't feel the need to provide a nice DX on top.

The direction @timreichen is taking here would be much more preferred imo then the existing parseArgs. I believe it would help bridge the gap between very simplistic scripts/CLIs and a more advanced CLI which I'd typically reach for cliffy to implement parsing with.

@kt3k
Copy link
Member

kt3k commented Apr 30, 2024

There are plenty of popular 3rd party tools out there (commander, yargs, cliffy, etc.) that have this declarative approach implemented and are far more popular than minimist.

How do you measure the popularity of packages? yargs has 72M weekly downloads, and minimist has 44M. I think these are similarly popular.

Also I don't see what declarative approach means. minimist (and therefore parseArgs of std/cli) accepts the entire parameter all at once as an option object. There's nothing procedural here. I would say minimist also has 'declarative' API.

So I think publishing another 3rd party tool wouldn't make much sense to prove that point.

Without being tested as 3rd party library, how can we be convinced that this design is better than minimist?

@timreichen
Copy link
Contributor Author

timreichen commented May 1, 2024

Sidenote: What I mean by declarative approach

I mean having all declarations in one object structured in a certain way:

Option properties

While minimist declares options in an object, it is done so by splitting the declaration by property.

property->objectName

parseArgs(Deno.args, {
  boolean: ["color"], // type is declared here
  default: { color: true }, // default value is declared here
  negatable: ["color"], // negatable is declared here
});

commanderjs, yargs and friends do it the other way around, where all properties of an option are gathered in one object (or in commanderjs case in one command).

object->properties

parse(Deno.args, {
  options: [
    { name: "color", type: Boolean, default: true, negatable: true } // all option properties declared in one object
  ]
});

Subcommands

As explored here, minimist does not support subcommand parsing and a possible implementation would lead to messy code due to the nature of property->objectName.

Named values

Since there is no option object but a collection of properties declared, it is not possible to have named values for options. This is a problem, when one wants to be able to print help for a cli.

property->objectName

???

object->properties

parse(Deno.args, {
  options: [
    { name: "foo", value: { name: "VALUE" } }
  ],
});

How do you measure the popularity of packages? yargs has 72M weekly downloads, and minimist has 44M. I think these are similarly popular.

Yes, but commanderjs has 141M weekly downloads. So I would say together with yargs, that this kind of approach is much more popular.

Without being tested as 3rd party library, how can we be convinced that this design is better than minimist?

I was talking about the fact that commanderjs and other libs that follow such an implementation are already used extensively and are more popular than minimist (by weekly downloads).
Some functionality like subcommands are apparently not possible with minimist.

@kt3k
Copy link
Member

kt3k commented May 1, 2024

Since there is no option object but a collection of properties declared, it is not possible to have named values for options. This is a problem, when one wants to be able to print help for a cli.

I don't see this argument very well. --help is usually done like the below with the current parseArgs:

import { parseArgs } from "jsr:@std/cli/parse-args";

const args = parseArgs(Deno.args, {
  boolean: ["help"],
});
// This becomes { _: [], help: true }, { _: [], help: false }, etc.

if (args.help) {
  console.log("Usage");
  Deno.exit(0);
}

Isn't { _: [], help: true } named option?

Also I don't see well what { name: "VALUE" } part does in your example..

I was talking about the fact that commanderjs and other libs that follow such an implementation are already used extensively and are more popular than minimist (by weekly downloads).

But this suggested API design isn't similar to commander nor yargs at all. It's completely an invented design, which is not tested anywhere.

@kt3k
Copy link
Member

kt3k commented May 1, 2024

@timreichen BTW what do you think about util.parseArgs of Node.js, which is also available in Deno via node:util module

@timreichen
Copy link
Contributor Author

@timreichen BTW what do you think about util.parseArgs of Node.js, which is also available in Deno via node:util module

I think it is very bare-bone. But I like how one defines the options as an object with properties.

Copy link

codecov bot commented May 13, 2024

Codecov Report

Attention: Patch coverage is 85.64593% with 30 lines in your changes are missing coverage. Please review.

Project coverage is 91.40%. Comparing base (4398642) to head (d16bc74).

Files Patch % Lines
cli/parse.ts 85.64% 29 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4362      +/-   ##
==========================================
- Coverage   91.44%   91.40%   -0.04%     
==========================================
  Files         480      481       +1     
  Lines       37318    37527     +209     
  Branches     5316     5382      +66     
==========================================
+ Hits        34126    34303     +177     
- Misses       3136     3167      +31     
- Partials       56       57       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@denoland denoland deleted a comment May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli feedback welcome We want community's feedback on this issue or PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants