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

Enforce single-hyphen properties, handle boolean array arguments #14

Closed
wants to merge 0 commits into from
Closed

Enforce single-hyphen properties, handle boolean array arguments #14

wants to merge 0 commits into from

Conversation

pacocoursey
Copy link
Contributor

Fixes #4 and #11.

Properties that start with a single hyphen and are longer than one character (like -xyz) now throw an error.

Boolean array arguments now return a count of occurrences, rather than an array of booleans. For example, ./my-program -v -vvv will return '-v': 4.

This also means that ./my-program -v will return '-v': 1. If that's undesired, let me know, and I'll fix it.

@Qix-
Copy link
Contributor

Qix- commented Dec 7, 2018

Nope that's correct behavior as long as non-array Boolean arguments return true or false.

@Qix-
Copy link
Contributor

Qix- commented Dec 9, 2018

@rauchg et al, let's wait for this + #15 before releasing a major version since there are some breaking changes for code that expects an array of Booleans.

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
test.js Outdated Show resolved Hide resolved
@rauchg
Copy link
Member

rauchg commented Dec 10, 2018

I'm +1 on ensuring single - props are only 1 character long.
I'm -1 on the Number-ification of booleans (#11 (comment))

@Qix- Qix- requested review from Qix- and removed request for Qix- December 10, 2018 04:16
@Qix-
Copy link
Contributor

Qix- commented Dec 10, 2018

New changes look good.

Awaiting discussion in #11.

index.js Outdated
if (shortArg in handlers &&
Array.isArray(handlers[shortArg]) &&
rest.length > 0 &&
rest === argName.substring(1, 2).repeat(rest.length)
Copy link
Contributor

@blakeembrey blakeembrey Dec 10, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't appear this PR is designed to support -abc right? Should it instead be iterating character by character until it sees a non-boolean, then treat the rest as the value of the first non-boolean key?

Edit: Feel free to ignore me, I see you're not trying to support it anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as of now -abc would be invalid because single-hyphen flags should be one character.

Expanding -abc to equal -a -b -c isn't a bad idea, but should be opened as another issue.

@Qix-
Copy link
Contributor

Qix- commented Dec 13, 2018

@pacocoursey mind splitting out the enforcement of a single letters and standalone - errors into its own PR so it can be merged asynchronously? :)

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.

Make sure that single-hyphen properties only have one letter
4 participants