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

Support combined boolean flags #18

Closed
wants to merge 8 commits into from
Closed

Support combined boolean flags #18

wants to merge 8 commits into from

Conversation

pacocoursey
Copy link
Contributor

This pull request builds upon #14 and should be a major bump if merged.

I guess 14 can be closed? Not sure of the best way to build upon an open pull request and also introduce a new feature.

  • Support combining multiple boolean flags into a single flag. Now -abcd will correctly be interpreted as -a -b -c -d.
  • Works in combination with boolean array arguments, such that -abcc will be result in [true, true] if -c was declared as a [Boolean].

Also generally improves the logic so that it will be easier to add support for whatever comes out of #11.

pacocoursey added 6 commits December 6, 2018 16:06
…sure short argument keys are only one character, return boolean array instead of integer in boolean array key types, add tests
This commit adds support for combined boolean flags so that `-abc` will
expand to `-a -b -c`.

It also improves the logic of handling repeating boolean array flags
like `-vvvv`.

Both features now simply split the string into characters, and push them
onto the argv array if they are have valid handlers. This avoids having
more complex logic.
@rauchg
Copy link
Member

rauchg commented Dec 11, 2018

Very nice work @pacocoursey – wrote down some notes

index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
@leo
Copy link
Contributor

leo commented Dec 11, 2018

Awesome PR, thank you! 😊

Would you mind adding Codegov to the repo? Now CLI is a great example for this.

Here's the badge:

[![codecov](https://codecov.io/gh/zeit/arg/branch/master/graph/badge.svg)](https://codecov.io/gh/zeit/arg)

I added the CODECOV_TOKEN environment variable to Circle CI.

You can find the Codecov page here.

@pacocoursey
Copy link
Contributor Author

Having never used Codecov or CircleCI, I feel a little out of my depth. But there's my attempt anyways, based on the configs in ncc and now-cli.

Also added a .npmignore as per #19.

@Qix-
Copy link
Contributor

Qix- commented Dec 12, 2018

In the future can we keep these as separate PRs? It's quickly become a kitchen sink of changes.

Copy link
Contributor

@Qix- Qix- left a comment

Choose a reason for hiding this comment

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

Was the yarn.lock here before?

README.md Show resolved Hide resolved
index.js Show resolved Hide resolved
@Qix-
Copy link
Contributor

Qix- commented Dec 26, 2018

Superseded by #27. Thanks for all of the work, @pacocoursey :)

@Qix- Qix- closed this Dec 26, 2018
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.

4 participants