-
-
Notifications
You must be signed in to change notification settings - Fork 597
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 to pass flags to node.js #1195
Conversation
1e9c1ab
to
a0e8036
Compare
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 left some comments. Nice PR! Also, would you mind to add some tests inside the test
folder?
|
||
if (helpFlagExists) { | ||
cli.runHelp(process.argv); | ||
return; | ||
} else if (versionFlagExists) { | ||
cli.runVersion(); | ||
return; | ||
} else if (nodeArgsExists) { | ||
args = cmdArgs(core, { stopAtFirstUnknown: false, partial: true }); | ||
const [, , ...rawArgs] = process.argv; |
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.
Gosh this is lovely! <3
@smelukov Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
There were the following issues with this Pull Request
You may need to change the commit messages to comply with the repository contributing guidelines. 🤖 This comment was generated by commitlint[bot]. Please report issues here. Happy coding! |
c91b06f
to
d979897
Compare
d979897
to
4bfff9c
Compare
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.
Just one last thing, I think. In case we pass a arguement that is not recognised by node and node throws an error, how do we handle that error? I believe we should show a correct error, saying to the user that "too" argument is not a node argument. Basically WHY the CLI blown up.
@ematipico It already works > node cli.js --node-args "-foo=123"
node: bad option: -foo=123 |
2ac6ff9
to
69d6202
Compare
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
Thanks! |
What kind of change does this PR introduce?
feature
Did you add tests for your changes?
yes
If relevant, did you update the documentation?
About
--node-args
optionSummary
closes #1084
closes #289
Does this PR introduce a breaking change?
no