pkgjs / parseargs

Polyfill of `util.parseArgs()`
Apache License 2.0
121 stars 9 forks source link

refactor!: rework results to remove redundant `flags` property and store value true for boolean options #83

Closed shadowspawn closed 2 years ago

shadowspawn commented 2 years ago

Leaner, meaner, cleaner!

See #70 for longer description (deleting flags in https://github.com/pkgjs/parseargs/issues/70#issuecomment-1070527258). See #80 for previous proposal to rename flags rather than delete, and this PR built on that.

The big change in this PR is removing the flags property in the results. flags is not encoding any additional information, and is not adding much. The lack of a compelling use case has made renaming it difficult!

In parse results:

In README, a number of minor fixes after reading through examples carefully:

shadowspawn commented 2 years ago

Got a couple of 👍 (thanks) but still looking for an approving review before can merge.

shadowspawn commented 2 years ago

I'm okay with this simplification, if we've thought things through and don't think it will limit any use-cases.

I worked through some core use cases in #38 using various patterns for the results, and didn't find any holes. (At the time the approach used in this PR was just included for interest!)

bcoe commented 2 years ago

@shadowspawn I think this is ready to go IMO, except it appears to have one failing test.

shadowspawn commented 2 years ago

I'll need to add type:* in various tests after the merge.

bcoe commented 2 years ago

I'll need to add type:* in various tests after the merge.

:+1: if you're feeling good about the PR, I can do my best to merge early this week and get our current state synced with the Node.js PR.

shadowspawn commented 2 years ago

Ready, barring more changes landing on main first. 😄