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

add executable arguments to spawn #305

Closed
wants to merge 2 commits into from
Closed

add executable arguments to spawn #305

wants to merge 2 commits into from

Conversation

DigitalIO
Copy link

A slight modification to inject node arguments into sub-commands. Specifically, I made it to allow the --harmony flag to perpetuate to sub-commands.

I'm not quite sure how to write a test for this as no tests exist to test the function as is. See: #290

args = args.slice(1);
args.unshift(local);

// add executable arguments to spawn
for (var i = 0; i < process.execArgv.length; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about
args = process.execArgv.concat(args)

@zhiyelee
Copy link
Collaborator

Yes, #290 have not been solved. You can add start a new test file OR do that after the #290 be solved

@DigitalIO
Copy link
Author

I'll wait for #290 and then implement a test case.

zhiyelee added a commit to zhiyelee/commander.js that referenced this pull request Apr 3, 2015
fix the bug discussed in tj#372
Also merge the `execArg` change in tj#305, thx @DigitalIO
@zhiyelee zhiyelee mentioned this pull request Apr 3, 2015
@zhiyelee zhiyelee closed this Apr 3, 2015
@zhiyelee zhiyelee reopened this Apr 3, 2015
@zhiyelee
Copy link
Collaborator

zhiyelee commented Apr 3, 2015

This PR is involved in #387

@zhiyelee zhiyelee closed this Apr 3, 2015
rrthomas pushed a commit to rrthomas/commander.js that referenced this pull request Apr 8, 2015
fix the bug discussed in tj#372
Also merge the `execArg` change in tj#305, thx @DigitalIO
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.

2 participants