Skip to content

Commit 0a8ca63

Browse files
lramos15lramos15
authored
Resolve Terminal arguments (microsoft#76895)
* Resolved terminal argds * Move location of resolution * Fix routing of resolver in extHost * Remove unnecessary comma * Compilation errors * some async stuff * Undo changes * Variable resolver in constructor * Load resolver upon construction of extHostTerminalService * Utilize last active workspace root * Reevaluate variableReoslver whenever the workspace gets new folder * Fix types for string shellArgs * Use async one level higher * Fix compile issue * Initialize resolver when exthost is created * Fix ext host in remote case with no folder open * Resolve args that an ecxctension passes in * Remove TODO * Resolve extension arguments
1 parent 6d49c7a commit 0a8ca63

4 files changed

Lines changed: 99 additions & 17 deletions

File tree

‎src/vs/workbench/api/node/extHostTerminalService.ts‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,8 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
286286
private _terminalProcesses: { [id: number]: ITerminalChildProcess } = {};
287287
private _terminalRenderers: ExtHostTerminalRenderer[] = [];
288288
private _getTerminalPromises: { [id: number]: Promise<ExtHostTerminal> } = {};
289+
private _variableResolver: ExtHostVariableResolverService | undefined;
290+
private _lastActiveWorkspace: IWorkspaceFolder | undefined;
289291

290292
// TODO: Pull this from main side
291293
private _isWorkspaceShellAllowed: boolean = false;
@@ -307,9 +309,12 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
307309
private _extHostConfiguration: ExtHostConfiguration,
308310
private _extHostWorkspace: ExtHostWorkspace,
309311
private _extHostDocumentsAndEditors: ExtHostDocumentsAndEditors,
310-
private _logService: ILogService,
312+
private _logService: ILogService
311313
) {
312314
this._proxy = mainContext.getProxy(MainContext.MainThreadTerminalService);
315+
this.updateLastActiveWorkspace();
316+
this.updateVariableResolver();
317+
this.registerListeners();
313318
}
314319

315320
public createTerminal(name?: string, shellPath?: string, shellArgs?: string[] | string): vscode.Terminal {
@@ -366,18 +371,21 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
366371
this._isWorkspaceShellAllowed,
367372
getSystemShell(platform.platform),
368373
process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'),
369-
process.env.windir
374+
process.env.windir,
375+
this._lastActiveWorkspace,
376+
this._variableResolver
370377
);
371378
}
372379

373-
private _getDefaultShellArgs(configProvider: ExtHostConfigProvider): string[] | string | undefined {
380+
private _getDefaultShellArgs(configProvider: ExtHostConfigProvider): string[] | string {
374381
const fetchSetting = (key: string) => {
375382
const setting = configProvider
376383
.getConfiguration(key.substr(0, key.lastIndexOf('.')))
377384
.inspect<string | string[]>(key.substr(key.lastIndexOf('.') + 1));
378385
return this._apiInspectConfigToPlain<string | string[]>(setting);
379386
};
380-
return terminalEnvironment.getDefaultShellArgs(fetchSetting, this._isWorkspaceShellAllowed);
387+
388+
return terminalEnvironment.getDefaultShellArgs(fetchSetting, this._isWorkspaceShellAllowed, this._lastActiveWorkspace, this._variableResolver);
381389
}
382390

383391
public async resolveTerminalRenderer(id: number): Promise<vscode.TerminalRenderer> {
@@ -526,6 +534,24 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
526534
return env;
527535
}
528536

537+
private registerListeners(): void {
538+
this._extHostDocumentsAndEditors.onDidChangeActiveTextEditor(() => this.updateLastActiveWorkspace());
539+
this._extHostWorkspace.onDidChangeWorkspace(() => this.updateVariableResolver());
540+
}
541+
542+
private updateLastActiveWorkspace(): void {
543+
const activeEditor = this._extHostDocumentsAndEditors.activeEditor();
544+
if (activeEditor) {
545+
this._lastActiveWorkspace = this._extHostWorkspace.getWorkspaceFolder(activeEditor.document.uri) as IWorkspaceFolder;
546+
}
547+
}
548+
549+
private async updateVariableResolver(): Promise<void> {
550+
const configProvider = await this._extHostConfiguration.getConfigProvider();
551+
const workspaceFolders = await this._extHostWorkspace.getWorkspaceFolders2();
552+
this._variableResolver = new ExtHostVariableResolverService(workspaceFolders || [], this._extHostDocumentsAndEditors, configProvider);
553+
}
554+
529555
public async $createProcess(id: number, shellLaunchConfigDto: ShellLaunchConfigDto, activeWorkspaceRootUriComponents: UriComponents, cols: number, rows: number, isWorkspaceShellAllowed: boolean): Promise<void> {
530556
const shellLaunchConfig: IShellLaunchConfig = {
531557
name: shellLaunchConfigDto.name,
@@ -541,6 +567,21 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
541567
if (!shellLaunchConfig.executable) {
542568
shellLaunchConfig.executable = this.getDefaultShell(configProvider);
543569
shellLaunchConfig.args = this._getDefaultShellArgs(configProvider);
570+
} else {
571+
if (this._variableResolver) {
572+
shellLaunchConfig.executable = this._variableResolver.resolve(this._lastActiveWorkspace, shellLaunchConfig.executable);
573+
if (shellLaunchConfig.args) {
574+
if (Array.isArray(shellLaunchConfig.args)) {
575+
const resolvedArgs: string[] = [];
576+
for (const arg of shellLaunchConfig.args) {
577+
resolvedArgs.push(this._variableResolver.resolve(this._lastActiveWorkspace, arg));
578+
}
579+
shellLaunchConfig.args = resolvedArgs;
580+
} else {
581+
shellLaunchConfig.args = this._variableResolver.resolve(this._lastActiveWorkspace, shellLaunchConfig.args);
582+
}
583+
}
584+
}
544585
}
545586

546587
// Get the initial cwd
@@ -559,14 +600,12 @@ export class ExtHostTerminalService implements ExtHostTerminalServiceShape {
559600
}
560601
} as IWorkspaceFolder : null;
561602
const envFromConfig = this._apiInspectConfigToPlain(configProvider.getConfiguration('terminal.integrated').inspect<ITerminalEnvironment>(`env.${platformKey}`));
562-
const workspaceFolders = await this._extHostWorkspace.getWorkspaceFolders2();
563-
const variableResolver = workspaceFolders ? new ExtHostVariableResolverService(workspaceFolders, this._extHostDocumentsAndEditors, configProvider) : undefined;
564603
const baseEnv = terminalConfig.get<boolean>('inheritEnv', true) ? process.env as platform.IProcessEnvironment : await this._getNonInheritedEnv();
565604
const env = terminalEnvironment.createTerminalEnvironment(
566605
shellLaunchConfig,
567606
lastActiveWorkspace,
568607
envFromConfig,
569-
variableResolver,
608+
this._variableResolver,
570609
isWorkspaceShellAllowed,
571610
pkg.version,
572611
terminalConfig.get<boolean>('setLocaleVariables', false),

‎src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,17 +184,29 @@ export class TerminalProcessManager implements ITerminalProcessManager {
184184
rows: number,
185185
isScreenReaderModeEnabled: boolean
186186
): Promise<ITerminalChildProcess> {
187+
const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file);
188+
const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux');
189+
const lastActiveWorkspace = activeWorkspaceRootUri ? this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri) : null;
187190
if (!shellLaunchConfig.executable) {
188191
const defaultConfig = await this._terminalInstanceService.getDefaultShellAndArgs();
189192
shellLaunchConfig.executable = defaultConfig.shell;
190193
shellLaunchConfig.args = defaultConfig.args;
194+
} else {
195+
shellLaunchConfig.executable = this._configurationResolverService.resolve(lastActiveWorkspace === null ? undefined : lastActiveWorkspace, shellLaunchConfig.executable);
196+
if (shellLaunchConfig.args) {
197+
if (Array.isArray(shellLaunchConfig.args)) {
198+
const resolvedArgs: string[] = [];
199+
for (const arg of shellLaunchConfig.args) {
200+
resolvedArgs.push(this._configurationResolverService.resolve(lastActiveWorkspace === null ? undefined : lastActiveWorkspace, arg));
201+
}
202+
shellLaunchConfig.args = resolvedArgs;
203+
} else {
204+
shellLaunchConfig.args = this._configurationResolverService.resolve(lastActiveWorkspace === null ? undefined : lastActiveWorkspace, shellLaunchConfig.args);
205+
}
206+
}
191207
}
192208

193-
const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot(Schemas.file);
194209
const initialCwd = terminalEnvironment.getCwd(shellLaunchConfig, this._environmentService.userHome, activeWorkspaceRootUri, this._configHelper.config.cwd);
195-
196-
const platformKey = platform.isWindows ? 'windows' : (platform.isMacintosh ? 'osx' : 'linux');
197-
const lastActiveWorkspace = activeWorkspaceRootUri ? this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri) : null;
198210
const envFromConfigValue = this._workspaceConfigurationService.inspect<ITerminalEnvironment | undefined>(`terminal.integrated.env.${platformKey}`);
199211
const isWorkspaceShellAllowed = this._configHelper.checkWorkspaceShellPermissions();
200212
const baseEnv = this._configHelper.config.inheritEnv ? process.env as platform.IProcessEnvironment : await this._terminalInstanceService.getMainProcessParentEnv();

‎src/vs/workbench/contrib/terminal/common/terminalEnvironment.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,9 @@ export function getDefaultShell(
169169
defaultShell: string,
170170
isWoW64: boolean,
171171
windir: string | undefined,
172-
platformOverride: platform.Platform = platform.platform
172+
lastActiveWorkspace: IWorkspaceFolder | undefined,
173+
configurationResolverService: IConfigurationResolverService | undefined,
174+
platformOverride: platform.Platform = platform.platform,
173175
): string {
174176
const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux';
175177
const shellConfigValue = fetchSetting(`terminal.integrated.shell.${platformKey}`);
@@ -190,17 +192,33 @@ export function getDefaultShell(
190192
executable = executable.replace(/\//g, '\\');
191193
}
192194

195+
if (configurationResolverService) {
196+
executable = configurationResolverService.resolve(lastActiveWorkspace, executable);
197+
}
198+
193199
return executable;
194200
}
195201

196202
export function getDefaultShellArgs(
197203
fetchSetting: (key: string) => { user: string | string[] | undefined, value: string | string[] | undefined, default: string | string[] | undefined },
198204
isWorkspaceShellAllowed: boolean,
199-
platformOverride: platform.Platform = platform.platform
200-
): string[] {
205+
lastActiveWorkspace: IWorkspaceFolder | undefined,
206+
configurationResolverService: IConfigurationResolverService | undefined,
207+
platformOverride: platform.Platform = platform.platform,
208+
): string | string[] {
201209
const platformKey = platformOverride === platform.Platform.Windows ? 'windows' : platformOverride === platform.Platform.Mac ? 'osx' : 'linux';
202210
const shellArgsConfigValue = fetchSetting(`terminal.integrated.shellArgs.${platformKey}`);
203-
const args = (isWorkspaceShellAllowed ? <string[]>shellArgsConfigValue.value : <string[]>shellArgsConfigValue.user) || <string[]>shellArgsConfigValue.default;
211+
let args = <string[] | string>((isWorkspaceShellAllowed ? shellArgsConfigValue.value : shellArgsConfigValue.user) || shellArgsConfigValue.default);
212+
if (typeof args === 'string' && platformOverride === platform.Platform.Windows) {
213+
return configurationResolverService ? configurationResolverService.resolve(lastActiveWorkspace, args) : args;
214+
}
215+
if (configurationResolverService) {
216+
const resolvedArgs: string[] = [];
217+
for (const arg of args) {
218+
resolvedArgs.push(configurationResolverService.resolve(lastActiveWorkspace, arg));
219+
}
220+
args = resolvedArgs;
221+
}
204222
return args;
205223
}
206224

‎src/vs/workbench/contrib/terminal/electron-browser/terminalInstanceService.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
1717
import { getDefaultShell, getDefaultShellArgs } from 'vs/workbench/contrib/terminal/common/terminalEnvironment';
1818
import { StorageScope, IStorageService } from 'vs/platform/storage/common/storage';
1919
import { getMainProcessParentEnv } from 'vs/workbench/contrib/terminal/node/terminalEnvironment';
20+
import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver';
21+
import { IHistoryService } from 'vs/workbench/services/history/common/history';
22+
import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
2023

2124
let Terminal: typeof XTermTerminal;
2225
let WebLinksAddon: typeof XTermWebLinksAddon;
@@ -28,7 +31,10 @@ export class TerminalInstanceService implements ITerminalInstanceService {
2831
constructor(
2932
@IInstantiationService private readonly _instantiationService: IInstantiationService,
3033
@IConfigurationService private readonly _configurationService: IConfigurationService,
31-
@IStorageService private readonly _storageService: IStorageService
34+
@IStorageService private readonly _storageService: IStorageService,
35+
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
36+
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
37+
@IHistoryService private readonly _historyService: IHistoryService
3238
) {
3339
}
3440

@@ -65,19 +71,26 @@ export class TerminalInstanceService implements ITerminalInstanceService {
6571
return this._storageService.getBoolean(IS_WORKSPACE_SHELL_ALLOWED_STORAGE_KEY, StorageScope.WORKSPACE, false);
6672
}
6773

68-
public getDefaultShellAndArgs(platformOverride: Platform = platform): Promise<{ shell: string, args: string[] | undefined }> {
74+
public getDefaultShellAndArgs(platformOverride: Platform = platform): Promise<{ shell: string, args: string | string[] }> {
6975
const isWorkspaceShellAllowed = this._isWorkspaceShellAllowed();
76+
const activeWorkspaceRootUri = this._historyService.getLastActiveWorkspaceRoot();
77+
let lastActiveWorkspace = activeWorkspaceRootUri ? this._workspaceContextService.getWorkspaceFolder(activeWorkspaceRootUri) : undefined;
78+
lastActiveWorkspace = lastActiveWorkspace === null ? undefined : lastActiveWorkspace;
7079
const shell = getDefaultShell(
7180
(key) => this._configurationService.inspect(key),
7281
isWorkspaceShellAllowed,
7382
getSystemShell(platformOverride),
7483
process.env.hasOwnProperty('PROCESSOR_ARCHITEW6432'),
7584
process.env.windir,
85+
lastActiveWorkspace,
86+
this._configurationResolverService,
7687
platformOverride
7788
);
7889
const args = getDefaultShellArgs(
7990
(key) => this._configurationService.inspect(key),
8091
isWorkspaceShellAllowed,
92+
lastActiveWorkspace,
93+
this._configurationResolverService,
8194
platformOverride
8295
);
8396
return Promise.resolve({ shell, args });

0 commit comments

Comments
 (0)