-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
allow "-" hyphen as an option argument with subcommand #697
Conversation
, should = require('should'); | ||
|
||
program | ||
.command('subcommand') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please set the indent width to 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abetomo I fixed it. Thank you for your review!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thank you PR! |
I have got as far as reproducing the problem. I have not confirmed the fix yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed and tested code and happy. Thanks for adding test too.
There are two issues, both introduced by changes made since Pull Request submitted:
- The test for index being valid has changed on master:
if ((i + 1) < argv.length && argv[i + 1][0] !== '-') {
- There are a number of style changes enforced by eslint (
npm run lint
)
Are you still interested in making these changes @miyajan or should we make them ourselves?
Merging into |
I made some minor changes as per my comments, and already had one approval. Merged into v3 and will be included in that release. Closing to make it clear that should not be merged into master. Thank you for your contributions. |
Available now as a prerelease. See #1001 |
Shipped in v3: https://github.com/tj/commander.js/releases/tag/v3.0.0 |
One character "-" hyphen is not accepted as an option argument with subcommand.
e.x. node test subcommand -a - --bravo - --charlie=-
Main command allowed "-" as an option argument in #139 . This patch will allow it also with subcommand.