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 shellPath to allow user specify preferred shell. Fix #10917 #11700

Merged
merged 2 commits into from
Sep 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/vs/vscode.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3497,9 +3497,10 @@ declare namespace vscode {
* [Terminal.show](#Terminal.show) in order to show the terminal panel.
*
* @param name Optional human-readable string which will be used to represent the terminal in the UI.
* @param shellPath Optional path to a custom shell executable to be used in the terminal.
* @return A new Terminal.
*/
export function createTerminal(name?: string): Terminal;
export function createTerminal(name?: string, shellPath?: string): Terminal;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/node/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,8 @@ export class ExtHostAPIImplementation {
createOutputChannel(name: string): vscode.OutputChannel {
return extHostOutputService.createOutputChannel(name);
},
createTerminal(name?: string): vscode.Terminal {
return extHostTerminalService.createTerminal(name);
createTerminal(name?: string, shellPath?: string): vscode.Terminal {
return extHostTerminalService.createTerminal(name, shellPath);
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/api/node/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export abstract class MainThreadOutputServiceShape {
}

export abstract class MainThreadTerminalServiceShape {
$createTerminal(name?: string): TPromise<number> { throw ni(); }
$createTerminal(name?: string, shellPath?: string): TPromise<number> { throw ni(); }
$dispose(terminalId: number): void { throw ni(); }
$hide(terminalId: number): void { throw ni(); }
$sendText(terminalId: number, text: string, addNewLine: boolean): void { throw ni(); }
Expand Down
10 changes: 6 additions & 4 deletions src/vs/workbench/api/node/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,16 +11,18 @@ import {MainContext, MainThreadTerminalServiceShape} from './extHost.protocol';
export class ExtHostTerminal implements vscode.Terminal {

public _name: string;
public _shellPath: string;

private _id: number;
private _proxy: MainThreadTerminalServiceShape;
private _disposed: boolean;
private _queuedRequests: ApiRequest[] = [];

constructor(proxy: MainThreadTerminalServiceShape, id: number, name?: string) {
constructor(proxy: MainThreadTerminalServiceShape, id: number, name?: string, shellPath?: string) {
this._name = name;
this._shellPath = shellPath;
this._proxy = proxy;
this._proxy.$createTerminal(name).then((terminalId) => {
this._proxy.$createTerminal(name, shellPath).then((terminalId) => {
this._id = terminalId;
this._queuedRequests.forEach((r) => {
r.run(this._proxy, this._id);
Expand Down Expand Up @@ -85,8 +87,8 @@ export class ExtHostTerminalService {
this._proxy = threadService.get(MainContext.MainThreadTerminalService);
}

public createTerminal(name?: string): vscode.Terminal {
return new ExtHostTerminal(this._proxy, -1, name);
public createTerminal(name?: string, shellPath?: string): vscode.Terminal {
return new ExtHostTerminal(this._proxy, -1, name, shellPath);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/api/node/mainThreadTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export class MainThreadTerminalService extends MainThreadTerminalServiceShape {
this._terminalService = terminalService;
}

public $createTerminal(name?: string): TPromise<number> {
return this._terminalService.createNew(name);
public $createTerminal(name?: string, shellPath?: string): TPromise<number> {
return this._terminalService.createNew(name, shellPath);
}

public $show(terminalId: number, preserveFocus: boolean): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export interface ITerminalService {

close(): TPromise<any>;
copySelection(): TPromise<any>;
createNew(name?: string): TPromise<number>;
createNew(name?: string, shellPath?: string): TPromise<number>;
focusNext(): TPromise<any>;
focusPrevious(): TPromise<any>;
hide(): TPromise<any>;
Expand Down
40 changes: 28 additions & 12 deletions src/vs/workbench/parts/terminal/electron-browser/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export class TerminalService implements ITerminalService {

private activeTerminalIndex: number = 0;
private terminalProcesses: ITerminalProcess[] = [];
private nextTerminalName: string;
private _nextTerminalProcessConfiguration: ITerminalProcessConfiguration = { name: null, shell: { executable: '', args: [] } };
protected _terminalFocusContextKey: IContextKey<boolean>;

private configHelper: TerminalConfigHelper;
Expand Down Expand Up @@ -180,7 +180,7 @@ export class TerminalService implements ITerminalService {
return this.focus();
}

public createNew(name?: string): TPromise<number> {
public createNew(name?: string, shellPath?: string): TPromise<number> {
let processCount = this.terminalProcesses.length;

// When there are 0 processes it means that the panel is not yet created, so the name needs
Expand All @@ -189,12 +189,22 @@ export class TerminalService implements ITerminalService {
// the TerminalPanel is restored on launch if it was open previously.

if (processCount === 0 && !name) {
name = this.nextTerminalName;
this.nextTerminalName = undefined;
name = this._nextTerminalProcessConfiguration.name;
shellPath = this._nextTerminalProcessConfiguration.shell.executable;
this._nextTerminalProcessConfiguration = undefined;
} else {
this.nextTerminalName = name;
this._nextTerminalProcessConfiguration.name = name;
this._nextTerminalProcessConfiguration.shell.executable = shellPath;
}

// If user doesn't specify a shell, set it to null and let terminalConfigHelper to getShell()
// later.
// Todo: Also let user specify shell args
const terminalProcessConfiguration: ITerminalProcessConfiguration = {
name: name ? name : '',
shell: shellPath ? { executable: shellPath, args: [] } : null
};

return this.focus().then((terminalPanel) => {
// If the terminal panel has not been initialized yet skip this, the terminal will be
// created via a call from TerminalPanel.setVisible
Expand All @@ -210,7 +220,7 @@ export class TerminalService implements ITerminalService {
}

this.initConfigHelper(terminalPanel.getContainer());
return terminalPanel.createNewTerminalInstance(this.createTerminalProcess(name), this._terminalFocusContextKey).then((terminalId) => {
return terminalPanel.createNewTerminalInstance(this.createTerminalProcess(terminalProcessConfiguration), this._terminalFocusContextKey).then((terminalId) => {
this._onInstancesChanged.fire();
return TPromise.as(terminalId);
});
Expand Down Expand Up @@ -291,11 +301,12 @@ export class TerminalService implements ITerminalService {
}
}

private createTerminalProcess(name?: string): ITerminalProcess {
let locale = this.configHelper.isSetLocaleVariables() ? platform.locale : undefined;
let env = TerminalService.createTerminalEnv(process.env, this.configHelper.getShell(), this.contextService.getWorkspace(), locale);
let terminalProcess = {
title: name ? name : '',
private createTerminalProcess(terminalProcessConfiguration: ITerminalProcessConfiguration): ITerminalProcess {
const locale = this.configHelper.isSetLocaleVariables() ? platform.locale : undefined;
const shell = terminalProcessConfiguration.shell ? terminalProcessConfiguration.shell : this.configHelper.getShell();
const env = TerminalService.createTerminalEnv(process.env, shell, this.contextService.getWorkspace(), locale);
const terminalProcess = {
title: terminalProcessConfiguration.name,
process: cp.fork('./terminalProcess', [], {
env: env,
cwd: URI.parse(path.dirname(require.toUrl('./terminalProcess'))).fsPath
Expand All @@ -305,7 +316,7 @@ export class TerminalService implements ITerminalService {
this._onInstancesChanged.fire();
this.activeTerminalIndex = this.terminalProcesses.length - 1;
this._onActiveInstanceChanged.fire();
if (!name) {
if (!terminalProcessConfiguration.name) {
// Only listen for process title changes when a name is not provided
terminalProcess.process.on('message', (message) => {
if (message.type === 'title') {
Expand Down Expand Up @@ -347,4 +358,9 @@ export class TerminalService implements ITerminalService {
}
return parts.join('_') + '.UTF-8';
}
}

export interface ITerminalProcessConfiguration {
name: string;
shell: IShell;
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ suite('Workbench - TerminalService', () => {
assert.ok(env1['ok'], 'Parent environment is copied');
assert.deepStrictEqual(parentEnv1, { ok: true }, 'Parent environment is unchanged');
assert.equal(env1['PTYPID'], process.pid.toString(), 'PTYPID is equal to the current PID');
assert.equal(env1['PTYSHELL'], '/bin/foosh', 'PTYSHELL is equal to the requested shell');
assert.equal(env1['PTYSHELL'], '/bin/foosh', 'PTYSHELL is equal to the provided shell');
assert.equal(env1['PTYSHELLARG0'], '-bar', 'PTYSHELLARG0 is equal to the first shell argument');
assert.equal(env1['PTYSHELLARG1'], 'baz', 'PTYSHELLARG1 is equal to the first shell argument');
assert.ok(!('PTYSHELLARG2' in env1), 'PTYSHELLARG2 is unset');
Expand Down