change the way to use spawn

[jdneo]

If the platform is win32 and path contains space, spawn will not execute correctly. For example, if the path of slimerjs is C:\Program Files\slimerjs\sliemrjs.bat. The spawn will treat C:\Program as the command and Files\slimerjs\sliemrjs.bat... as args. The way to solve it is to use \S and \C args of the cmd, which is methioned by @progmars of the following post.

Relate Post: https://github.com/nodejs/node-v0.x-archive/issues/25895

Relate Issue: https://github.com/baudehlo/node-phantom-simple/issues/140

[jdneo]

Hi, is there any one to check about this? Or should I give some additional information?

[baudehlo]

Not many people use Windows.

On Sep 7, 2016, at 9:23 PM, 陈晟 notifications@github.com wrote:

Hi, is there any one to check about this? Or should I give some additional information?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[puzrin]

I don't care much about win, but don't have objections to support it better if someone provides good & complete solution.

1. See code comments.
2. CI should be configured to run tests on win platform (appveyor, for example)
3. It's not clear how are you going to detect phantomjs port after spawn, because current commands are linux-specific

[jdneo]

@puzrin yes, I agree about what you said. This spawn way seems weird and you want to make the code easy enough to be understood. But the issue really exist on wnidows platform, which is mentioned in this post.

Currently I have no idea about configuring CI to run tests on win platform. So I understand the objections to merge. Maybe I'll create another PR after I find a more elegant solution.

@baudehlo what you said is true to some extend. But I dont think it's a right way to refuse the PR. We are all trying to make this tool better and better right?

[puzrin]

@jdneo see https://github.com/eslint/eslint/blob/master/appveyor.yml as example. There is nothing difficult.

@baudehlo what you said is true to some extend. But I dont think it's a right way to refuse the PR. We are all trying to make this tool better and better right?

Any code means additional maintenance cost (developpers time) in future. Bad code -> high cost, good code -> low cost. If you RP a clear code with verified config for appveyour, i see no problems to accept it.

[jdneo]

@puzrin thanks, I'll take a look.

[baudehlo]

@jdneo you misunderstood. I meant not many people can check it because they don't program this stuff on Windows. I certainly can't test it.