Open New Command Prompt should honor %COMSPEC% environment variable on Windows

[krizzdewizz]

On Windows, The 'Open New Command Prompt' command should lookup the %comspec% environment variable instead of using hard-coded 'cmd.exe'.
This enables alternate interpreters/shells to be started - such as TCCLE.
There may be similar things for Linux/OSX.

Thanks for this great tool!

[joaomoreno]

Want to contribute a PR?

[krizzdewizz]

Just sent my first PR ever. Hope everything is fine.

[joaomoreno]

Awesome, I'll take a look as soon as possible! Thanks!

[pflannery]

@krizzdewizz @joaomoreno
I have an issue when setting comspec to use bash.
The issue is that the forced arguments passed to spawn affect bash in a different way and closes the process immediately.

Source here -> https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/execution/electron-browser/terminalService.ts#L27
Looking at the code I cant see how this would work for other terminals as it's trying to pass /c start /wait as command arguments which other terminals may or may not support.

I've done some tests and I can get bash to work when supplying cp.spawn('cmd.exe', ['/c', 'start', '/wait', 'bash']).
(Replacing bash with powershell works for too. Also omitting bash leaves cmd.exe to work as default)

This is the test script code I used to get this working for bash.

'use strict';
const cp = require('child_process');
const child = cp.spawn('cmd.exe', ['/c', 'start', '/wait', 'bash'], { cwd: __dirname });
child.on('error', err => {
    console.log("error", err);
});
child.on('exit', code => {
    console.log("terminal closed");
});

I think we should also add the error handling code as it's not present

[pflannery]

I think we can keep the %comspec% environment variable as is and add a new user\workspace setting like "terminal.launchCommand" for spawning new terminals that are requested by the user.