From 1e3cc1a30d4fec3f4744e403e534f84c9b645217 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Thu, 1 Aug 2019 16:04:32 -0700 Subject: [PATCH] Don't fail hard when shell/shellArgs vars aren't resolved Fixes #78307 --- .../api/node/extHostTerminalService.ts | 5 ++-- .../terminal/common/terminalEnvironment.ts | 26 +++++++++++++++---- .../terminalInstanceService.ts | 8 ++++-- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/api/node/extHostTerminalService.ts b/src/vs/workbench/api/node/extHostTerminalService.ts index ef336f561a2..a17a9c0622f 100644 --- a/src/vs/workbench/api/node/extHostTerminalService.ts +++ b/src/vs/workbench/api/node/extHostTerminalService.ts @@ -371,7 +371,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'), process.env.windir, this._lastActiveWorkspace, - this._variableResolver + this._variableResolver, + this._logService ); } @@ -383,7 +384,7 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape { return this._apiInspectConfigToPlain(setting); }; - return terminalEnvironment.getDefaultShellArgs(fetchSetting, this._isWorkspaceShellAllowed, this._lastActiveWorkspace, this._variableResolver); + return terminalEnvironment.getDefaultShellArgs(fetchSetting, this._isWorkspaceShellAllowed, this._lastActiveWorkspace, this._variableResolver, this._logService); } public async resolveTerminalRenderer(id: number): Promise { diff --git a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts index 820506f8fe9..4a75398475f 100644 --- a/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts +++ b/src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts @@ -78,7 +78,11 @@ function resolveConfigurationVariables(configurationResolverService: IConfigurat Object.keys(env).forEach((key) => { const value = env[key]; if (typeof value === 'string' && lastActiveWorkspaceRoot !== null) { - env[key] = configurationResolverService.resolve(lastActiveWorkspaceRoot, value); + try { + env[key] = configurationResolverService.resolve(lastActiveWorkspaceRoot, value); + } catch (e) { + env[key] = value; + } } }); return env; @@ -143,7 +147,7 @@ export function getCwd( // There was an issue resolving a variable, just use the unresolved customCwd which // which will fail, and log the error in the console. if (logService) { - logService.error('Resolving terminal.integrated.cwd', e); + logService.error('Could not resolve terminal.integrated.cwd', e); } return customCwd; } @@ -192,7 +196,8 @@ export function getDefaultShell( windir: string | undefined, lastActiveWorkspace: IWorkspaceFolder | undefined, configurationResolverService: IConfigurationResolverService | undefined, - platformOverride: platform.Platform = platform.platform, + logService: ILogService, + platformOverride: platform.Platform = platform.platform ): string { const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux'; const shellConfigValue = fetchSetting(`terminal.integrated.shell.${platformKey}`); @@ -214,7 +219,12 @@ export function getDefaultShell( } if (configurationResolverService) { - executable = configurationResolverService.resolve(lastActiveWorkspace, executable); + try { + executable = configurationResolverService.resolve(lastActiveWorkspace, executable); + } catch (e) { + logService.error(`Could not resolve terminal.integrated.shell.${platformKey}`, e); + executable = executable; + } } return executable; @@ -225,6 +235,7 @@ export function getDefaultShellArgs( isWorkspaceShellAllowed: boolean, lastActiveWorkspace: IWorkspaceFolder | undefined, configurationResolverService: IConfigurationResolverService | undefined, + logService: ILogService, platformOverride: platform.Platform = platform.platform, ): string | string[] { const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux'; @@ -236,7 +247,12 @@ export function getDefaultShellArgs( if (configurationResolverService) { const resolvedArgs: string[] = []; for (const arg of args) { - resolvedArgs.push(configurationResolverService.resolve(lastActiveWorkspace, arg)); + try { + resolvedArgs.push(configurationResolverService.resolve(lastActiveWorkspace, arg)); + } catch (e) { + logService.error(`Could not resolve terminal.integrated.shellArgs.${platformKey}`, e); + resolvedArgs.push(arg); + } } args = resolvedArgs; } diff --git a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts index e9b13c0d68f..2b7fa1c4fa3 100644 --- a/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts +++ b/src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts @@ -20,6 +20,7 @@ import { getMainProcessParentEnv } from 'vs/workbench/contrib/terminal/node/term import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; import { IHistoryService } from 'vs/workbench/services/history/common/history'; import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace'; +import { ILogService } from 'vs/platform/log/common/log'; let Terminal: typeof XTermTerminal; let WebLinksAddon: typeof XTermWebLinksAddon; @@ -34,7 +35,8 @@ export class TerminalInstanceService implements ITerminalInstanceService { @IStorageService private readonly _storageService: IStorageService, @IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService, @IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService, - @IHistoryService private readonly _historyService: IHistoryService + @IHistoryService private readonly _historyService: IHistoryService, + @ILogService private readonly _logService: ILogService ) { } @@ -84,6 +86,7 @@ export class TerminalInstanceService implements ITerminalInstanceService { process.env.windir, lastActiveWorkspace, this._configurationResolverService, + this._logService, platformOverride ); const args = getDefaultShellArgs( @@ -91,6 +94,7 @@ export class TerminalInstanceService implements ITerminalInstanceService { isWorkspaceShellAllowed, lastActiveWorkspace, this._configurationResolverService, + this._logService, platformOverride ); return Promise.resolve({ shell, args }); @@ -99,4 +103,4 @@ export class TerminalInstanceService implements ITerminalInstanceService { public getMainProcessParentEnv(): Promise { return getMainProcessParentEnv(); } -} \ No newline at end of file +} -- GitLab