add executable arguments to spawn in win32

[hxsf]

In win32, commander just spawn the subcommander in this package's bin scripts.
so, just add the process.execArgv into args is ok.

[shadowspawn]

@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 spawn(process.execPath that it is probably appropriate to add process.execArgv as proposed. This matches the isExplicitJS code in the non-windows code branch.

[hxsf]

the example project: https://gist.github.com/hxsf/80a51ded2817ba2cb8c36f55e9ec77d4

there is a '--harmony' args in both two js file

if run demo-list will get

[demo-list] [ '--harmony' ]

if run demo list (the sub command mode)
will get

[demo] [ '--harmony' ]
[demo-list] []

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.
I think we need to parse the first line of sub command to get the execArgv we need

[shadowspawn]

Great, thanks @hxsf

[shadowspawn]

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.

[shadowspawn]

(I think the Travis failures are because of the old .travis.yml file at the time of the Pull Request?)

[shadowspawn]

so I want to add them back.

but, it is just add the execArgs from main to sub.
I think we need to parse the first line of sub command to get the execArgv we need

Adding back the execArgs is what we do on non-Windows, rather than parse first line (implicitly or explicitly), so this is consistent.

[abetomo]

Thank you!

[shadowspawn]

@abetomo
I am merging into release/3.0.0 rather than master so that there are not breaking changes on master until we are ready to release (in case people pulling from git).

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 release/3.0.0 and marking them as Pending Release once merged. (This means doing merges very manually, and not using Merge button in GitHub.)

[abetomo]

Oh, I'm sorry!

[abetomo]

Revert "add executable arguments to spawn in win32" #981

[shadowspawn]

Something for future, we might add a develop branch and ask people to submit Pull Requests against that to make the Merge button safer! It does mean an extra step for releases as there is a merge from develop to master, but everything can land at once with the version number change and code changes and changelog.

Have you used master+develop in other projects? (This naming is used in GitFlow pattern.)

[abetomo]

There was also a project using develop.
I think it's good.

[shadowspawn]

1. Thanks for review @abetomo ! 😄 I will manually merge from this Pull Request into release/3.0.0.

2. I will add a develop branch and update the CONTRIBUTING guidelines.

3. I did a lot in Commander in the weekend, you would have got a lot of emails! FYI: I added some milestone comments at the end of this issue before closing it: Commander.js v3.0 #666

4. Sorry about the email noise @hxsf. 🙂

[shadowspawn]

Merged master down to release/3.0.0 and reapplied this change after the merge+revert. Will be included in v3 release.

Thank you for your contributions.

[shadowspawn]

Available now as a prerelease. See #1001

[shadowspawn]

Shipped in v3: https://github.com/tj/commander.js/releases/tag/v3.0.0