From 524698473fc4eadc93bcdde56d1fb1c065846ff5 Mon Sep 17 00:00:00 2001 From: Andre Weinand Date: Sat, 21 Apr 2018 01:13:40 +0200 Subject: [PATCH] restructure command variable resolving --- .../workbench/api/node/extHostDebugService.ts | 8 +-- .../parts/debug/node/debugAdapter.ts | 5 +- src/vs/workbench/parts/debug/node/debugger.ts | 29 +++++---- .../common/configurationResolver.ts | 4 +- .../configurationResolverService.ts | 62 ++++++++++--------- .../node/variableResolver.ts | 23 ++++--- .../configurationResolverService.test.ts | 14 +++-- 7 files changed, 84 insertions(+), 61 deletions(-) diff --git a/src/vs/workbench/api/node/extHostDebugService.ts b/src/vs/workbench/api/node/extHostDebugService.ts index b969522ba5e..e85cb0fe1a7 100644 --- a/src/vs/workbench/api/node/extHostDebugService.ts +++ b/src/vs/workbench/api/node/extHostDebugService.ts @@ -584,11 +584,11 @@ export class ExtHostVariableResolverService implements IConfigurationResolverSer return this._variableResolver.resolveAny(root ? root.uri : undefined, value); } - public resolveAny(root: IWorkspaceFolder, value: any): any { - return this._variableResolver.resolveAny(root ? root.uri : undefined, value); + public resolveAny(root: IWorkspaceFolder, value: T, commandMapping?: IStringDictionary): T { + return this._variableResolver.resolveAny(root ? root.uri : undefined, value, commandMapping); } - resolveInteractiveVariables(configuration: any, interactiveVariablesMap: { [key: string]: string; }): TPromise { - throw new Error('Method not implemented.'); + public executeCommandVariables(configuration: any, variables: IStringDictionary): TPromise> { + throw new Error('findAndExecuteCommandVariables not implemented.'); } } \ No newline at end of file diff --git a/src/vs/workbench/parts/debug/node/debugAdapter.ts b/src/vs/workbench/parts/debug/node/debugAdapter.ts index c74bd1c9e4d..f3cc2f9fcab 100644 --- a/src/vs/workbench/parts/debug/node/debugAdapter.ts +++ b/src/vs/workbench/parts/debug/node/debugAdapter.ts @@ -20,6 +20,7 @@ import { IOutputService } from 'vs/workbench/parts/output/common/output'; import { IDebugAdapter, IAdapterExecutable, IDebuggerContribution, IPlatformSpecificAdapterContribution, IConfig } from 'vs/workbench/parts/debug/common/debug'; import { IConfigurationResolverService } from 'vs/workbench/services/configurationResolver/common/configurationResolver'; import { IWorkspaceFolder } from 'vs/platform/workspace/common/workspace'; +import { IStringDictionary } from 'vs/base/common/collections'; /** * Abstract implementation of the low level API for a debug adapter. @@ -409,7 +410,7 @@ export class DebugAdapter extends StreamDebugAdapter { } } - static substituteVariables(workspaceFolder: IWorkspaceFolder, config: IConfig, resolverService: IConfigurationResolverService): IConfig { + static substituteVariables(workspaceFolder: IWorkspaceFolder, config: IConfig, resolverService: IConfigurationResolverService, commandValueMapping?: IStringDictionary): IConfig { const result = objects.deepClone(config) as IConfig; @@ -428,7 +429,7 @@ export class DebugAdapter extends StreamDebugAdapter { delete result.linux; // substitute all variables in string values - return resolverService.resolveAny(workspaceFolder, result); + return resolverService.resolveAny(workspaceFolder, result, commandValueMapping); } } diff --git a/src/vs/workbench/parts/debug/node/debugger.ts b/src/vs/workbench/parts/debug/node/debugger.ts index e6e1ffed7ab..3a637954e12 100644 --- a/src/vs/workbench/parts/debug/node/debugger.ts +++ b/src/vs/workbench/parts/debug/node/debugger.ts @@ -63,21 +63,24 @@ export class Debugger { public substituteVariables(folder: IWorkspaceFolder, config: IConfig): TPromise { - let configP: TPromise; - const debugConfigs = this.configurationService.getValue('debug'); - if (debugConfigs.extensionHostDebugAdapter) { - configP = this.configurationManager.substituteVariables(this.type, folder, config); - } else { - try { - configP = TPromise.as(DebugAdapter.substituteVariables(folder, config, this.configurationResolverService)); - } catch (e) { - return TPromise.wrapError(e); + // first resolve command variables (which might have a UI) + return this.configurationResolverService.executeCommandVariables(config, this.variables).then(commandValueMapping => { + + if (!commandValueMapping) { // cancelled by user + return null; } - } - return configP.then(result => { - // substitute 'command' variables (including interactive) - return this.configurationResolverService.resolveInteractiveVariables(result, this.variables); + // optionally substitute in EH + const inEh = this.configurationService.getValue('debug').extensionHostDebugAdapter; + + // now substitute all other variables + return (inEh ? this.configurationManager.substituteVariables(this.type, folder, config) : TPromise.as(config)).then(config => { + try { + return TPromise.as(DebugAdapter.substituteVariables(folder, config, this.configurationResolverService, commandValueMapping)); + } catch (e) { + return TPromise.wrapError(e); + } + }); }); } diff --git a/src/vs/workbench/services/configurationResolver/common/configurationResolver.ts b/src/vs/workbench/services/configurationResolver/common/configurationResolver.ts index 2ef8daf1ccd..ec8a9eab100 100644 --- a/src/vs/workbench/services/configurationResolver/common/configurationResolver.ts +++ b/src/vs/workbench/services/configurationResolver/common/configurationResolver.ts @@ -16,6 +16,6 @@ export interface IConfigurationResolverService { resolve(root: IWorkspaceFolder, value: string): string; resolve(root: IWorkspaceFolder, value: string[]): string[]; resolve(root: IWorkspaceFolder, value: IStringDictionary): IStringDictionary; - resolveAny(root: IWorkspaceFolder, value: T): T; - resolveInteractiveVariables(configuration: any, interactiveVariablesMap: { [key: string]: string }): TPromise; + resolveAny(root: IWorkspaceFolder, value: T, commandMapping?: IStringDictionary): T; + executeCommandVariables(value: any, variables: IStringDictionary): TPromise>; } diff --git a/src/vs/workbench/services/configurationResolver/electron-browser/configurationResolverService.ts b/src/vs/workbench/services/configurationResolver/electron-browser/configurationResolverService.ts index 080f600ce81..47ef388a39e 100644 --- a/src/vs/workbench/services/configurationResolver/electron-browser/configurationResolverService.ts +++ b/src/vs/workbench/services/configurationResolver/electron-browser/configurationResolverService.ts @@ -94,44 +94,52 @@ export class ConfigurationResolverService implements IConfigurationResolverServi return this.resolver.resolveAny(root ? root.uri : undefined, value); } - public resolveAny(root: IWorkspaceFolder, value: any): any { - return this.resolver.resolveAny(root ? root.uri : undefined, value); + public resolveAny(root: IWorkspaceFolder, value: any, commandValueMapping?: IStringDictionary): any { + return this.resolver.resolveAny(root ? root.uri : undefined, value, commandValueMapping); } /** - * Resolve all interactive variables in configuration #6569 + * Finds and executes all command variables (see #6569) */ - public resolveInteractiveVariables(configuration: any, interactiveVariablesMap: { [key: string]: string }): TPromise { + public executeCommandVariables(configuration: any, variableToCommandMap: IStringDictionary): TPromise> { + if (!configuration) { return TPromise.as(null); } - // We need a map from interactive variables to keys because we only want to trigger an command once per key - - // even though it might occur multiple times in configuration #7026. - const interactiveVariablesToSubstitutes: { [interactiveVariable: string]: { object: any, key: string }[] } = Object.create(null); - const findInteractiveVariables = (object: any) => { + // use an array to preserve order of first appearance + const commands: string[] = []; + + const cmd_var = /\${command:(.*?)}/g; + + const findCommandVariables = (object: any) => { Object.keys(object).forEach(key => { - if (object[key] && typeof object[key] === 'object') { - findInteractiveVariables(object[key]); - } else if (typeof object[key] === 'string') { - const matches = /\${command:(.*?)}/.exec(object[key]); - if (matches && matches.length === 2) { - const interactiveVariable = matches[1]; - if (!interactiveVariablesToSubstitutes[interactiveVariable]) { - interactiveVariablesToSubstitutes[interactiveVariable] = []; + const value = object[key]; + if (value && typeof value === 'object') { + findCommandVariables(value); + } else if (typeof value === 'string') { + let matches; + while ((matches = cmd_var.exec(value)) !== null) { + if (matches.length === 2) { + const command = matches[1]; + if (commands.indexOf(command) < 0) { + commands.push(command); + } } - interactiveVariablesToSubstitutes[interactiveVariable].push({ object, key }); } } }); }; - findInteractiveVariables(configuration); - let substitionCanceled = false; - const factory: { (): TPromise }[] = Object.keys(interactiveVariablesToSubstitutes).map(interactiveVariable => { + findCommandVariables(configuration); + + let cancelled = false; + const commandValueMapping: IStringDictionary = Object.create(null); + + const factory: { (): TPromise }[] = commands.map(interactiveVariable => { return () => { - let commandId: string = null; - commandId = interactiveVariablesMap ? interactiveVariablesMap[interactiveVariable] : null; + + let commandId = variableToCommandMap ? variableToCommandMap[interactiveVariable] : null; if (!commandId) { // Just launch any command if the interactive variable is not contributed by the adapter #12735 commandId = interactiveVariable; @@ -139,18 +147,14 @@ export class ConfigurationResolverService implements IConfigurationResolverServi return this.commandService.executeCommand(commandId, configuration).then(result => { if (result) { - interactiveVariablesToSubstitutes[interactiveVariable].forEach(substitute => { - if (substitute.object[substitute.key].indexOf(`\${command:${interactiveVariable}}`) >= 0) { - substitute.object[substitute.key] = substitute.object[substitute.key].replace(`\${command:${interactiveVariable}}`, result); - } - }); + commandValueMapping[interactiveVariable] = result; } else { - substitionCanceled = true; + cancelled = true; } }); }; }); - return sequence(factory).then(() => substitionCanceled ? null : configuration); + return sequence(factory).then(() => cancelled ? null : commandValueMapping); } } diff --git a/src/vs/workbench/services/configurationResolver/node/variableResolver.ts b/src/vs/workbench/services/configurationResolver/node/variableResolver.ts index a8fc1e8b982..0b493b1314f 100644 --- a/src/vs/workbench/services/configurationResolver/node/variableResolver.ts +++ b/src/vs/workbench/services/configurationResolver/node/variableResolver.ts @@ -12,7 +12,6 @@ import { normalizeDriveLetter } from 'vs/base/common/labels'; import { localize } from 'vs/nls'; import uri from 'vs/base/common/uri'; - export interface IVariableAccessor { getFolderUri(folderName: string): uri | undefined; getWorkspaceFolderCount(): number; @@ -43,22 +42,22 @@ export class VariableResolver { } } - resolveAny(folderUri: uri, value: any): any { + resolveAny(folderUri: uri, value: any, commandValueMapping?: IStringDictionary): any { if (types.isString(value)) { - return this.resolve(folderUri, value); + return this.resolve(folderUri, value, commandValueMapping); } else if (types.isArray(value)) { - return value.map(s => this.resolveAny(folderUri, s)); + return value.map(s => this.resolveAny(folderUri, s, commandValueMapping)); } else if (types.isObject(value)) { let result: IStringDictionary | string[]> = Object.create(null); Object.keys(value).forEach(key => { - result[key] = this.resolveAny(folderUri, value[key]); + result[key] = this.resolveAny(folderUri, value[key], commandValueMapping); }); return result; } return value; } - resolve(folderUri: uri, value: string): string { + resolve(folderUri: uri, value: string, commandValueMapping: IStringDictionary): string { const filePath = this.accessor.getFilePath(); @@ -100,6 +99,16 @@ export class VariableResolver { } throw new Error(localize('missingConfigName', "'{0}' can not be resolved because no settings name is given.", match)); + case 'command': + if (argument && commandValueMapping) { + const v = commandValueMapping[argument]; + if (typeof v === 'string') { + return v; + } + throw new Error(localize('noValueForCommand', "'{0}' can not be resolved because the command has no value.", match)); + } + return match; + default: { // common error handling for all variables that require an open folder and accept a folder name argument @@ -197,7 +206,7 @@ export class VariableResolver { if (ep) { return ep; } - throw new Error(localize('canNotResolveExecPath', "'{0}' can not be resolved.", match)); + return match; default: return match; diff --git a/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts b/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts index 5b514fb535b..8054b3dd566 100644 --- a/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts +++ b/src/vs/workbench/services/configurationResolver/test/electron-browser/configurationResolverService.test.ts @@ -240,8 +240,11 @@ suite('Configuration Resolver Service', () => { interactiveVariables['interactiveVariable1'] = 'command1'; interactiveVariables['interactiveVariable2'] = 'command2'; - configurationResolverService.resolveInteractiveVariables(configuration, interactiveVariables).then(resolved => { - assert.deepEqual(resolved, { + configurationResolverService.executeCommandVariables(configuration, interactiveVariables).then(mapping => { + + const result = configurationResolverService.resolveAny(undefined, configuration, mapping); + + assert.deepEqual(result, { 'name': 'Attach to Process', 'type': 'node', 'request': 'attach', @@ -272,8 +275,11 @@ suite('Configuration Resolver Service', () => { interactiveVariables['interactiveVariable1'] = 'command1'; interactiveVariables['interactiveVariable2'] = 'command2'; - configurationResolverService.resolveInteractiveVariables(configuration, interactiveVariables).then(resolved => { - assert.deepEqual(resolved, { + configurationResolverService.executeCommandVariables(configuration, interactiveVariables).then(mapping => { + + const result = configurationResolverService.resolveAny(undefined, configuration, mapping); + + assert.deepEqual(result, { 'name': 'Attach to Process', 'type': 'node', 'request': 'attach', -- GitLab