Skip to content

Commit

Permalink
Merge pull request #73245 from microsoft/tyriar/r38_default_shell
Browse files Browse the repository at this point in the history
Use default shell from the process side
  • Loading branch information
Tyriar committed May 10, 2019
2 parents 5bc3ac1 + 3a315ea commit 8fe2d38
Show file tree
Hide file tree
Showing 12 changed files with 32 additions and 24 deletions.
3 changes: 2 additions & 1 deletion src/vs/workbench/api/node/extHostTerminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { ExtHostWorkspace } from 'vs/workbench/api/common/extHostWorkspace';
import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace';
import { ExtHostVariableResolverService } from 'vs/workbench/api/node/extHostDebugService';
import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors';
import { getDefaultShell } from 'vs/workbench/contrib/terminal/node/terminal';

const RENDERER_NO_PROCESS_ID = -1;

Expand Down Expand Up @@ -470,7 +471,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
.inspect<string | string[]>(key.substr(key.lastIndexOf('.') + 1));
return this._apiInspectConfigToPlain<string | string[]>(setting);
};
terminalEnvironment.mergeDefaultShellPathAndArgs(shellLaunchConfig, fetchSetting, isWorkspaceShellAllowed || false);
terminalEnvironment.mergeDefaultShellPathAndArgs(shellLaunchConfig, fetchSetting, isWorkspaceShellAllowed || false, getDefaultShell(platform.platform));
}

// Get the initial cwd
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ export class TerminalTaskSystem implements ITaskSystem {
let originalCommand = task.command.name;
if (isShellCommand) {
shellLaunchConfig = { name: terminalName, executable: undefined, args: undefined, waitOnExit };
this.terminalService.configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig, platform);
this.terminalService.configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig, this.terminalService.getDefaultShell(platform), platform);
let shellSpecified: boolean = false;
let shellOptions: ShellConfiguration | undefined = task.command.options && task.command.options.shell;
if (shellOptions) {
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/terminal/browser/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { Terminal as XTermTerminal } from 'vscode-xterm';
import { ITerminalInstance, IWindowsShellHelper, ITerminalProcessManager, ITerminalConfigHelper, ITerminalChildProcess, IShellLaunchConfig } from 'vs/workbench/contrib/terminal/common/terminal';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { IProcessEnvironment } from 'vs/base/common/platform';
import { IProcessEnvironment, Platform } from 'vs/base/common/platform';

export const ITerminalInstanceService = createDecorator<ITerminalInstanceService>('terminalInstanceService');

Expand All @@ -17,6 +17,7 @@ export interface ITerminalInstanceService {
createWindowsShellHelper(shellProcessId: number, instance: ITerminalInstance, xterm: XTermTerminal): IWindowsShellHelper;
createTerminalProcessManager(id: number, configHelper: ITerminalConfigHelper): ITerminalProcessManager;
createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean): ITerminalChildProcess;
getDefaultShell(p: Platform): string;
}

export interface IBrowserTerminalConfigHelper extends ITerminalConfigHelper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,9 +227,9 @@ export class TerminalConfigHelper implements IBrowserTerminalConfigHelper {
return !!isWorkspaceShellAllowed;
}

public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, platformOverride: platform.Platform = platform.platform): void {
public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, platformOverride: platform.Platform = platform.platform): void {
const isWorkspaceShellAllowed = this.checkWorkspaceShellPermissions(platformOverride === platform.Platform.Windows ? platform.OperatingSystem.Windows : (platformOverride === platform.Platform.Mac ? platform.OperatingSystem.Macintosh : platform.OperatingSystem.Linux));
mergeDefaultShellPathAndArgs(shell, (key) => this._workspaceConfigurationService.inspect(key), isWorkspaceShellAllowed, platformOverride);
mergeDefaultShellPathAndArgs(shell, (key) => this._workspaceConfigurationService.inspect(key), isWorkspaceShellAllowed, defaultShell, platformOverride);
}

private _toInteger(source: any, minimum: number, maximum: number, fallback: number): number {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export class TerminalProcessManager implements ITerminalProcessManager {

private _launchProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number): ITerminalChildProcess {
if (!shellLaunchConfig.executable) {
this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig);
this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig, this._terminalInstanceService.getDefaultShell(platform.platform));
}

const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file);
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/terminal/browser/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export abstract class TerminalService extends CommonTerminalService implements I
super(contextKeyService, panelService, lifecycleService, storageService, notificationService, dialogService, extensionService, fileService, remoteAgentService);
}

protected abstract _getDefaultShell(p: platform.Platform): string;
public abstract getDefaultShell(p: platform.Platform): string;

public createInstance(terminalFocusContextKey: IContextKey<boolean>, configHelper: ITerminalConfigHelper, container: HTMLElement | undefined, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance {
const instance = this._instantiationService.createInstance(TerminalInstance, terminalFocusContextKey, configHelper, container, shellLaunchConfig);
Expand Down Expand Up @@ -101,7 +101,7 @@ export abstract class TerminalService extends CommonTerminalService implements I
}

// Never suggest if the setting is non-default already (ie. they set the setting manually)
if (this.configHelper.config.shell.windows !== this._getDefaultShell(platform.Platform.Windows)) {
if (this.configHelper.config.shell.windows !== this.getDefaultShell(platform.Platform.Windows)) {
this._storageService.store(NEVER_SUGGEST_SELECT_WINDOWS_SHELL_STORAGE_KEY, true, StorageScope.GLOBAL);
return;
}
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/terminal/common/terminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export interface ITerminalConfigHelper {
/**
* Merges the default shell path and args into the provided launch configuration
*/
mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, platformOverride?: platform.Platform): void;
mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, platformOverride?: platform.Platform): void;
/** Sets whether a workspace shell configuration is allowed or not */
setWorkspaceShellAllowed(isAllowed: boolean): void;
checkWorkspaceShellPermissions(osOverride?: platform.OperatingSystem): boolean;
Expand Down Expand Up @@ -259,6 +259,7 @@ export interface ITerminalService {
findPrevious(): void;

setContainers(panelContainer: HTMLElement, terminalContainer: HTMLElement): void;
getDefaultShell(p: platform.Platform): string;
selectDefaultWindowsShell(): Promise<string | undefined>;
setWorkspaceShellAllowed(isAllowed: boolean): void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,15 +164,14 @@ export function mergeDefaultShellPathAndArgs(
shell: IShellLaunchConfig,
fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined },
isWorkspaceShellAllowed: boolean,
defaultShell: string,
platformOverride: platform.Platform = platform.platform
): void {
const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux';
const shellConfigValue = fetchSetting(`terminal.integrated.shell.${platformKey}`);
// const shellConfigValue = this._workspaceConfigurationService.inspect<string>(`terminal.integrated.shell.${platformKey}`);
const shellArgsConfigValue = fetchSetting(`terminal.integrated.shellArgs.${platformKey}`);
// const shellArgsConfigValue = this._workspaceConfigurationService.inspect<string[]>(`terminal.integrated.shellArgs.${platformKey}`);

shell.executable = (isWorkspaceShellAllowed ? <string>shellConfigValue.value : <string>shellConfigValue.user) || <string>shellConfigValue.default;
shell.executable = (isWorkspaceShellAllowed ? <string>shellConfigValue.value : <string>shellConfigValue.user) || (<string | null>shellConfigValue.default || defaultShell);
shell.args = (isWorkspaceShellAllowed ? <string[]>shellArgsConfigValue.value : <string[]>shellArgsConfigValue.user) || <string[]>shellArgsConfigValue.default;

// Change Sysnative to System32 if the OS is Windows but NOT WoW64. It's
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/contrib/terminal/common/terminalService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions';
import { IFileService } from 'vs/platform/files/common/files';
import { escapeNonWindowsPath } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
import { isWindows } from 'vs/base/common/platform';
import { isWindows, Platform } from 'vs/base/common/platform';
import { basename } from 'vs/base/common/path';
import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService';
import { timeout } from 'vs/base/common/async';
Expand Down Expand Up @@ -106,6 +106,7 @@ export abstract class TerminalService implements ITerminalService {

public abstract createTerminal(shell?: IShellLaunchConfig, wasNewTerminalAction?: boolean): ITerminalInstance;
public abstract createInstance(terminalFocusContextKey: IContextKey<boolean>, configHelper: ITerminalConfigHelper, container: HTMLElement, shellLaunchConfig: IShellLaunchConfig, doCreateProcess: boolean): ITerminalInstance;
public abstract getDefaultShell(platform: Platform): string;
public abstract selectDefaultWindowsShell(): Promise<string | undefined>;
public abstract setContainers(panelContainer: HTMLElement, terminalContainer: HTMLElement): void;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,19 @@ configurationRegistry.registerConfiguration({
type: 'object',
properties: {
'terminal.integrated.shell.linux': {
markdownDescription: nls.localize('terminal.integrated.shell.linux', "The path of the shell that the terminal uses on Linux. [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration)."),
type: 'string',
default: getDefaultShell(platform.Platform.Linux)
markdownDescription: nls.localize('terminal.integrated.shell.linux', "The path of the shell that the terminal uses on Linux (default: {0}). [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration).", getDefaultShell(platform.Platform.Linux)),
type: ['string', 'null'],
default: null
},
'terminal.integrated.shell.osx': {
markdownDescription: nls.localize('terminal.integrated.shell.osx', "The path of the shell that the terminal uses on macOS. [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration)."),
type: 'string',
default: getDefaultShell(platform.Platform.Mac)
markdownDescription: nls.localize('terminal.integrated.shell.osx', "The path of the shell that the terminal uses on macOS (default: {0}). [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration).", getDefaultShell(platform.Platform.Mac)),
type: ['string', 'null'],
default: null
},
'terminal.integrated.shell.windows': {
markdownDescription: nls.localize('terminal.integrated.shell.windows', "The path of the shell that the terminal uses on Windows. [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration)."),
type: 'string',
default: getDefaultShell(platform.Platform.Windows)
markdownDescription: nls.localize('terminal.integrated.shell.windows', "The path of the shell that the terminal uses on Windows (default: {0}). [Read more about configuring the shell](https://code.visualstudio.com/docs/editor/integrated-terminal#_configuration).", getDefaultShell(platform.Platform.Windows)),
type: ['string', 'null'],
default: null
}
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import { ITerminalInstance, IWindowsShellHelper, ITerminalConfigHelper, ITermina
import { WindowsShellHelper } from 'vs/workbench/contrib/terminal/node/windowsShellHelper';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { TerminalProcessManager } from 'vs/workbench/contrib/terminal/browser/terminalProcessManager';
import { IProcessEnvironment } from 'vs/base/common/platform';
import { IProcessEnvironment, Platform } from 'vs/base/common/platform';
import { TerminalProcess } from 'vs/workbench/contrib/terminal/node/terminalProcess';
import * as typeAheadAddon from 'vs/workbench/contrib/terminal/browser/terminalTypeAheadAddon';
import { getDefaultShell } from 'vs/workbench/contrib/terminal/node/terminal';

let Terminal: typeof XTermTerminal;

Expand Down Expand Up @@ -56,4 +57,8 @@ export class TerminalInstanceService implements ITerminalInstanceService {
public createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean): ITerminalChildProcess {
return new TerminalProcess(shellLaunchConfig, cwd, cols, rows, env, windowsEnableConpty);
}

public getDefaultShell(p: Platform): string {
return getDefaultShell(p);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class TerminalService extends BrowserTerminalService implements ITerminal
});
}

protected _getDefaultShell(p: platform.Platform): string {
public getDefaultShell(p: platform.Platform): string {
return getDefaultShell(p);
}

Expand Down

0 comments on commit 8fe2d38

Please sign in to comment.