restructure command variable resolving

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<T>(root: IWorkspaceFolder, value: T, commandMapping?: IStringDictionary<string>): T {
+		return this._variableResolver.resolveAny(root ? root.uri : undefined, value, commandMapping);
 	}
 
-	resolveInteractiveVariables(configuration: any, interactiveVariablesMap: { [key: string]: string; }): TPromise<any, any> {
-		throw new Error('Method not implemented.');
+	public executeCommandVariables(configuration: any, variables: IStringDictionary<string>): TPromise<IStringDictionary<string>> {
+		throw new Error('findAndExecuteCommandVariables not implemented.');
 	}
 }
\ No newline at end of file

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<string>): 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);
 	}
 }
 

src/vs/workbench/parts/debug/node/debugger.ts

@@ -63,21 +63,24 @@ export class Debugger {
 
 	public substituteVariables(folder: IWorkspaceFolder, config: IConfig): TPromise<IConfig> {
 
-		let configP: TPromise<IConfig>;
-		const debugConfigs = this.configurationService.getValue<IDebugConfiguration>('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<IDebugConfiguration>('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);
+				}
+			});
 		});
 	}
 

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<string>): IStringDictionary<string>;
-	resolveAny<T>(root: IWorkspaceFolder, value: T): T;
-	resolveInteractiveVariables(configuration: any, interactiveVariablesMap: { [key: string]: string }): TPromise<any>;
+	resolveAny<T>(root: IWorkspaceFolder, value: T, commandMapping?: IStringDictionary<string>): T;
+	executeCommandVariables(value: any, variables: IStringDictionary<string>): TPromise<IStringDictionary<string>>;
 }

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<string>): 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<any> {
+	public executeCommandVariables(configuration: any, variableToCommandMap: IStringDictionary<string>): TPromise<IStringDictionary<string>> {
+
 		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<any> }[] = Object.keys(interactiveVariablesToSubstitutes).map(interactiveVariable => {
+		findCommandVariables(configuration);
+
+		let cancelled = false;
+		const commandValueMapping: IStringDictionary<string> = Object.create(null);
+
+		const factory: { (): TPromise<any> }[] = 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<string>(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);
 	}
 }

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<string>): 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 | IStringDictionary<string> | 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>): 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;

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',