EnvironmentVariableMutatorOptions API proposal

src/vs/platform/terminal/common/environmentVariable.ts

@@ -11,24 +11,24 @@ export enum EnvironmentVariableMutatorType {
 	Append = 2,
 	Prepend = 3
 }
-// export enum EnvironmentVariableMutatorTiming {
-// 	AtSpawn = 1,
-// 	AfterShellIntegration = 2
-// 	// TODO: Do we need a both?
-// }
 export interface IEnvironmentVariableMutator {
 	readonly variable: string;
 	readonly value: string;
 	readonly type: EnvironmentVariableMutatorType;
 	readonly scope?: EnvironmentVariableScope;
-	// readonly timing?: EnvironmentVariableMutatorTiming;
+	readonly options?: IEnvironmentVariableMutatorOptions;
 }
 
 export interface IEnvironmentDescriptionMutator {
 	readonly description: string | undefined;
 	readonly scope?: EnvironmentVariableScope;
 }
 
+export interface IEnvironmentVariableMutatorOptions {
+	applyAtProcessCreation?: boolean;
+	applyAtShellIntegration?: boolean;
+}
+
 export type EnvironmentVariableScope = {
 	workspaceFolder?: IWorkspaceFolderData;
 };

src/vs/platform/terminal/common/environmentVariableCollection.ts

@@ -8,11 +8,11 @@ import { EnvironmentVariableMutatorType, EnvironmentVariableScope, IEnvironmentV
 
 type VariableResolver = (str: string) => Promise<string>;
 
-// const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
-// 	[EnvironmentVariableMutatorType.Append, 'APPEND'],
-// 	[EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
-// 	[EnvironmentVariableMutatorType.Replace, 'REPLACE']
-// ]);
+const mutatorTypeToLabelMap: Map<EnvironmentVariableMutatorType, string> = new Map([
+	[EnvironmentVariableMutatorType.Append, 'APPEND'],
+	[EnvironmentVariableMutatorType.Prepend, 'PREPEND'],
+	[EnvironmentVariableMutatorType.Replace, 'REPLACE']
+]);
 
 export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVariableCollection {
 	private readonly map: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
@@ -46,7 +46,8 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
 					value: mutator.value,
 					type: mutator.type,
 					scope: mutator.scope,
-					variable: mutator.variable
+					variable: mutator.variable,
+					options: mutator.options
 				};
 				if (!extensionMutator.scope) {
 					delete extensionMutator.scope; // Convenient for tests
@@ -69,26 +70,33 @@ export class MergedEnvironmentVariableCollection implements IMergedEnvironmentVa
 			const actualVariable = isWindows ? lowerToActualVariableNames![variable.toLowerCase()] || variable : variable;
 			for (const mutator of mutators) {
 				const value = variableResolver ? await variableResolver(mutator.value) : mutator.value;
-				// if (mutator.timing === EnvironmentVariableMutatorTiming.AfterShellIntegration) {
-				// 	const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
-				// 	env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + value;
-				// 	continue;
-				// }
-				switch (mutator.type) {
-					case EnvironmentVariableMutatorType.Append:
-						env[actualVariable] = (env[actualVariable] || '') + value;
-						break;
-					case EnvironmentVariableMutatorType.Prepend:
-						env[actualVariable] = value + (env[actualVariable] || '');
-						break;
-					case EnvironmentVariableMutatorType.Replace:
-						env[actualVariable] = value;
-						break;
+				// Default: true
+				if (mutator.options?.applyAtProcessCreation ?? true) {
+					switch (mutator.type) {
+						case EnvironmentVariableMutatorType.Append:
+							env[actualVariable] = (env[actualVariable] || '') + value;
+							break;
+						case EnvironmentVariableMutatorType.Prepend:
+							env[actualVariable] = value + (env[actualVariable] || '');
+							break;
+						case EnvironmentVariableMutatorType.Replace:
+							env[actualVariable] = value;
+							break;
+					}
+				}
+				// Default: false
+				if (mutator.options?.applyAtShellIntegration ?? false) {
+					const key = `VSCODE_ENV_${mutatorTypeToLabelMap.get(mutator.type)!}`;
+					env[key] = (env[key] ? env[key] + ':' : '') + variable + '=' + this._encodeColons(value);
 				}
 			}
 		}
 	}
 
+	private _encodeColons(value: string): string {
+		return value.replaceAll(':', '\\x3a');
+	}
+
 	diff(other: IMergedEnvironmentVariableCollection, scope: EnvironmentVariableScope | undefined): IMergedEnvironmentVariableCollectionDiff | undefined {
 		const added: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();
 		const changed: Map<string, IExtensionOwnedEnvironmentVariableMutator[]> = new Map();

src/vs/workbench/api/browser/mainThreadTerminalService.ts

@@ -407,6 +407,7 @@ export class MainThreadTerminalService implements MainThreadTerminalServiceShape
 				map: deserializeEnvironmentVariableCollection(collection),
 				descriptionMap: deserializeEnvironmentDescriptionMap(descriptionMap)
 			};
+			console.log('translatedCollection', translatedCollection);
 			this._environmentVariableService.set(extensionIdentifier, translatedCollection);
 		} else {
 			this._environmentVariableService.delete(extensionIdentifier);

src/vs/workbench/api/common/extHostTerminalService.ts

@@ -25,6 +25,7 @@ import { withNullAsUndefined } from 'vs/base/common/types';
 import { Promises } from 'vs/base/common/async';
 import { EditorGroupColumn } from 'vs/workbench/services/editor/common/editorGroupColumn';
 import { ViewColumn } from 'vs/workbench/api/common/extHostTypeConverters';
+import { isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';
 
 export interface IExtHostTerminalService extends ExtHostTerminalServiceShape, IDisposable {
 
@@ -826,7 +827,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
 	public getEnvironmentVariableCollection(extension: IExtensionDescription): IEnvironmentVariableCollection {
 		let collection = this._environmentVariableCollections.get(extension.identifier.value);
 		if (!collection) {
-			collection = new EnvironmentVariableCollection();
+			collection = new EnvironmentVariableCollection(extension);
 			this._setEnvironmentVariableCollection(extension.identifier.value, collection);
 		}
 		return collection.getScopedEnvironmentVariableCollection(undefined);
@@ -841,7 +842,7 @@ export abstract class BaseExtHostTerminalService extends Disposable implements I
 	public $initEnvironmentVariableCollections(collections: [string, ISerializableEnvironmentVariableCollection][]): void {
 		collections.forEach(entry => {
 			const extensionIdentifier = entry[0];
-			const collection = new EnvironmentVariableCollection(entry[1]);
+			const collection = new EnvironmentVariableCollection(undefined, entry[1]);
 			this._setEnvironmentVariableCollection(extensionIdentifier, collection);
 		});
 	}
@@ -883,6 +884,11 @@ class EnvironmentVariableCollection {
 	get onDidChangeCollection(): Event<void> { return this._onDidChangeCollection && this._onDidChangeCollection.event; }
 
 	constructor(
+		// HACK: Only check proposed options if extension is set (when the collection is not
+		// restored by serialization). This saves us from getting the extension details and
+		// shouldn't ever happen since you can only set them initially via the proposed check.
+		// TODO: This should be removed when the env var extension API(s) are stabilized
+		private readonly _extension: IExtensionDescription | undefined,
 		serialized?: ISerializableEnvironmentVariableCollection
 	) {
 		this.map = new Map(serialized);
@@ -899,19 +905,31 @@ class EnvironmentVariableCollection {
 		return scopedCollection;
 	}
 
-	replace(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
-		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, scope });
+	replace(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
+		if (this._extension && options) {
+			isProposedApiEnabled(this._extension, 'envCollectionOptions');
+		}
+		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace, options: options ?? {}, scope });
 	}
 
-	append(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
-		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, scope });
+	append(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
+		if (this._extension && options) {
+			isProposedApiEnabled(this._extension, 'envCollectionOptions');
+		}
+		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append, options: options ?? {}, scope });
 	}
 
-	prepend(variable: string, value: string, scope: vscode.EnvironmentVariableScope | undefined): void {
-		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, scope });
+	prepend(variable: string, value: string, options: vscode.EnvironmentVariableMutatorOptions | undefined, scope: vscode.EnvironmentVariableScope | undefined): void {
+		if (this._extension && options) {
+			isProposedApiEnabled(this._extension, 'envCollectionOptions');
+		}
+		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend, options: options ?? {}, scope });
 	}
 
 	private _setIfDiffers(variable: string, mutator: vscode.EnvironmentVariableMutator & { scope: vscode.EnvironmentVariableScope | undefined }): void {
+		if (mutator.options && mutator.options.applyAtProcessCreation === false && !mutator.options.applyAtShellIntegration) {
+			throw new Error('EnvironmentVariableMutatorOptions must apply at either process creation or shell integration');
+		}
 		if (!mutator.scope) {
 			delete (mutator as any).scope; // Convenient for tests
 		}
@@ -928,6 +946,7 @@ class EnvironmentVariableCollection {
 	get(variable: string, scope: vscode.EnvironmentVariableScope | undefined): vscode.EnvironmentVariableMutator | undefined {
 		const key = this.getKey(variable, scope);
 		const value = this.map.get(key);
+		// TODO: Set options to defaults if needed
 		return value ? convertMutator(value) : undefined;
 	}
 
@@ -944,11 +963,11 @@ class EnvironmentVariableCollection {
 		return workspaceFolder ? workspaceFolder.uri.toString() : undefined;
 	}
 
-	public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, IEnvironmentVariableMutator> {
-		const map = new Map<string, IEnvironmentVariableMutator>();
+	public getVariableMap(scope: vscode.EnvironmentVariableScope | undefined): Map<string, vscode.EnvironmentVariableMutator> {
+		const map = new Map<string, vscode.EnvironmentVariableMutator>();
 		for (const [key, value] of this.map) {
 			if (this.getScopeKey(value.scope) === this.getScopeKey(scope)) {
-				map.set(key, value);
+				map.set(key, convertMutator(value));
 			}
 		}
 		return map;
@@ -1022,24 +1041,24 @@ class ScopedEnvironmentVariableCollection implements vscode.EnvironmentVariableC
 		return this.collection.getScopedEnvironmentVariableCollection(this.scope);
 	}
 
-	replace(variable: string, value: string): void {
-		this.collection.replace(variable, value, this.scope);
+	replace(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
+		this.collection.replace(variable, value, options, this.scope);
 	}
 
-	append(variable: string, value: string): void {
-		this.collection.append(variable, value, this.scope);
+	append(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
+		this.collection.append(variable, value, options, this.scope);
 	}
 
-	prepend(variable: string, value: string): void {
-		this.collection.prepend(variable, value, this.scope);
+	prepend(variable: string, value: string, options?: vscode.EnvironmentVariableMutatorOptions | undefined): void {
+		this.collection.prepend(variable, value, options, this.scope);
 	}
 
 	get(variable: string): vscode.EnvironmentVariableMutator | undefined {
 		return this.collection.get(variable, this.scope);
 	}
 
 	forEach(callback: (variable: string, mutator: vscode.EnvironmentVariableMutator, collection: vscode.EnvironmentVariableCollection) => any, thisArg?: any): void {
-		this.collection.getVariableMap(this.scope).forEach((value, variable) => callback.call(thisArg, variable, convertMutator(value), this), this.scope);
+		this.collection.getVariableMap(this.scope).forEach((value, variable) => callback.call(thisArg, variable, value, this), this.scope);
 	}
 
 	[Symbol.iterator](): IterableIterator<[variable: string, mutator: vscode.EnvironmentVariableMutator]> {
@@ -1102,6 +1121,7 @@ function asTerminalColor(color?: vscode.ThemeColor): ThemeColor | undefined {
 function convertMutator(mutator: IEnvironmentVariableMutator): vscode.EnvironmentVariableMutator {
 	const newMutator = { ...mutator };
 	newMutator.scope = newMutator.scope ?? undefined;
+	newMutator.options = newMutator.options ?? undefined;
 	delete (newMutator as any).variable;
 	return newMutator as vscode.EnvironmentVariableMutator;
 }