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

Thoughts, post MVP #124

Open
bcoe opened this issue May 23, 2022 · 12 comments
Open

Thoughts, post MVP #124

bcoe opened this issue May 23, 2022 · 12 comments

Comments

@bcoe
Copy link
Collaborator

bcoe commented May 23, 2022

We're close to landing the MVP of parseAargs, I'd like to avoid losing momentum as we've got such a great set of contributors.

Stuff I'd love to do post-MVP

  1. Reporting indices this work proposed by @bakkot would enable complex parsing behavior, without bloating the core implementation (@darcyclarke indicated that they would be able to build a minimal parser on top of this behavior alone, and would love a way to grab just this information from a parse.).
  2. Better documentation, @shadowspawn proposes additional examples in the Node.js docs, but I bet there are other ways we can improve the docs to help with adoption.
  3. The Node codebase itself has argument parsing implement in a few places (see: test-npm-package.js, the test runner that @cjihrig has been working on). We should use the Node codebase itself to test parseArgs.

Once we land the MVP I would love to help enable some other folks contributing to Node core, let me know who's interested.

@bakkot
Copy link
Collaborator

bakkot commented May 23, 2022

I bet there are other ways we can improve the docs to help with adoption.

I think the main thing is to write examples for ~half a dozen of the most common cases. Unfortunately the node docs don't tend to go in for having so many examples, in my experience, so I'm not sure where such docs would live.

@shadowspawn
Copy link
Collaborator

Once we land the MVP I would love to help enable some other folks contributing to Node core, let me know who's interested.

I am not sure I understand this offer, like submitting a future PR with parseArgs changes?

@shadowspawn
Copy link
Collaborator

  1. The Node codebase itself has argument parsing implement in a few places (see: test-npm-package.js

Ah, a compact hand-parser! There is one behaviour parseArgs can't do that is used there. The arguments after the first positional (srcDir) are not further parsed and are returned in testArgs.

I think the behaviour matches up with stopEarly mentioned in the node PR (nodejs/node#42675 (comment)), and yargs-parser has halt-at-non-option, and Commander has passThroughOptions.

@shadowspawn
Copy link
Collaborator

Woop woop! In nightly build.

% n nightly
     copying : nightly/19.0.0-nightly20220525810893f145
   installed : v19.0.0-nightly20220525810893f145 (with npm 8.10.0)
% node -e 'console.log(util.parseArgs({ strict: false }))' -- -ab --foo=bar bongo
{
  values: [Object: null prototype] { a: true, b: true, foo: 'bar' },
  positionals: [ 'bongo' ]
}

@bcoe
Copy link
Collaborator Author

bcoe commented May 25, 2022

I am not sure I understand this offer, like submitting a future PR with parseArgs changes?

It can be somewhat daunting going through the code review process for your first few PRs in the Node codebase, I'm happy to help with the process. My hope is that we can keep some momentum and have some follow up PRs.

I think the main thing is to write examples for ~half a dozen of the most common cases. Unfortunately the node docs don't tend to go in for having so many examples, in my experience, so I'm not sure where such docs would live.

@Trott, @mhdawson, do you have any thoughts about where this type of documentation could live?

I think the behaviour matches up with stopEarly mentioned in the node PR (nodejs/node#42675 (comment)), and yargs-parser has halt-at-non-option, and Commander has passThroughOptions.

@bakkot I'm wondering, with the addition of indices, is there an elegant way we could implement a parser that terminates the parse early on the first positional?

@bakkot
Copy link
Collaborator

bakkot commented May 25, 2022

I think the behaviour matches up with stopEarly mentioned in the node PR (nodejs/node#42675 (comment)), and yargs-parser has halt-at-non-option, and Commander has passThroughOptions.

I was actually just in the process of opening an issue for that.

I'm wondering, with the addition of indices, is there an elegant way we could implement a parser that terminates the parse early on the first positional?

It's doable, but I wouldn't call it elegant. You can parse once with strict: false, find the first positional, slice the input array into stuff before that point and stuff after, and re-parse the first half with strict: true. (I put a full example in the linked issue.)

@mhdawson
Copy link
Member

@Trott, @mhdawson, do you have any thoughts about where this type of documentation could live?

I'm not 100% sure what kind of doc is being referred to. Examples of how to write PRs or example of how to use parse args ?

@shadowspawn
Copy link
Collaborator

shadowspawn commented May 25, 2022

@mhdawson : docs on how to use parseArgs (thanks)

#124 (comment)

@bcoe
Copy link
Collaborator Author

bcoe commented May 26, 2022

@mhdawson : docs on how to use parseArgs (thanks)

@shadowspawn we could update this article:

https://nodejs.org/en/knowledge/command-line/how-to-parse-command-line-arguments/

@mhdawson do we find that folks find these knowledge base articles? what's the process for updating them.

@Trott
Copy link

Trott commented May 26, 2022

@Trott, @mhdawson, do you have any thoughts about where this type of documentation could live?

I would put it in the regular docs. If people complain, then split it out into a separate guide to live on the website, but don't do that preemptively. I think there are a lot of people who feel like our docs don't contain enough examples. The separate guides tend to get ignored and go out of date and so avoiding that and just having it all in the API ref doc is the way to go, IMO.

@mhdawson
Copy link
Member

+1 to what @Trott said, for examples on how to use parseArgs the regular docs make the most sense to me.

@shadowspawn
Copy link
Collaborator

If we need out-of-line examples, I stumbled across this tonight:

Code need not be complete. Treat code blocks as an illustration or aid to your point, not as complete running programs. If a complete running program is necessary, include it as an asset in assets/code-examples and link to it.

https://github.com/nodejs/node/tree/master/doc

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

No branches or pull requests

5 participants