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

Update debug-win command #6

Closed
wants to merge 1 commit into from

Conversation

ForNeVeR
Copy link
Contributor

After long thoughts I've asked this question on StackOverflow and got the answer.

We have to use PowerShell for this functionality to work properly, but that shouldn't be a problem because today there are no up-to-date Windows systems without PowerShell.

PowerShell itself will create two child processes - one in a job object with PowerShell process itself, and second with derived IO (thanks to -Wait switch).

After I press Ctrl-C, the second process will be killed, and after that PowerShell process will terminate itself (because it was blocked on the killed process), and OS will terminate all processes in the same job as PowerShell (so everyone will be killed, nice).

It will close issue #5 and improve debugging experience on Windows.

@rdner
Copy link
Member

rdner commented May 24, 2015

But what if we don't have PowerShell on the machine? I think we need to save previous approach as the fallback, don't you think so?

@rdner rdner self-assigned this May 24, 2015
@ForNeVeR
Copy link
Contributor Author

What kind of Windows machine today really have no PowerShell? I think everything starting from (and including) Windows XP SP2 have it out of the box.

But if you have any examples of it - then we can leave previous, legacy script as a fallback.

@rdner
Copy link
Member

rdner commented May 24, 2015

I think we should use the universal solution, like

WHERE powershell && IF %ERRORLEVEL% EQ 0 (powershell -Command \"Start-Process -NoNewWindow node ./build.js; Start-Process -NoNewWindow -Wait node ./server.js\") ELSE (start /B node ./build.js && start /B node ./server.js)

I'm not sure it works, I'll check it later (or maybe you could help me).

@rdner
Copy link
Member

rdner commented May 24, 2015

As I know PowerShell was not a part of Windows before Windows 8, am I right?

@ForNeVeR
Copy link
Contributor Author

Well, I don't like that solution, that's terrible mess of cmd and powershell :(

And PowerShell was built in starting from Windows XP SP2 (I'm 100% sure that it is built in Win 7 at least; Wikipedia says it was there from XP SP2).

I think that the best strategy is to make a separate script for legacy software users and do not mix old and new approach.

@rdner
Copy link
Member

rdner commented May 24, 2015

Well, I agree and I don't think we need to support the WinXP users since node.js doesn't in fact.

@rdner
Copy link
Member

rdner commented May 24, 2015

👍

@rdner
Copy link
Member

rdner commented May 24, 2015

Could you re-open this PR with develop as a destination branch?

@ForNeVeR
Copy link
Contributor Author

Regarding nodejs - in fact it does (BTW, we are still talking about io.js; nodejs support for XP wasn't dropped at all); check the comment at the bottom of the page you linked.

Okay, I'll rebase my code now, gimme a minute.

It will create two processes and kill them both just as the bash version.
@ForNeVeR ForNeVeR force-pushed the feature/debug-win branch from 86429fa to 09ab5d2 Compare May 24, 2015 08:27
@ForNeVeR
Copy link
Contributor Author

Rebase done.

@ForNeVeR
Copy link
Contributor Author

Ah, I cannot change the target branch without reopening PR. >_<

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants