Conversation
bcoe
left a comment
bcoe
left a comment
There was a problem hiding this comment.
This looks like a great simple start. With regards to single dash options -rf, sounds like they just aren't yet implemented?
| 'use strict'; | ||
|
|
||
| const parseArgs = ( | ||
| argv = process.argv.slice(require.main ? 2 : 1), |
There was a problem hiding this comment.
we might want to extend this further to handle environments like electron, I'm not sure of the best approach. Here's what we do in yargs.
| options = {} | ||
| ) => { | ||
| if (typeof options !== 'object' || options === null) { | ||
| throw new Error('Whoops!') |
There was a problem hiding this comment.
perhaps open a tracking ticket to add a clearer type checking message eventually 😆 (if you'd like to land this with Whoops!).
| } | ||
| // look for shortcodes: -fXzy | ||
| else if (arg.charAt(1) !== '-') { | ||
| throw new Error('What are we doing with shortcodes!?!') |
There was a problem hiding this comment.
for this first implementation we're only supporting --foo, --foo bar?
|
|
||
| // Any number of leading dashes are allowed | ||
| // remove all leading dashes | ||
| arg = arg.replace(/^-+/, '') |
There was a problem hiding this comment.
I could do -----hello world? I guess, why not?
| "main": "index.js", | ||
| "scripts": { | ||
| "test": "echo \"Error: no test specified\" && exit 1" | ||
| "test": "node test/index.js" |
There was a problem hiding this comment.
For test coverage, which I think will be helpful for writing a parser, I suggest we try c8 which is what I've got the Node.js using 😄
Also, I think it would be nice to pull in standard or standardx as a linter, I usually do this:
"posttest": "standard",
"fix": "standard --fix"
That way someone can just run npm run fix, to address linting issues.
There was a problem hiding this comment.
let's please just use eslint, regardless of how it's configured; that package's name doesn't need any additional artificial inflation of its standard-ness.
|
I've captured the comments in this PR as issues and will land the PR as is. |
6 participants
Initial work here for review and consideration.
Please review: @bcoe @darcyclarke @ruyadorno @iansu
Thanks!