diff --git a/src/vs/workbench/api/browser/mainThreadTerminalService.ts b/src/vs/workbench/api/browser/mainThreadTerminalService.ts index ba53e31e444..5bf1387b85d 100644 --- a/src/vs/workbench/api/browser/mainThreadTerminalService.ts +++ b/src/vs/workbench/api/browser/mainThreadTerminalService.ts @@ -224,7 +224,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape cwd: request.shellLaunchConfig.cwd, env: request.shellLaunchConfig.env }; - this._proxy.$createProcess(request.proxy.terminalId, shellLaunchConfigDto, request.activeWorkspaceRootUri, request.cols, request.rows); + this._proxy.$createProcess(request.proxy.terminalId, shellLaunchConfigDto, request.activeWorkspaceRootUri, request.cols, request.rows, request.isWorkspaceShellAllowed); request.proxy.onInput(data => this._proxy.$acceptProcessInput(request.proxy.terminalId, data)); request.proxy.onResize(dimensions => this._proxy.$acceptProcessResize(request.proxy.terminalId, dimensions.cols, dimensions.rows)); request.proxy.onShutdown(immediate => this._proxy.$acceptProcessShutdown(request.proxy.terminalId, immediate)); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 4ca975d247a..012e03a6c00 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1074,7 +1074,7 @@ export interface ExtHostTerminalServiceShape { $acceptTerminalRendererInput(id: number, data: string): void; $acceptTerminalTitleChange(id: number, name: string): void; $acceptTerminalDimensions(id: number, cols: number, rows: number): void; - $createProcess(id: number, shellLaunchConfig: ShellLaunchConfigDto, activeWorkspaceRootUri: UriComponents, cols: number, rows: number): void; + $createProcess(id: number, shellLaunchConfig: ShellLaunchConfigDto, activeWorkspaceRootUri: UriComponents, cols: number, rows: number, isWorkspaceShellAllowed: boolean): void; $acceptProcessInput(id: number, data: string): void; $acceptProcessResize(id: number, cols: number, rows: number): void; $acceptProcessShutdown(id: number, immediate: boolean): void; diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index f6393364e83..fa9b867c2cf 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -436,7 +436,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { } } - public async $createProcess(id: number, shellLaunchConfigDto: ShellLaunchConfigDto, activeWorkspaceRootUriComponents: UriComponents, cols: number, rows: number): Promise { + public async $createProcess(id: number, shellLaunchConfigDto: ShellLaunchConfigDto, activeWorkspaceRootUriComponents: UriComponents, cols: number, rows: number, isWorkspaceShellAllowed: boolean): Promise { const shellLaunchConfig: IShellLaunchConfig = { name: shellLaunchConfigDto.name, executable: shellLaunchConfigDto.executable, @@ -445,31 +445,31 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { env: shellLaunchConfigDto.env }; - // TODO: This function duplicates a lot of TerminalProcessManager.createProcess, ideally - // they would be merged into a single implementation. + // Get cwd const configProvider = await this._extHostConfiguration.getConfigProvider(); const terminalConfig = configProvider.getConfiguration('terminal.integrated'); - - if (!shellLaunchConfig.executable) { - // TODO: This duplicates some of TerminalConfigHelper.mergeDefaultShellPathAndArgs and should be merged - // this._configHelper.mergeDefaultShellPathAndArgs(shellLaunchConfig); - - const platformKey = platform.isWindows ? 'windows' : platform.isMacintosh ? 'osx' : 'linux'; - const shellConfigValue: string | undefined = terminalConfig.get(`shell.${platformKey}`); - const shellArgsConfigValue: string | undefined = terminalConfig.get(`shellArgs.${platformKey}`); - - shellLaunchConfig.executable = shellConfigValue; - shellLaunchConfig.args = shellArgsConfigValue; - } - - // TODO: @daniel const activeWorkspaceRootUri = URI.revive(activeWorkspaceRootUriComponents); const initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, os.homedir(), activeWorkspaceRootUri, terminalConfig.cwd); + // Merge in shell and args from settings + const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); + if (!shellLaunchConfig.executable) { + const fetchSetting = (key: string) => { + const setting = configProvider + .getConfiguration(key.substr(0, key.lastIndexOf('.'))) + .inspect(key.substr(key.lastIndexOf('.') + 1)); + return { + user: setting ? setting.globalValue : undefined, + value: setting ? setting.workspaceValue : undefined, + default: setting ? setting.defaultValue : undefined, + }; + }; + terminalEnvironment.mergeDefaultShellPathAndArgs(shellLaunchConfig, fetchSetting, isWorkspaceShellAllowed || false); + } + // TODO: Pull in and resolve config settings // // Resolve env vars from config and shell // const lastActiveWorkspaceRoot = this._workspaceContextService.getWorkspaceFolder(lastActiveWorkspaceRootUri); - const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux'); // const envFromConfig = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...terminalConfig.env[platformKey] }, lastActiveWorkspaceRoot); const envFromConfig = { ...terminalConfig.env[platformKey] }; // const envFromShell = terminalEnvironment.resolveConfigurationVariables(this._configurationResolverService, { ...shellLaunchConfig.env }, lastActiveWorkspaceRoot); @@ -501,7 +501,6 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { this._terminalProcesses[id] = p; } - public $acceptProcessInput(id: number, data: string): void { this._terminalProcesses[id].input(data); } diff --git a/src/vs/workbench/contrib/terminal/browser/terminalConfigHelper.ts b/src/vs/workbench/contrib/terminal/browser/terminalConfigHelper.ts index 506ceef0bf0..f92c761b159 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalConfigHelper.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalConfigHelper.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import * as nls from 'vs/nls'; -import * as path from 'vs/base/common/path'; import * as platform from 'vs/base/common/platform'; import { EDITOR_FONT_DEFAULTS, IEditorOptions } from 'vs/editor/common/config/editorOptions'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; @@ -14,6 +13,7 @@ import Severity from 'vs/base/common/severity'; import { Terminal as XTermTerminal } from 'vscode-xterm'; import { INotificationService } from 'vs/platform/notification/common/notification'; import { IBrowserTerminalConfigHelper } from 'vs/workbench/contrib/terminal/browser/terminal'; +import { mergeDefaultShellPathAndArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment'; const MINIMUM_FONT_SIZE = 6; const MAXIMUM_FONT_SIZE = 25; @@ -167,9 +167,9 @@ export class TerminalConfigHelper implements IBrowserTerminalConfigHelper { return this._storageService.getBoolean(IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY, StorageScope.WORKSPACE, defaultValue); } - public checkWorkspaceShellPermissions(platformOverride: platform.Platform = platform.platform): boolean { + public checkWorkspaceShellPermissions(osOverride: platform.OperatingSystem = platform.OS): boolean { // Check whether there is a workspace setting - const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux'; + const platformKey = osOverride === platform.OperatingSystem.Windows ? 'windows' : osOverride === platform.OperatingSystem.Macintosh ? 'osx' : 'linux'; const shellConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.shell.${platformKey}`); const shellArgsConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.shellArgs.${platformKey}`); const envConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.env.${platformKey}`); @@ -228,28 +228,8 @@ export class TerminalConfigHelper implements IBrowserTerminalConfigHelper { } public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, platformOverride: platform.Platform = platform.platform): void { - const isWorkspaceShellAllowed = this.checkWorkspaceShellPermissions(platformOverride); - const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux'; - const shellConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.shell.${platformKey}`); - const shellArgsConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.shellArgs.${platformKey}`); - - shell.executable = (isWorkspaceShellAllowed ? shellConfigValue.value : shellConfigValue.user) || shellConfigValue.default; - shell.args = (isWorkspaceShellAllowed ? shellArgsConfigValue.value : shellArgsConfigValue.user) || shellArgsConfigValue.default; - - // Change Sysnative to System32 if the OS is Windows but NOT WoW64. It's - // safe to assume that this was used by accident as Sysnative does not - // exist and will break the terminal in non-WoW64 environments. - if ((platformOverride === platform.Platform.Windows) && !process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432') && process.env.windir) { - const sysnativePath = path.join(process.env.windir, 'Sysnative').toLowerCase(); - if (shell.executable.toLowerCase().indexOf(sysnativePath) === 0) { - shell.executable = path.join(process.env.windir, 'System32', shell.executable.substr(sysnativePath.length)); - } - } - - // Convert / to \ on Windows for convenience - if (platformOverride === platform.Platform.Windows) { - shell.executable = shell.executable.replace(/\//g, '\\'); - } + 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); } private _toInteger(source: any, minimum: number, maximum: number, fallback: number): number { diff --git a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts index d1e9a26ff09..6421801d933 100644 --- a/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts +++ b/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts @@ -133,16 +133,13 @@ export class TerminalProcessManager implements ITerminalProcessManager { } const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(hasRemoteAuthority ? REMOTE_HOST_SCHEME : undefined); - this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows); + this._process = this._instantiationService.createInstance(TerminalProcessExtHostProxy, this._terminalId, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, this._configHelper); } else { this._process = this._launchProcess(shellLaunchConfig, cols, rows); } this.processState = ProcessState.LAUNCHING; - // The process is non-null, but TS isn't clever enough to know - const p = this._process!; - - p.onProcessData(data => { + this._process.onProcessData(data => { const beforeProcessDataEvent: IBeforeProcessDataEvent = { data }; this._onBeforeProcessData.fire(beforeProcessDataEvent); if (beforeProcessDataEvent.data && beforeProcessDataEvent.data.length > 0) { @@ -150,19 +147,19 @@ export class TerminalProcessManager implements ITerminalProcessManager { } }); - p.onProcessIdReady(pid => { + this._process.onProcessIdReady(pid => { this.shellProcessId = pid; this._onProcessReady.fire(); // Send any queued data that's waiting - if (this._preLaunchInputQueue.length > 0) { - p.input(this._preLaunchInputQueue.join('')); + if (this._preLaunchInputQueue.length > 0 && this._process) { + this._process.input(this._preLaunchInputQueue.join('')); this._preLaunchInputQueue.length = 0; } }); - p.onProcessTitleChanged(title => this._onProcessTitle.fire(title)); - p.onProcessExit(exitCode => this._onExit(exitCode)); + this._process.onProcessTitleChanged(title => this._onProcessTitle.fire(title)); + this._process.onProcessExit(exitCode => this._onExit(exitCode)); setTimeout(() => { if (this.processState === ProcessState.LAUNCHING) { diff --git a/src/vs/workbench/contrib/terminal/common/terminal.ts b/src/vs/workbench/contrib/terminal/common/terminal.ts index c1f58a7fc8f..a1932f39345 100644 --- a/src/vs/workbench/contrib/terminal/common/terminal.ts +++ b/src/vs/workbench/contrib/terminal/common/terminal.ts @@ -115,7 +115,7 @@ export interface ITerminalConfigHelper { mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig, platformOverride?: platform.Platform): void; /** Sets whether a workspace shell configuration is allowed or not */ setWorkspaceShellAllowed(isAllowed: boolean): void; - checkWorkspaceShellPermissions(platformOverride?: platform.Platform): boolean; + checkWorkspaceShellPermissions(osOverride?: platform.OperatingSystem): boolean; } export interface ITerminalFont { @@ -268,7 +268,7 @@ export interface ITerminalService { preparePathForTerminalAsync(path: string, executable: string | undefined, title: string): Promise; extHostReady(remoteAuthority: string): void; - requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number): void; + requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number, isWorkspaceShellAllowed: boolean): void; } export const enum Direction { @@ -714,6 +714,7 @@ export interface ITerminalProcessExtHostRequest { activeWorkspaceRootUri: URI; cols: number; rows: number; + isWorkspaceShellAllowed: boolean; } export enum LinuxDistro { diff --git a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts index 5480374bba8..ccf556638f5 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts @@ -144,3 +144,34 @@ export function escapeNonWindowsPath(path: string): string { } return newPath; } + +export function mergeDefaultShellPathAndArgs( + shell: IShellLaunchConfig, + fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined }, + isWorkspaceShellAllowed: boolean, + 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(`terminal.integrated.shell.${platformKey}`); + const shellArgsConfigValue = fetchSetting(`terminal.integrated.shellArgs.${platformKey}`); + // const shellArgsConfigValue = this._workspaceConfigurationService.inspect(`terminal.integrated.shellArgs.${platformKey}`); + + shell.executable = (isWorkspaceShellAllowed ? shellConfigValue.value : shellConfigValue.user) || shellConfigValue.default; + shell.args = (isWorkspaceShellAllowed ? shellArgsConfigValue.value : shellArgsConfigValue.user) || shellArgsConfigValue.default; + + // Change Sysnative to System32 if the OS is Windows but NOT WoW64. It's + // safe to assume that this was used by accident as Sysnative does not + // exist and will break the terminal in non-WoW64 environments. + if ((platformOverride === platform.Platform.Windows) && !process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432') && process.env.windir) { + const sysnativePath = path.join(process.env.windir, 'Sysnative').toLowerCase(); + if (shell.executable && shell.executable.toLowerCase().indexOf(sysnativePath) === 0) { + shell.executable = path.join(process.env.windir, 'System32', shell.executable.substr(sysnativePath.length)); + } + } + + // Convert / to \ on Windows for convenience + if (shell.executable && platformOverride === platform.Platform.Windows) { + shell.executable = shell.executable.replace(/\//g, '\\'); + } +} diff --git a/src/vs/workbench/contrib/terminal/common/terminalProcessExtHostProxy.ts b/src/vs/workbench/contrib/terminal/common/terminalProcessExtHostProxy.ts index b311ce44b90..5ba5923420c 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalProcessExtHostProxy.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalProcessExtHostProxy.ts @@ -4,9 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import { Event, Emitter } from 'vs/base/common/event'; -import { ITerminalService, ITerminalProcessExtHostProxy, IShellLaunchConfig, ITerminalChildProcess } from 'vs/workbench/contrib/terminal/common/terminal'; +import { ITerminalService, ITerminalProcessExtHostProxy, IShellLaunchConfig, ITerminalChildProcess, ITerminalConfigHelper } from 'vs/workbench/contrib/terminal/common/terminal'; import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; +import { IRemoteAgentService } from 'vs/workbench/services/remote/common/remoteAgentService'; export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerminalProcessExtHostProxy { private _disposables: IDisposable[] = []; @@ -43,9 +44,16 @@ export class TerminalProcessExtHostProxy implements ITerminalChildProcess, ITerm activeWorkspaceRootUri: URI, cols: number, rows: number, - @ITerminalService private readonly _terminalService: ITerminalService + configHelper: ITerminalConfigHelper, + @ITerminalService private readonly _terminalService: ITerminalService, + @IRemoteAgentService readonly remoteAgentService: IRemoteAgentService ) { - this._terminalService.requestExtHostProcess(this, shellLaunchConfig, activeWorkspaceRootUri, cols, rows); + remoteAgentService.getEnvironment().then(env => { + if (!env) { + throw new Error('Could not fetch environment'); + } + this._terminalService.requestExtHostProcess(this, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, configHelper.checkWorkspaceShellPermissions(env.os)); + }); setTimeout(() => this._onProcessTitleChanged.fire('Starting...'), 0); } diff --git a/src/vs/workbench/contrib/terminal/common/terminalService.ts b/src/vs/workbench/contrib/terminal/common/terminalService.ts index 1b973709755..2fc857e432b 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalService.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalService.ts @@ -118,7 +118,7 @@ export abstract class TerminalService implements ITerminalService { return activeInstance ? activeInstance : this.createTerminal(undefined, wasNewTerminalAction); } - public requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number): void { + public requestExtHostProcess(proxy: ITerminalProcessExtHostProxy, shellLaunchConfig: IShellLaunchConfig, activeWorkspaceRootUri: URI, cols: number, rows: number, isWorkspaceShellAllowed: boolean): void { this._extensionService.whenInstalledExtensionsRegistered().then(async () => { // Wait for the remoteAuthority to be ready (and listening for events) before proceeding const conn = this._remoteAgentService.getConnection(); @@ -127,7 +127,7 @@ export abstract class TerminalService implements ITerminalService { while (!this._extHostsReady[remoteAuthority] && ++retries < 50) { await timeout(100); } - this._onInstanceRequestExtHostProcess.fire({ proxy, shellLaunchConfig, activeWorkspaceRootUri, cols, rows }); + this._onInstanceRequestExtHostProcess.fire({ proxy, shellLaunchConfig, activeWorkspaceRootUri, cols, rows, isWorkspaceShellAllowed }); }); }