From 87e16500d265d393a9e0f8ccd5c3ffcd61103f53 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Jun 2019 12:57:26 -0700 Subject: [PATCH 1/5] Call getDefaultShell via ext host on web Part of #75795 --- .../api/browser/mainThreadTerminalService.ts | 17 +++++++++++++-- .../workbench/api/common/extHost.protocol.ts | 1 + .../api/node/extHostTerminalService.ts | 4 ++++ .../tasks/browser/terminalTaskSystem.ts | 21 +------------------ .../contrib/terminal/browser/terminal.ts | 6 +++++- .../browser/terminalInstanceService.ts | 8 +++++-- .../browser/terminalProcessManager.ts | 2 +- .../terminalInstanceService.ts | 10 +++++---- .../terminalLinkHandler.test.ts | 9 ++++---- 9 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index c37b7c685f0..270f8e332fd 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -9,6 +9,7 @@ import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceS import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { UriComponents, URI } from 'vs/base/common/uri'; import { StopWatch } from 'vs/base/common/stopwatch'; +import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; @extHostNamedCustomer(MainContext.MainThreadTerminalService) export class MainThreadTerminalService implements MainThreadTerminalServiceShape { @@ -22,10 +23,13 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape constructor( extHostContext: IExtHostContext, - @ITerminalService private readonly _terminalService: ITerminalService + @ITerminalService private readonly _terminalService: ITerminalService, + @ITerminalInstanceService readonly terminalInstanceService: ITerminalInstanceService ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostTerminalService); this._remoteAuthority = extHostContext.remoteAuthority; + + // ITerminalService listeners this._toDispose.push(_terminalService.onInstanceCreated((instance) => { // Delay this message so the TerminalInstance constructor has a chance to finish and // return the ID normally to the extension host. The ID that is passed here will be used @@ -44,6 +48,11 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._toDispose.push(_terminalService.configHelper.onWorkspacePermissionsChanged(isAllowed => this._onWorkspacePermissionsChanged(isAllowed))); this._toDispose.push(_terminalService.onRequestAvailableShells(r => this._onRequestAvailableShells(r))); + // ITerminalInstanceService listeners + if (terminalInstanceService.onRequestDefaultShell) { + this._toDispose.push(terminalInstanceService.onRequestDefaultShell(r => this._onRequestDefaultShell(r))); + } + // Set initial ext host state this._terminalService.terminalInstances.forEach(t => { this._onTerminalOpened(t); @@ -278,6 +287,10 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape } private _onRequestAvailableShells(resolve: (shells: IShellDefinition[]) => void): void { - this._proxy.$requestAvailableShells().then(shells => resolve(shells)); + this._proxy.$requestAvailableShells().then(e => resolve(e)); + } + + private _onRequestDefaultShell(resolve: (defaultShell: string) => void): void { + this._proxy.$requestDefaultShell().then(e => resolve(e)); } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index bcd55cea583..f5c00e92830 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1129,6 +1129,7 @@ export interface ExtHostTerminalServiceShape { $acceptProcessRequestLatency(id: number): number; $acceptWorkspacePermissionsChanged(isAllowed: boolean): void; $requestAvailableShells(): Promise; + $requestDefaultShell(): Promise; } export interface ExtHostSCMShape { diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 878127edcc5..9bcfd2d149f 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -579,6 +579,10 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return detectAvailableShells(); } + public $requestDefaultShell(): Promise { + return Promise.resolve(getDefaultShell(platform.platform)); + } + private _onProcessExit(id: number, exitCode: number): void { // Remove listeners this._terminalProcesses[id].dispose(); diff --git a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts index a748fc70f71..6b7d6af46a0 100644 --- a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts @@ -763,25 +763,6 @@ export class TerminalTaskSystem implements ITaskSystem { return nls.localize('TerminalTaskSystem.terminalName', 'Task - {0}', needsFolderQualification ? task.getQualifiedLabel() : task.configurationProperties.name); } - private getDefaultShell(platform: Platform.Platform): string { - let defaultShell: string | undefined = undefined; - try { - defaultShell = this.terminalInstanceService.getDefaultShell(platform); - } catch { - // Do nothing - } - if (!defaultShell) { - // Make up a guess for the default shell. - if (platform === Platform.Platform.Windows) { - defaultShell = 'cmd.exe'; - } else { - defaultShell = 'bash'; - } - console.warn('Cannot get the default shell.'); - } - return defaultShell; - } - private async getUserHome(): Promise { const env = await this.remoteAgentService.getEnvironment(); if (env) { @@ -798,7 +779,7 @@ export class TerminalTaskSystem implements ITaskSystem { let originalCommand = task.command.name; if (isShellCommand) { shellLaunchConfig = { name: terminalName, executable: undefined, args: undefined, waitOnExit }; - this.terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, this.getDefaultShell(platform), this.terminalService.configHelper, platform); + this.terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, await this.terminalInstanceService.getDefaultShell(), this.terminalService.configHelper, platform); let shellSpecified: boolean = false; let shellOptions: ShellConfiguration | undefined = task.command.options && task.command.options.shell; if (shellOptions) { diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index b7648c67e3c..806dcee37d6 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -9,6 +9,7 @@ import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; import { ITerminalInstance, IWindowsShellHelper, ITerminalConfigHelper, ITerminalChildProcess, IShellLaunchConfig } from 'vs/workbench/contrib/terminal/common/terminal'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IProcessEnvironment, Platform } from 'vs/base/common/platform'; +import { Event } from 'vs/base/common/event'; export const ITerminalInstanceService = createDecorator('terminalInstanceService'); @@ -20,16 +21,19 @@ export const ITerminalInstanceService = createDecorator void>; + getXtermConstructor(): Promise; getXtermWebLinksConstructor(): Promise; getXtermSearchConstructor(): Promise; createWindowsShellHelper(shellProcessId: number, instance: ITerminalInstance, xterm: XTermTerminal): IWindowsShellHelper; createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean): ITerminalChildProcess; - getDefaultShell(p: Platform): string; /** * Merges the default shell path and args into the provided launch configuration */ mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride?: Platform): void; + + getDefaultShell(): Promise; getMainProcessParentEnv(): Promise; } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts index dbbb9313e12..d00843b80d6 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts @@ -9,6 +9,7 @@ import { Terminal as XTermTerminal } from 'xterm'; import { WebLinksAddon as XTermWebLinksAddon } from 'xterm-addon-web-links'; import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; import { IProcessEnvironment } from 'vs/base/common/platform'; +import { Emitter, Event } from 'vs/base/common/event'; let Terminal: typeof XTermTerminal; let WebLinksAddon: typeof XTermWebLinksAddon; @@ -17,6 +18,9 @@ let SearchAddon: typeof XTermSearchAddon; export class TerminalInstanceService implements ITerminalInstanceService { public _serviceBrand: any; + private readonly _onRequestDefaultShell = new Emitter<(defaultShell: string) => void>(); + public get onRequestDefaultShell(): Event<(defaultShell: string) => void> { return this._onRequestDefaultShell.event; } + constructor() { } public async getXtermConstructor(): Promise { @@ -48,8 +52,8 @@ export class TerminalInstanceService implements ITerminalInstanceService { throw new Error('Not implemented'); } - public getDefaultShell(): string { - throw new Error('Not implemented'); + public getDefaultShell(): Promise { + return new Promise(r => this._onRequestDefaultShell.fire(r)); } public async getMainProcessParentEnv(): Promise { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 3eca2ccd977..9977b972255 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -163,7 +163,7 @@ export class TerminalProcessManager implements ITerminalProcessManager { private async _launchProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number): Promise { if (!shellLaunchConfig.executable) { - this._terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, this._terminalInstanceService.getDefaultShell(platform.platform), this._configHelper); + this._terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, await this._terminalInstanceService.getDefaultShell(), this._configHelper); } const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file); diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index 357bd5dc736..6326711f628 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -62,10 +62,6 @@ export class TerminalInstanceService implements ITerminalInstanceService { return this._instantiationService.createInstance(TerminalProcess, shellLaunchConfig, cwd, cols, rows, env, windowsEnableConpty); } - public getDefaultShell(p: Platform): string { - return getDefaultShell(p); - } - public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride: Platform = platform): void { const isWorkspaceShellAllowed = configHelper.checkWorkspaceShellPermissions(platformOverride === Platform.Windows ? OperatingSystem.Windows : (platformOverride === Platform.Mac ? OperatingSystem.Macintosh : OperatingSystem.Linux)); mergeDefaultShellPathAndArgs( @@ -79,6 +75,12 @@ export class TerminalInstanceService implements ITerminalInstanceService { ); } + public getDefaultShell(): Promise { + // Don't go via ext host as that would delay terminal start up until after the extension + // host is ready. + return Promise.resolve(getDefaultShell(platform)); + } + public async getMainProcessParentEnv(): Promise { if (this._mainProcessParentEnv) { return this._mainProcessParentEnv; diff --git a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts index d6abc8d9ccf..99d4a6208b7 100644 --- a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as assert from 'assert'; -import { Platform, OperatingSystem } from 'vs/base/common/platform'; +import { OperatingSystem } from 'vs/base/common/platform'; import { TerminalLinkHandler, LineColumnInfo } from 'vs/workbench/contrib/terminal/browser/terminalLinkHandler'; import * as strings from 'vs/base/common/strings'; import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; @@ -30,6 +30,10 @@ class TestXterm { } class MockTerminalInstanceService implements ITerminalInstanceService { + onRequestDefaultShell: any; + getDefaultShell(): Promise { + throw new Error('Method not implemented.'); + } mergeDefaultShellPathAndArgs(): void { throw new Error('Method not implemented.'); } @@ -49,9 +53,6 @@ class MockTerminalInstanceService implements ITerminalInstanceService { createTerminalProcess(): any { throw new Error('Method not implemented.'); } - getDefaultShell(p: Platform): string { - throw new Error('Method not implemented.'); - } getMainProcessParentEnv(): any { throw new Error('Method not implemented.'); } From 0aab08edf27db421b8a1a764e4f30ca4b8629255 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Jun 2019 15:25:44 -0700 Subject: [PATCH 2/5] Add getDefaultShellAndArgs Also differentiate default from system shell --- .../api/browser/mainThreadTerminalService.ts | 17 ++++------- .../workbench/api/common/extHost.protocol.ts | 6 ++++ .../api/node/extHostTerminalService.ts | 29 +++++++++++++++---- .../workbench/contrib/debug/node/terminals.ts | 8 ++--- .../tasks/browser/terminalTaskSystem.ts | 4 +-- .../contrib/terminal/browser/terminal.ts | 4 ++- .../browser/terminalInstanceService.ts | 8 ++++- .../browser/terminalProcessManager.ts | 4 ++- .../contrib/terminal/common/terminal.ts | 4 +++ .../terminal/common/terminalEnvironment.ts | 3 +- .../electron-browser/terminal.contribution.ts | 4 +-- .../terminalInstanceService.ts | 23 +++++++++++++-- .../contrib/terminal/node/terminal.ts | 15 ++++++---- .../terminalLinkHandler.test.ts | 5 ++++ 14 files changed, 98 insertions(+), 36 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index 270f8e332fd..e730222d096 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { IDisposable, dispose } from 'vs/base/common/lifecycle'; -import { ITerminalService, ITerminalInstance, IShellLaunchConfig, ITerminalProcessExtHostProxy, ITerminalProcessExtHostRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY, IShellDefinition } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalService, ITerminalInstance, IShellLaunchConfig, ITerminalProcessExtHostProxy, ITerminalProcessExtHostRequest, ITerminalDimensions, EXT_HOST_CREATION_DELAY } from 'vs/workbench/contrib/terminal/common/terminal'; import { ExtHostContext, ExtHostTerminalServiceShape, MainThreadTerminalServiceShape, MainContext, IExtHostContext, ShellLaunchConfigDto } from 'vs/workbench/api/common/extHost.protocol'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { UriComponents, URI } from 'vs/base/common/uri'; @@ -46,11 +46,14 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._toDispose.push(_terminalService.onActiveInstanceChanged(instance => this._onActiveTerminalChanged(instance ? instance.id : null))); this._toDispose.push(_terminalService.onInstanceTitleChanged(instance => this._onTitleChanged(instance.id, instance.title))); this._toDispose.push(_terminalService.configHelper.onWorkspacePermissionsChanged(isAllowed => this._onWorkspacePermissionsChanged(isAllowed))); - this._toDispose.push(_terminalService.onRequestAvailableShells(r => this._onRequestAvailableShells(r))); + this._toDispose.push(_terminalService.onRequestAvailableShells(r => this._proxy.$requestAvailableShells().then(e => r(e)))); // ITerminalInstanceService listeners if (terminalInstanceService.onRequestDefaultShell) { - this._toDispose.push(terminalInstanceService.onRequestDefaultShell(r => this._onRequestDefaultShell(r))); + this._toDispose.push(terminalInstanceService.onRequestDefaultShell(r => this._proxy.$requestDefaultShell().then(e => r(e)))); + } + if (terminalInstanceService.onRequestDefaultShellAndArgs) { + this._toDispose.push(terminalInstanceService.onRequestDefaultShellAndArgs(r => this._proxy.$requestDefaultShellAndArgs().then(e => r(e.shell, e.args)))); } // Set initial ext host state @@ -285,12 +288,4 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape } this._terminalProcesses[terminalId].emitLatency(sum / COUNT); } - - private _onRequestAvailableShells(resolve: (shells: IShellDefinition[]) => void): void { - this._proxy.$requestAvailableShells().then(e => resolve(e)); - } - - private _onRequestDefaultShell(resolve: (defaultShell: string) => void): void { - this._proxy.$requestDefaultShell().then(e => resolve(e)); - } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index f5c00e92830..e3ceb14440a 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1111,6 +1111,11 @@ export interface IShellDefinitionDto { path: string; } +export interface IShellAndArgsDto { + shell: string; + args: string[] | string | undefined; +} + export interface ExtHostTerminalServiceShape { $acceptTerminalClosed(id: number): void; $acceptTerminalOpened(id: number, name: string): void; @@ -1130,6 +1135,7 @@ export interface ExtHostTerminalServiceShape { $acceptWorkspacePermissionsChanged(isAllowed: boolean): void; $requestAvailableShells(): Promise; $requestDefaultShell(): Promise; + $requestDefaultShellAndArgs(): Promise; } export interface ExtHostSCMShape { diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 9bcfd2d149f..749487c88e1 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -10,7 +10,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import * as platform from 'vs/base/common/platform'; import * as terminalEnvironment from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; import { Event, Emitter } from 'vs/base/common/event'; -import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto, IShellDefinitionDto } from 'vs/workbench/api/common/extHost.protocol'; +import { ExtHostTerminalServiceShape, MainContext, MainThreadTerminalServiceShape, IMainContext, ShellLaunchConfigDto, IShellDefinitionDto, IShellAndArgsDto } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostConfiguration, ExtHostConfigProvider } from 'vs/workbench/api/common/extHostConfiguration'; import { ILogService } from 'vs/platform/log/common/log'; import { EXT_HOST_CREATION_DELAY, IShellLaunchConfig, ITerminalEnvironment } from 'vs/workbench/contrib/terminal/common/terminal'; @@ -20,7 +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, detectAvailableShells } from 'vs/workbench/contrib/terminal/node/terminal'; +import { getSystemShell, detectAvailableShells } from 'vs/workbench/contrib/terminal/node/terminal'; const RENDERER_NO_PROCESS_ID = -1; @@ -338,12 +338,22 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return terminalEnvironment.getDefaultShell( fetchSetting, this._isWorkspaceShellAllowed, - getDefaultShell(platform.platform), + getSystemShell(platform.platform), process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), process.env.windir ); } + private _getDefaultShellArgs(configProvider: ExtHostConfigProvider): string[] | string | undefined { + const fetchSetting = (key: string) => { + const setting = configProvider + .getConfiguration(key.substr(0, key.lastIndexOf('.'))) + .inspect(key.substr(key.lastIndexOf('.') + 1)); + return this._apiInspectConfigToPlain(setting); + }; + return terminalEnvironment.getDefaultShellArgs(fetchSetting, this._isWorkspaceShellAllowed); + } + public async resolveTerminalRenderer(id: number): Promise { // Check to see if the extension host already knows about this terminal. for (const terminalRenderer of this._terminalRenderers) { @@ -495,7 +505,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { shellLaunchConfig, fetchSetting, isWorkspaceShellAllowed || false, - getDefaultShell(platform.platform), + getSystemShell(platform.platform), process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), process.env.windir ); @@ -579,8 +589,17 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return detectAvailableShells(); } + // TODO: Remove this once requestDefaultShellAndArgs is working public $requestDefaultShell(): Promise { - return Promise.resolve(getDefaultShell(platform.platform)); + return Promise.resolve(getSystemShell(platform.platform)); + } + + public async $requestDefaultShellAndArgs(): Promise { + const configProvider = await this._extHostConfiguration.getConfigProvider(); + return Promise.resolve({ + shell: this.getDefaultShell(configProvider), + args: this._getDefaultShellArgs(configProvider) + }); } private _onProcessExit(id: number, exitCode: number): void { diff --git a/src/vs/workbench/contrib/debug/node/terminals.ts b/src/vs/workbench/contrib/debug/node/terminals.ts index 300c738f0eb..164dd27a15a 100644 --- a/src/vs/workbench/contrib/debug/node/terminals.ts +++ b/src/vs/workbench/contrib/debug/node/terminals.ts @@ -10,7 +10,7 @@ import * as pfs from 'vs/base/node/pfs'; import { assign } from 'vs/base/common/objects'; import { ITerminalLauncher, ITerminalSettings } from 'vs/workbench/contrib/debug/common/debug'; import { getPathFromAmdModule } from 'vs/base/common/amd'; -import { getDefaultShell } from 'vs/workbench/contrib/terminal/node/terminal'; +import { getSystemShell } from 'vs/workbench/contrib/terminal/node/terminal'; const TERMINAL_TITLE = nls.localize('console.title', "VS Code Console"); @@ -315,13 +315,13 @@ export function prepareCommand(args: DebugProtocol.RunInTerminalRequestArguments let shell: string; const shell_config = config.integrated.shell; if (env.isWindows) { - shell = shell_config.windows || getDefaultShell(env.Platform.Windows); + shell = shell_config.windows || getSystemShell(env.Platform.Windows); shellType = ShellType.cmd; } else if (env.isLinux) { - shell = shell_config.linux || getDefaultShell(env.Platform.Linux); + shell = shell_config.linux || getSystemShell(env.Platform.Linux); shellType = ShellType.bash; } else if (env.isMacintosh) { - shell = shell_config.osx || getDefaultShell(env.Platform.Mac); + shell = shell_config.osx || getSystemShell(env.Platform.Mac); shellType = ShellType.bash; } else { throw new Error('Unknown platform'); diff --git a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts index 6b7d6af46a0..a481fcc0f69 100644 --- a/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts +++ b/src/vs/workbench/contrib/tasks/browser/terminalTaskSystem.ts @@ -778,8 +778,8 @@ export class TerminalTaskSystem implements ITaskSystem { let terminalName = this.createTerminalName(task); let originalCommand = task.command.name; if (isShellCommand) { - shellLaunchConfig = { name: terminalName, executable: undefined, args: undefined, waitOnExit }; - this.terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, await this.terminalInstanceService.getDefaultShell(), this.terminalService.configHelper, platform); + const defaultConfig = await this.terminalInstanceService.getDefaultShellAndArgs(); + shellLaunchConfig = { name: terminalName, executable: defaultConfig.shell, args: defaultConfig.args, waitOnExit }; let shellSpecified: boolean = false; let shellOptions: ShellConfiguration | undefined = task.command.options && task.command.options.shell; if (shellOptions) { diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 806dcee37d6..bdc6f1e6171 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -6,7 +6,7 @@ import { Terminal as XTermTerminal } from 'xterm'; import { WebLinksAddon as XTermWebLinksAddon } from 'xterm-addon-web-links'; import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; -import { ITerminalInstance, IWindowsShellHelper, ITerminalConfigHelper, ITerminalChildProcess, IShellLaunchConfig } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalInstance, IWindowsShellHelper, ITerminalConfigHelper, ITerminalChildProcess, IShellLaunchConfig, IDefaultShellAndArgsRequest } from 'vs/workbench/contrib/terminal/common/terminal'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { IProcessEnvironment, Platform } from 'vs/base/common/platform'; import { Event } from 'vs/base/common/event'; @@ -22,6 +22,7 @@ export interface ITerminalInstanceService { _serviceBrand: any; onRequestDefaultShell?: Event<(defaultShell: string) => void>; + onRequestDefaultShellAndArgs?: Event; getXtermConstructor(): Promise; getXtermWebLinksConstructor(): Promise; @@ -34,6 +35,7 @@ export interface ITerminalInstanceService { mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride?: Platform): void; getDefaultShell(): Promise; + getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }>; getMainProcessParentEnv(): Promise; } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts index d00843b80d6..1b708b0e2e5 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; -import { IWindowsShellHelper, ITerminalChildProcess } from 'vs/workbench/contrib/terminal/common/terminal'; +import { IWindowsShellHelper, ITerminalChildProcess, IDefaultShellAndArgsRequest } from 'vs/workbench/contrib/terminal/common/terminal'; import { Terminal as XTermTerminal } from 'xterm'; import { WebLinksAddon as XTermWebLinksAddon } from 'xterm-addon-web-links'; import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; @@ -20,6 +20,8 @@ export class TerminalInstanceService implements ITerminalInstanceService { private readonly _onRequestDefaultShell = new Emitter<(defaultShell: string) => void>(); public get onRequestDefaultShell(): Event<(defaultShell: string) => void> { return this._onRequestDefaultShell.event; } + private readonly _onRequestDefaultShellAndArgs = new Emitter(); + public get onRequestDefaultShellAndArgs(): Event { return this._onRequestDefaultShellAndArgs.event; } constructor() { } @@ -56,6 +58,10 @@ export class TerminalInstanceService implements ITerminalInstanceService { return new Promise(r => this._onRequestDefaultShell.fire(r)); } + public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { + return new Promise(r => this._onRequestDefaultShellAndArgs.fire((shell, args) => r({ shell, args }))); + } + public async getMainProcessParentEnv(): Promise { return {}; } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index 9977b972255..723cbe41cd4 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -163,7 +163,9 @@ export class TerminalProcessManager implements ITerminalProcessManager { private async _launchProcess(shellLaunchConfig: IShellLaunchConfig, cols: number, rows: number): Promise { if (!shellLaunchConfig.executable) { - this._terminalInstanceService.mergeDefaultShellPathAndArgs(shellLaunchConfig, await this._terminalInstanceService.getDefaultShell(), this._configHelper); + const defaultConfig = await this._terminalInstanceService.getDefaultShellAndArgs(); + shellLaunchConfig.executable = defaultConfig.shell; + shellLaunchConfig.args = defaultConfig.args; } const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file); diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index 7c7d7824119..2c49b9dff8e 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -789,4 +789,8 @@ export interface ITerminalChildProcess { getInitialCwd(): Promise; getCwd(): Promise; getLatency(): Promise; +} + +export interface IDefaultShellAndArgsRequest { + (shell: string, args: string[] | string | undefined): void; } \ No newline at end of file diff --git a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts index 78a6f0e734f..0ff17ebf440 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts @@ -192,7 +192,7 @@ export function getDefaultShell( return executable; } -function getDefaultShellArgs( +export function getDefaultShellArgs( fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined }, isWorkspaceShellAllowed: boolean, platformOverride: platform.Platform = platform.platform @@ -203,6 +203,7 @@ function getDefaultShellArgs( return args; } +// TODO: Remove this? export function mergeDefaultShellPathAndArgs( shell: IShellLaunchConfig, fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined }, diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts index 9937eea4c5b..f9dd46267ca 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminal.contribution.ts @@ -6,11 +6,11 @@ import { registerSingleton } from 'vs/platform/instantiation/common/extensions'; import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; import { TerminalInstanceService } from 'vs/workbench/contrib/terminal/electron-browser/terminalInstanceService'; -import { getDefaultShell } from 'vs/workbench/contrib/terminal/node/terminal'; +import { getSystemShell } from 'vs/workbench/contrib/terminal/node/terminal'; import { registerShellConfiguration } from 'vs/workbench/contrib/terminal/common/terminalShellConfig'; import { TerminalNativeService } from 'vs/workbench/contrib/terminal/electron-browser/terminalNativeService'; import { ITerminalNativeService } from 'vs/workbench/contrib/terminal/common/terminal'; -registerShellConfiguration(getDefaultShell); +registerShellConfiguration(getSystemShell); registerSingleton(ITerminalNativeService, TerminalNativeService, true); registerSingleton(ITerminalInstanceService, TerminalInstanceService, true); diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index 6326711f628..cc8d96b81d5 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -9,14 +9,14 @@ import { WindowsShellHelper } from 'vs/workbench/contrib/terminal/node/windowsSh import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IProcessEnvironment, Platform, isLinux, isMacintosh, isWindows, OperatingSystem, platform } from 'vs/base/common/platform'; import { TerminalProcess } from 'vs/workbench/contrib/terminal/node/terminalProcess'; -import { getDefaultShell } from 'vs/workbench/contrib/terminal/node/terminal'; +import { getSystemShell } from 'vs/workbench/contrib/terminal/node/terminal'; import { Terminal as XTermTerminal } from 'xterm'; import { WebLinksAddon as XTermWebLinksAddon } from 'xterm-addon-web-links'; import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; import { readFile } from 'vs/base/node/pfs'; import { basename } from 'vs/base/common/path'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { mergeDefaultShellPathAndArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; +import { mergeDefaultShellPathAndArgs, getDefaultShell, getDefaultShellArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; let Terminal: typeof XTermTerminal; let WebLinksAddon: typeof XTermWebLinksAddon; @@ -78,7 +78,24 @@ export class TerminalInstanceService implements ITerminalInstanceService { public getDefaultShell(): Promise { // Don't go via ext host as that would delay terminal start up until after the extension // host is ready. - return Promise.resolve(getDefaultShell(platform)); + return Promise.resolve(getSystemShell(platform)); + } + + public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { + // TODO: Pull the workspace shell permissions setting + const isWorkspaceShellAllowed = false; // configHelper.checkWorkspaceShellPermissions(platform); + const shell = getDefaultShell( + (key) => this._configurationService.inspect(key), + isWorkspaceShellAllowed, + getSystemShell(platform), + process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), + process.env.windir + ); + const args = getDefaultShellArgs( + (key) => this._configurationService.inspect(key), + isWorkspaceShellAllowed + ); + return Promise.resolve({ shell, args }); } public async getMainProcessParentEnv(): Promise { diff --git a/src/vs/workbench/contrib/terminal/node/terminal.ts b/src/vs/workbench/contrib/terminal/node/terminal.ts index 195879eb52f..00cd0d8a33a 100644 --- a/src/vs/workbench/contrib/terminal/node/terminal.ts +++ b/src/vs/workbench/contrib/terminal/node/terminal.ts @@ -11,10 +11,15 @@ import { LinuxDistro, IShellDefinition } from 'vs/workbench/contrib/terminal/com import { coalesce } from 'vs/base/common/arrays'; import { normalize, basename } from 'vs/base/common/path'; -export function getDefaultShell(p: platform.Platform): string { +/** + * Gets the detected default shell for the _system_, not to be confused with VS Code's _default_ + * shell that the terminal uses by default. + * @param p The platform to detect the shell of. + */ +export function getSystemShell(p: platform.Platform): string { if (p === platform.Platform.Windows) { if (platform.isWindows) { - return getTerminalDefaultShellWindows(); + return getSystemShellWindows(); } // Don't detect Windows shell when not on Windows return processes.getWindowsShell(); @@ -23,11 +28,11 @@ export function getDefaultShell(p: platform.Platform): string { if (platform.isLinux && p === platform.Platform.Mac || platform.isMacintosh && p === platform.Platform.Linux) { return '/bin/bash'; } - return getTerminalDefaultShellUnixLike(); + return getSystemShellUnixLike(); } let _TERMINAL_DEFAULT_SHELL_UNIX_LIKE: string | null = null; -function getTerminalDefaultShellUnixLike(): string { +function getSystemShellUnixLike(): string { if (!_TERMINAL_DEFAULT_SHELL_UNIX_LIKE) { let unixLikeTerminal = 'sh'; if (!platform.isWindows && process.env.SHELL) { @@ -46,7 +51,7 @@ function getTerminalDefaultShellUnixLike(): string { } let _TERMINAL_DEFAULT_SHELL_WINDOWS: string | null = null; -function getTerminalDefaultShellWindows(): string { +function getSystemShellWindows(): string { if (!_TERMINAL_DEFAULT_SHELL_WINDOWS) { const isAtLeastWindows10 = platform.isWindows && parseFloat(os.release()) >= 10; const is32ProcessOn64Windows = process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'); diff --git a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts index 99d4a6208b7..1e5162d8f70 100644 --- a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts @@ -8,6 +8,7 @@ import { OperatingSystem } from 'vs/base/common/platform'; import { TerminalLinkHandler, LineColumnInfo } from 'vs/workbench/contrib/terminal/browser/terminalLinkHandler'; import * as strings from 'vs/base/common/strings'; import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { Event } from 'vs/base/common/event'; class TestTerminalLinkHandler extends TerminalLinkHandler { public get localLinkRegex(): RegExp { @@ -30,6 +31,10 @@ class TestXterm { } class MockTerminalInstanceService implements ITerminalInstanceService { + onRequestDefaultShellAndArgs?: Event | undefined; + getDefaultShellAndArgs(): Promise<{ shell: string; args: string | string[] | undefined; }> { + throw new Error('Method not implemented.'); + } onRequestDefaultShell: any; getDefaultShell(): Promise { throw new Error('Method not implemented.'); From 994bfe2d6a7df0cc60867dd9acbd812636469f4b Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Jun 2019 15:31:44 -0700 Subject: [PATCH 3/5] Remove getDefaultShell --- .../api/browser/mainThreadTerminalService.ts | 3 --- src/vs/workbench/api/common/extHost.protocol.ts | 1 - .../workbench/api/node/extHostTerminalService.ts | 5 ----- .../workbench/contrib/terminal/browser/terminal.ts | 2 -- .../terminal/browser/terminalInstanceService.ts | 6 ------ .../contrib/terminal/common/terminalShellConfig.ts | 14 +++++++------- .../electron-browser/terminalInstanceService.ts | 6 ------ .../electron-browser/terminalLinkHandler.test.ts | 4 ---- 8 files changed, 7 insertions(+), 34 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index e730222d096..ff3183d0712 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -49,9 +49,6 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape this._toDispose.push(_terminalService.onRequestAvailableShells(r => this._proxy.$requestAvailableShells().then(e => r(e)))); // ITerminalInstanceService listeners - if (terminalInstanceService.onRequestDefaultShell) { - this._toDispose.push(terminalInstanceService.onRequestDefaultShell(r => this._proxy.$requestDefaultShell().then(e => r(e)))); - } if (terminalInstanceService.onRequestDefaultShellAndArgs) { this._toDispose.push(terminalInstanceService.onRequestDefaultShellAndArgs(r => this._proxy.$requestDefaultShellAndArgs().then(e => r(e.shell, e.args)))); } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index e3ceb14440a..9d7bb48a9ca 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1134,7 +1134,6 @@ export interface ExtHostTerminalServiceShape { $acceptProcessRequestLatency(id: number): number; $acceptWorkspacePermissionsChanged(isAllowed: boolean): void; $requestAvailableShells(): Promise; - $requestDefaultShell(): Promise; $requestDefaultShellAndArgs(): Promise; } diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 749487c88e1..5a2aff7018d 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -589,11 +589,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return detectAvailableShells(); } - // TODO: Remove this once requestDefaultShellAndArgs is working - public $requestDefaultShell(): Promise { - return Promise.resolve(getSystemShell(platform.platform)); - } - public async $requestDefaultShellAndArgs(): Promise { const configProvider = await this._extHostConfiguration.getConfigProvider(); return Promise.resolve({ diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index bdc6f1e6171..36b30c70160 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -21,7 +21,6 @@ export const ITerminalInstanceService = createDecorator void>; onRequestDefaultShellAndArgs?: Event; getXtermConstructor(): Promise; @@ -34,7 +33,6 @@ export interface ITerminalInstanceService { */ mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride?: Platform): void; - getDefaultShell(): Promise; getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }>; getMainProcessParentEnv(): Promise; } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts index 1b708b0e2e5..099aaf41c5e 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts @@ -18,8 +18,6 @@ let SearchAddon: typeof XTermSearchAddon; export class TerminalInstanceService implements ITerminalInstanceService { public _serviceBrand: any; - private readonly _onRequestDefaultShell = new Emitter<(defaultShell: string) => void>(); - public get onRequestDefaultShell(): Event<(defaultShell: string) => void> { return this._onRequestDefaultShell.event; } private readonly _onRequestDefaultShellAndArgs = new Emitter(); public get onRequestDefaultShellAndArgs(): Event { return this._onRequestDefaultShellAndArgs.event; } @@ -54,10 +52,6 @@ export class TerminalInstanceService implements ITerminalInstanceService { throw new Error('Not implemented'); } - public getDefaultShell(): Promise { - return new Promise(r => this._onRequestDefaultShell.fire(r)); - } - public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { return new Promise(r => this._onRequestDefaultShellAndArgs.fire((shell, args) => r({ shell, args }))); } diff --git a/src/vs/workbench/contrib/terminal/common/terminalShellConfig.ts b/src/vs/workbench/contrib/terminal/common/terminalShellConfig.ts index 6867fd01871..c08a6f47fe8 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalShellConfig.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalShellConfig.ts @@ -8,7 +8,7 @@ import { Extensions, IConfigurationRegistry } from 'vs/platform/configuration/co import { Registry } from 'vs/platform/registry/common/platform'; import { Platform } from 'vs/base/common/platform'; -export function registerShellConfiguration(getDefaultShell?: (p: Platform) => string): void { +export function registerShellConfiguration(getSystemShell?: (p: Platform) => string): void { const configurationRegistry = Registry.as(Extensions.Configuration); configurationRegistry.registerConfiguration({ id: 'terminal', @@ -18,24 +18,24 @@ export function registerShellConfiguration(getDefaultShell?: (p: Platform) => st properties: { 'terminal.integrated.shell.linux': { markdownDescription: - getDefaultShell - ? 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.Linux)) + getSystemShell + ? 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).", getSystemShell(Platform.Linux)) : nls.localize('terminal.integrated.shell.linux.noDefault', "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', 'null'], default: null }, 'terminal.integrated.shell.osx': { markdownDescription: - getDefaultShell - ? 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.Mac)) + getSystemShell + ? 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).", getSystemShell(Platform.Mac)) : nls.localize('terminal.integrated.shell.osx.noDefault', "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', 'null'], default: null }, 'terminal.integrated.shell.windows': { markdownDescription: - getDefaultShell - ? 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.Windows)) + getSystemShell + ? 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).", getSystemShell(Platform.Windows)) : nls.localize('terminal.integrated.shell.windows.noDefault', "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', 'null'], default: null diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index cc8d96b81d5..912580984b0 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -75,12 +75,6 @@ export class TerminalInstanceService implements ITerminalInstanceService { ); } - public getDefaultShell(): Promise { - // Don't go via ext host as that would delay terminal start up until after the extension - // host is ready. - return Promise.resolve(getSystemShell(platform)); - } - public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { // TODO: Pull the workspace shell permissions setting const isWorkspaceShellAllowed = false; // configHelper.checkWorkspaceShellPermissions(platform); diff --git a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts index 1e5162d8f70..e5b3f5d7f3c 100644 --- a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts @@ -35,10 +35,6 @@ class MockTerminalInstanceService implements ITerminalInstanceService { getDefaultShellAndArgs(): Promise<{ shell: string; args: string | string[] | undefined; }> { throw new Error('Method not implemented.'); } - onRequestDefaultShell: any; - getDefaultShell(): Promise { - throw new Error('Method not implemented.'); - } mergeDefaultShellPathAndArgs(): void { throw new Error('Method not implemented.'); } From 12afb8de03afa885d1b2ce29eceacfdbf850858a Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Jun 2019 15:37:52 -0700 Subject: [PATCH 4/5] Get workspace shell permissions correctly --- .../electron-browser/terminalInstanceService.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index 912580984b0..cfa3ec68b4d 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; -import { ITerminalInstance, IWindowsShellHelper, IShellLaunchConfig, ITerminalChildProcess, ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalInstance, IWindowsShellHelper, IShellLaunchConfig, ITerminalChildProcess, ITerminalConfigHelper, IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY } from 'vs/workbench/contrib/terminal/common/terminal'; import { WindowsShellHelper } from 'vs/workbench/contrib/terminal/node/windowsShellHelper'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IProcessEnvironment, Platform, isLinux, isMacintosh, isWindows, OperatingSystem, platform } from 'vs/base/common/platform'; @@ -17,6 +17,7 @@ import { readFile } from 'vs/base/node/pfs'; import { basename } from 'vs/base/common/path'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { mergeDefaultShellPathAndArgs, getDefaultShell, getDefaultShellArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; +import { StorageScope, IStorageService } from 'vs/platform/storage/common/storage'; let Terminal: typeof XTermTerminal; let WebLinksAddon: typeof XTermWebLinksAddon; @@ -29,7 +30,8 @@ export class TerminalInstanceService implements ITerminalInstanceService { constructor( @IInstantiationService private readonly _instantiationService: IInstantiationService, - @IConfigurationService private readonly _configurationService: IConfigurationService + @IConfigurationService private readonly _configurationService: IConfigurationService, + @IStorageService private readonly _storageService: IStorageService ) { } @@ -62,6 +64,10 @@ export class TerminalInstanceService implements ITerminalInstanceService { return this._instantiationService.createInstance(TerminalProcess, shellLaunchConfig, cwd, cols, rows, env, windowsEnableConpty); } + private _isWorkspaceShellAllowed(): boolean { + return this._storageService.getBoolean(IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY, StorageScope.WORKSPACE, false); + } + public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride: Platform = platform): void { const isWorkspaceShellAllowed = configHelper.checkWorkspaceShellPermissions(platformOverride === Platform.Windows ? OperatingSystem.Windows : (platformOverride === Platform.Mac ? OperatingSystem.Macintosh : OperatingSystem.Linux)); mergeDefaultShellPathAndArgs( @@ -76,8 +82,7 @@ export class TerminalInstanceService implements ITerminalInstanceService { } public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { - // TODO: Pull the workspace shell permissions setting - const isWorkspaceShellAllowed = false; // configHelper.checkWorkspaceShellPermissions(platform); + const isWorkspaceShellAllowed = this._isWorkspaceShellAllowed(); const shell = getDefaultShell( (key) => this._configurationService.inspect(key), isWorkspaceShellAllowed, From 7c127de60cb45884d5e925548360cbed84e97fff Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 19 Jun 2019 15:42:15 -0700 Subject: [PATCH 5/5] Remove mergeDefaultShellAndArgs --- .../api/node/extHostTerminalService.ts | 16 ++-------------- .../contrib/terminal/browser/terminal.ts | 6 +----- .../browser/terminalInstanceService.ts | 3 --- .../terminal/common/terminalEnvironment.ts | 14 -------------- .../terminalInstanceService.ts | 19 +++---------------- .../terminalLinkHandler.test.ts | 3 --- 6 files changed, 6 insertions(+), 55 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index 5a2aff7018d..e935ccdf20d 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -495,20 +495,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); const configProvider = await this._extHostConfiguration.getConfigProvider(); if (!shellLaunchConfig.executable) { - const fetchSetting = (key: string) => { - const setting = configProvider - .getConfiguration(key.substr(0, key.lastIndexOf('.'))) - .inspect(key.substr(key.lastIndexOf('.') + 1)); - return this._apiInspectConfigToPlain(setting); - }; - terminalEnvironment.mergeDefaultShellPathAndArgs( - shellLaunchConfig, - fetchSetting, - isWorkspaceShellAllowed || false, - getSystemShell(platform.platform), - process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), - process.env.windir - ); + shellLaunchConfig.executable = this.getDefaultShell(configProvider); + shellLaunchConfig.args = this._getDefaultShellArgs(configProvider); } // Get the initial cwd diff --git a/src/vs/workbench/contrib/terminal/browser/terminal.ts b/src/vs/workbench/contrib/terminal/browser/terminal.ts index 36b30c70160..e67c5dba924 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminal.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminal.ts @@ -8,7 +8,7 @@ import { WebLinksAddon as XTermWebLinksAddon } from 'xterm-addon-web-links'; import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; import { ITerminalInstance, IWindowsShellHelper, ITerminalConfigHelper, ITerminalChildProcess, IShellLaunchConfig, IDefaultShellAndArgsRequest } from 'vs/workbench/contrib/terminal/common/terminal'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; -import { IProcessEnvironment, Platform } from 'vs/base/common/platform'; +import { IProcessEnvironment } from 'vs/base/common/platform'; import { Event } from 'vs/base/common/event'; export const ITerminalInstanceService = createDecorator('terminalInstanceService'); @@ -28,10 +28,6 @@ export interface ITerminalInstanceService { getXtermSearchConstructor(): Promise; createWindowsShellHelper(shellProcessId: number, instance: ITerminalInstance, xterm: XTermTerminal): IWindowsShellHelper; createTerminalProcess(shellLaunchConfig: IShellLaunchConfig, cwd: string, cols: number, rows: number, env: IProcessEnvironment, windowsEnableConpty: boolean): ITerminalChildProcess; - /** - * Merges the default shell path and args into the provided launch configuration - */ - mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride?: Platform): void; getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }>; getMainProcessParentEnv(): Promise; diff --git a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts index 099aaf41c5e..08980a94000 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalInstanceService.ts @@ -59,7 +59,4 @@ export class TerminalInstanceService implements ITerminalInstanceService { public async getMainProcessParentEnv(): Promise { return {}; } - - public mergeDefaultShellPathAndArgs(): void { - } } \ No newline at end of file diff --git a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts index 0ff17ebf440..7309615c7fd 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts @@ -203,20 +203,6 @@ export function getDefaultShellArgs( return args; } -// TODO: Remove this? -export function mergeDefaultShellPathAndArgs( - shell: IShellLaunchConfig, - fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined }, - isWorkspaceShellAllowed: boolean, - defaultShell: string, - isWoW64: boolean, - windir: string | undefined, - platformOverride: platform.Platform = platform.platform -): void { - shell.executable = getDefaultShell(fetchSetting, isWorkspaceShellAllowed, defaultShell, isWoW64, windir, platformOverride); - shell.args = getDefaultShellArgs(fetchSetting, isWorkspaceShellAllowed, platformOverride); -} - export function createTerminalEnvironment( shellLaunchConfig: IShellLaunchConfig, lastActiveWorkspace: IWorkspaceFolder | null, diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index cfa3ec68b4d..7b3a08d1978 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -4,10 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import { ITerminalInstanceService } from 'vs/workbench/contrib/terminal/browser/terminal'; -import { ITerminalInstance, IWindowsShellHelper, IShellLaunchConfig, ITerminalChildProcess, ITerminalConfigHelper, IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalInstance, IWindowsShellHelper, IShellLaunchConfig, ITerminalChildProcess, IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY } from 'vs/workbench/contrib/terminal/common/terminal'; import { WindowsShellHelper } from 'vs/workbench/contrib/terminal/node/windowsShellHelper'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { IProcessEnvironment, Platform, isLinux, isMacintosh, isWindows, OperatingSystem, platform } from 'vs/base/common/platform'; +import { IProcessEnvironment, isLinux, isMacintosh, isWindows, platform } from 'vs/base/common/platform'; import { TerminalProcess } from 'vs/workbench/contrib/terminal/node/terminalProcess'; import { getSystemShell } from 'vs/workbench/contrib/terminal/node/terminal'; import { Terminal as XTermTerminal } from 'xterm'; @@ -16,7 +16,7 @@ import { SearchAddon as XTermSearchAddon } from 'xterm-addon-search'; import { readFile } from 'vs/base/node/pfs'; import { basename } from 'vs/base/common/path'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { mergeDefaultShellPathAndArgs, getDefaultShell, getDefaultShellArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; +import { getDefaultShell, getDefaultShellArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; import { StorageScope, IStorageService } from 'vs/platform/storage/common/storage'; let Terminal: typeof XTermTerminal; @@ -68,19 +68,6 @@ export class TerminalInstanceService implements ITerminalInstanceService { return this._storageService.getBoolean(IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY, StorageScope.WORKSPACE, false); } - public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, defaultShell: string, configHelper: ITerminalConfigHelper, platformOverride: Platform = platform): void { - const isWorkspaceShellAllowed = configHelper.checkWorkspaceShellPermissions(platformOverride === Platform.Windows ? OperatingSystem.Windows : (platformOverride === Platform.Mac ? OperatingSystem.Macintosh : OperatingSystem.Linux)); - mergeDefaultShellPathAndArgs( - shell, - (key) => this._configurationService.inspect(key), - isWorkspaceShellAllowed, - defaultShell, - process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), - process.env.windir, - platformOverride - ); - } - public getDefaultShellAndArgs(): Promise<{ shell: string, args: string[] | string | undefined }> { const isWorkspaceShellAllowed = this._isWorkspaceShellAllowed(); const shell = getDefaultShell( diff --git a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts index e5b3f5d7f3c..96daea4ad44 100644 --- a/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts +++ b/src/vs/workbench/contrib/terminal/test/electron-browser/terminalLinkHandler.test.ts @@ -35,9 +35,6 @@ class MockTerminalInstanceService implements ITerminalInstanceService { getDefaultShellAndArgs(): Promise<{ shell: string; args: string | string[] | undefined; }> { throw new Error('Method not implemented.'); } - mergeDefaultShellPathAndArgs(): void { - throw new Error('Method not implemented.'); - } _serviceBrand: any; getXtermConstructor(): Promise { throw new Error('Method not implemented.');