-
-
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
add executable arguments to spawn in win32 #611
Conversation
In win32, commander just spawn the subcommander in this package's `bin` scripts. so, just add the `process.execArgv` into `args` is ok.
@hxsf Can you give an example of what this is fixing, or link to some issues that this would fix? I think because the current code is calling |
the example project: https://gist.github.com/hxsf/80a51ded2817ba2cb8c36f55e9ec77d4 there is a '--harmony' args in both two js file if run
if run
we can see, in the sub command mode, the execArgv lost so I want to add them back. but, it is just add the execArgs from main to sub. |
Great, thanks @hxsf |
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.
Light testing with cmd, PowerShell, and git bash.
Because this changes node flags passed to subcommand (if node flags present), I think we will need to wait for a major version to release.
(I think the Travis failures are because of the old |
Adding back the execArgs is what we do on non-Windows, rather than parse first line (implicitly or explicitly), so this is consistent. |
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!
@abetomo I have not tried edited the Pull Request to change the destination branch (not even sure if we can do that), as I was worried might lose review comments if I did that. So I have been manually merging Pull Requests into |
Oh, I'm sorry! |
Revert "add executable arguments to spawn in win32" #981 |
Something for future, we might add a Have you used master+develop in other projects? (This naming is used in GitFlow pattern.) |
There was also a project using develop. |
|
Reapply #611 on release branch
Merged 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 |
In win32, commander just spawn the subcommander in this package's
bin
scripts.so, just add the
process.execArgv
intoargs
is ok.