Move EnvironmentVariableCollection API into ExtensionContext

extensions/vscode-api-tests/src/extension.ts

@@ -6,6 +6,6 @@
 import * as vscode from 'vscode';
 
 export function activate(_context: vscode.ExtensionContext) {
-	// noop
+	// Set context as a global as some tests depend on it
+	(global as any).testExtensionContext = _context;
 }
-

extensions/vscode-api-tests/src/singlefolder-tests/terminal.test.ts

@@ -3,20 +3,27 @@
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
 
-import { window, Pseudoterminal, EventEmitter, TerminalDimensions, workspace, ConfigurationTarget, Disposable, UIKind, env, EnvironmentVariableMutatorType, EnvironmentVariableMutator } from 'vscode';
+import { window, Pseudoterminal, EventEmitter, TerminalDimensions, workspace, ConfigurationTarget, Disposable, UIKind, env, EnvironmentVariableMutatorType, EnvironmentVariableMutator, extensions, ExtensionContext } from 'vscode';
 import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 
 // Disable terminal tests:
 // - Web https://github.com/microsoft/vscode/issues/92826
 // - Remote https://github.com/microsoft/vscode/issues/96057
 ((env.uiKind === UIKind.Web || typeof env.remoteName !== 'undefined') ? suite.skip : suite)('vscode API - terminal', () => {
+	let extensionContext: ExtensionContext;
+
 	suiteSetup(async () => {
+		// Trigger extension activation and grab the context as some tests depend on it
+		await extensions.getExtension('vscode.vscode-api-tests')?.activate();
+		extensionContext = (global as any).testExtensionContext;
+
 		const config = workspace.getConfiguration('terminal.integrated');
 		// Disable conpty in integration tests because of https://github.com/microsoft/vscode/issues/76548
 		await config.update('windowsEnableConpty', false, ConfigurationTarget.Global);
 		// Disable exit alerts as tests may trigger then and we're not testing the notifications
 		await config.update('showExitAlert', false, ConfigurationTarget.Global);
 	});
+
 	suite('Terminal', () => {
 		let disposables: Disposable[] = [];
 
@@ -25,7 +32,6 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 			disposables.length = 0;
 		});
 
-
 		test('sendText immediately after createTerminal should not throw', (done) => {
 			disposables.push(window.onDidOpenTerminal(term => {
 				try {
@@ -607,7 +613,7 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 			});
 		});
 
-		suite('getEnvironmentVariableCollection', () => {
+		suite('environmentVariableCollection', () => {
 			test('should have collection variables apply to terminals immediately after setting', (done) => {
 				// Text to match on before passing the test
 				const expectedText = [
@@ -632,8 +638,8 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 						}
 					}
 				}));
-				const collection = window.getEnvironmentVariableCollection();
-				disposables.push(collection);
+				const collection = extensionContext.environmentVariableCollection;
+				disposables.push({ dispose: () => collection.clear() });
 				collection.replace('A', '~a2~');
 				collection.append('B', '~b2~');
 				collection.prepend('C', '~c2~');
@@ -678,8 +684,8 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 						}
 					}
 				}));
-				const collection = window.getEnvironmentVariableCollection();
-				disposables.push(collection);
+				const collection = extensionContext.environmentVariableCollection;
+				disposables.push({ dispose: () => collection.clear() });
 				collection.replace('A', '~a2~');
 				collection.append('B', '~b2~');
 				collection.prepend('C', '~c2~');
@@ -723,8 +729,8 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 						}
 					}
 				}));
-				const collection = window.getEnvironmentVariableCollection();
-				disposables.push(collection);
+				const collection = extensionContext.environmentVariableCollection;
+				disposables.push({ dispose: () => collection.clear() });
 				collection.replace('A', '~a2~');
 				collection.replace('B', '~a2~');
 				collection.clear();
@@ -765,8 +771,8 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 						}
 					}
 				}));
-				const collection = window.getEnvironmentVariableCollection();
-				disposables.push(collection);
+				const collection = extensionContext.environmentVariableCollection;
+				disposables.push({ dispose: () => collection.clear() });
 				collection.replace('A', '~a2~');
 				collection.replace('B', '~b2~');
 				collection.delete('A');
@@ -785,8 +791,8 @@ import { doesNotThrow, equal, ok, deepEqual, throws } from 'assert';
 			});
 
 			test('get and forEach should work', () => {
-				const collection = window.getEnvironmentVariableCollection();
-				disposables.push(collection);
+				const collection = extensionContext.environmentVariableCollection;
+				disposables.push({ dispose: () => collection.clear() });
 				collection.replace('A', '~a2~');
 				collection.append('B', '~b2~');
 				collection.prepend('C', '~c2~');

product.json

@@ -21,6 +21,7 @@
 	"reportIssueUrl": "https://github.com/Microsoft/vscode/issues/new",
 	"urlProtocol": "code-oss",
 	"extensionAllowedProposedApi": [
+		"ms-vscode.vscode-js-profile-table",
 		"ms-vscode.references-view"
 	],
 	"builtInExtensions": [
@@ -86,7 +87,7 @@
 		},
 		{
 			"name": "ms-vscode.js-debug-nightly",
-			"version": "2020.4.2217",
+			"version": "2020.4.2417",
 			"repo": "https://github.com/Microsoft/vscode-js-debug",
 			"metadata": {
 				"id": "7acbb4ce-c85a-49d4-8d95-a8054406ae97",
@@ -98,6 +99,21 @@
 				},
 				"publisherDisplayName": "Microsoft"
 			}
+		},
+		{
+			"name": "ms-vscode.vscode-js-profile-table",
+			"version": "0.0.1",
+			"repo": "https://github.com/Microsoft/vscode-js-debug",
+			"metadata": {
+				"id": "7e52b41b-71ad-457b-ab7e-0620f1fc4feb",
+				"publisherId": {
+					"publisherId": "5f5636e7-69ed-4afe-b5d6-8d231fb3d3ee",
+					"publisherName": "ms-vscode",
+					"displayName": "Microsoft",
+					"flags": "verified"
+				},
+				"publisherDisplayName": "Microsoft"
+			}
 		}
 	]
 }

src/vs/vscode.proposed.d.ts

@@ -983,6 +983,15 @@ declare module 'vscode' {
 	 * A collection of mutations that an extension can apply to a process environment.
 	 */
 	export interface EnvironmentVariableCollection {
+		/**
+		 * Whether the collection should be cached for the workspace and applied to the terminal
+		 * across window reloads. When true the collection will be active immediately such when the
+		 * window reloads. Additionally, this API will return the cached version if it exists. The
+		 * collection will be invalidated when the extension is uninstalled or when the collection
+		 * is cleared. Defaults to true.
+		 */
+		persistent: boolean;
+
 		/**
 		 * Replace an environment variable with a value.
 		 *
@@ -1026,26 +1035,14 @@ declare module 'vscode' {
 		 * Clears all mutators from this collection.
 		 */
 		clear(): void;
-
-		/**
-		 * Disposes the collection, if the collection was persisted it will no longer be retained
-		 * across reloads.
-		 */
-		dispose(): void;
 	}
 
-	export namespace window {
+	export interface ExtensionContext {
 		/**
-		 * Creates or returns the extension's environment variable collection for this workspace,
-		 * enabling changes to be applied to terminal environment variables.
-		 *
-		 * @param persistent Whether the collection should be cached for the workspace and applied
-		 * to the terminal across window reloads. When true the collection will be active
-		 * immediately such when the window reloads. Additionally, this API will return the cached
-		 * version if it exists. The collection will be invalidated when the extension is
-		 * uninstalled or when the collection is disposed. Defaults to false.
+		 * Gets the extension's environment variable collection for this workspace, enabling changes
+		 * to be applied to terminal environment variables.
 		 */
-		export function getEnvironmentVariableCollection(persistent?: boolean): EnvironmentVariableCollection;
+		readonly environmentVariableCollection: EnvironmentVariableCollection;
 	}
 
 	//#endregion

src/vs/workbench/api/common/extHost.api.impl.ts

@@ -488,10 +488,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I
 				checkProposedApiEnabled(extension);
 				return extHostTerminalService.onDidWriteTerminalData(listener, thisArg, disposables);
 			},
-			getEnvironmentVariableCollection(persistent?: boolean): vscode.EnvironmentVariableCollection {
-				checkProposedApiEnabled(extension);
-				return extHostTerminalService.getEnvironmentVariableCollection(extension, persistent);
-			},
 			get state() {
 				return extHostWindow.state;
 			},

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

@@ -16,7 +16,7 @@ import { ExtHostConfiguration, IExtHostConfiguration } from 'vs/workbench/api/co
 import { ActivatedExtension, EmptyExtension, ExtensionActivationReason, ExtensionActivationTimes, ExtensionActivationTimesBuilder, ExtensionsActivator, IExtensionAPI, IExtensionModule, HostExtension, ExtensionActivationTimesFragment } from 'vs/workbench/api/common/extHostExtensionActivator';
 import { ExtHostStorage, IExtHostStorage } from 'vs/workbench/api/common/extHostStorage';
 import { ExtHostWorkspace, IExtHostWorkspace } from 'vs/workbench/api/common/extHostWorkspace';
-import { ExtensionActivationError } from 'vs/workbench/services/extensions/common/extensions';
+import { ExtensionActivationError, checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions';
 import { ExtensionDescriptionRegistry } from 'vs/workbench/services/extensions/common/extensionDescriptionRegistry';
 import { CancellationTokenSource } from 'vs/base/common/cancellation';
 import * as errors from 'vs/base/common/errors';
@@ -33,6 +33,7 @@ import { IExtensionStoragePaths } from 'vs/workbench/api/common/extHostStoragePa
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 import { IExtHostTunnelService } from 'vs/workbench/api/common/extHostTunnelService';
+import { IExtHostTerminalService } from 'vs/workbench/api/common/extHostTerminalService';
 
 interface ITestRunner {
 	/** Old test runner API, as exported from `vscode/lib/testrunner` */
@@ -78,6 +79,7 @@ export abstract class AbstractExtHostExtensionService implements ExtHostExtensio
 	protected readonly _extHostConfiguration: ExtHostConfiguration;
 	protected readonly _logService: ILogService;
 	protected readonly _extHostTunnelService: IExtHostTunnelService;
+	protected readonly _extHostTerminalService: IExtHostTerminalService;
 
 	protected readonly _mainThreadWorkspaceProxy: MainThreadWorkspaceShape;
 	protected readonly _mainThreadTelemetryProxy: MainThreadTelemetryShape;
@@ -107,7 +109,8 @@ export abstract class AbstractExtHostExtensionService implements ExtHostExtensio
 		@ILogService logService: ILogService,
 		@IExtHostInitDataService initData: IExtHostInitDataService,
 		@IExtensionStoragePaths storagePath: IExtensionStoragePaths,
-		@IExtHostTunnelService extHostTunnelService: IExtHostTunnelService
+		@IExtHostTunnelService extHostTunnelService: IExtHostTunnelService,
+		@IExtHostTerminalService extHostTerminalService: IExtHostTerminalService
 	) {
 		this._hostUtils = hostUtils;
 		this._extHostContext = extHostContext;
@@ -117,6 +120,7 @@ export abstract class AbstractExtHostExtensionService implements ExtHostExtensio
 		this._extHostConfiguration = extHostConfiguration;
 		this._logService = logService;
 		this._extHostTunnelService = extHostTunnelService;
+		this._extHostTerminalService = extHostTerminalService;
 		this._disposables = new DisposableStore();
 
 		this._mainThreadWorkspaceProxy = this._extHostContext.getProxy(MainContext.MainThreadWorkspace);
@@ -371,7 +375,11 @@ export abstract class AbstractExtHostExtensionService implements ExtHostExtensio
 				get storagePath() { return that._storagePath.workspaceValue(extensionDescription); },
 				get globalStoragePath() { return that._storagePath.globalValue(extensionDescription); },
 				asAbsolutePath(relativePath: string) { return path.join(extensionDescription.extensionLocation.fsPath, relativePath); },
-				get logPath() { return path.join(that._initData.logsLocation.fsPath, extensionDescription.identifier.value); }
+				get logPath() { return path.join(that._initData.logsLocation.fsPath, extensionDescription.identifier.value); },
+				get environmentVariableCollection() {
+					checkProposedApiEnabled(extensionDescription);
+					return that._extHostTerminalService.getEnvironmentVariableCollection(extensionDescription);
+				}
 			});
 		});
 	}

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

@@ -644,36 +644,36 @@ export abstract class BaseExtHostTerminalService implements IExtHostTerminalServ
 
 export class EnvironmentVariableCollection implements vscode.EnvironmentVariableCollection {
 	readonly map: Map<string, vscode.EnvironmentVariableMutator> = new Map();
+	private _persistent: boolean = true;
 
-	private _disposed = false;
+	public get persistent(): boolean { return this._persistent; }
+	public set persistent(value: boolean) {
+		this._persistent = value;
+		this._onDidChangeCollection.fire();
+	}
 
 	protected readonly _onDidChangeCollection: Emitter<void> = new Emitter<void>();
 	get onDidChangeCollection(): Event<void> { return this._onDidChangeCollection && this._onDidChangeCollection.event; }
 
 	constructor(
-		readonly persistent: boolean,
 		serialized?: ISerializableEnvironmentVariableCollection
 	) {
 		this.map = new Map(serialized);
 	}
 
 	get size(): number {
-		this._checkDisposed();
 		return this.map.size;
 	}
 
 	replace(variable: string, value: string): void {
-		this._checkDisposed();
 		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Replace });
 	}
 
 	append(variable: string, value: string): void {
-		this._checkDisposed();
 		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Append });
 	}
 
 	prepend(variable: string, value: string): void {
-		this._checkDisposed();
 		this._setIfDiffers(variable, { value, type: EnvironmentVariableMutatorType.Prepend });
 	}
 
@@ -686,40 +686,22 @@ export class EnvironmentVariableCollection implements vscode.EnvironmentVariable
 	}
 
 	get(variable: string): vscode.EnvironmentVariableMutator | undefined {
-		this._checkDisposed();
 		return this.map.get(variable);
 	}
 
 	forEach(callback: (variable: string, mutator: vscode.EnvironmentVariableMutator, collection: vscode.EnvironmentVariableCollection) => any, thisArg?: any): void {
-		this._checkDisposed();
 		this.map.forEach((value, key) => callback.call(thisArg, key, value, this));
 	}
 
 	delete(variable: string): void {
-		this._checkDisposed();
 		this.map.delete(variable);
 		this._onDidChangeCollection.fire();
 	}
 
 	clear(): void {
-		this._checkDisposed();
 		this.map.clear();
 		this._onDidChangeCollection.fire();
 	}
-
-	dispose(): void {
-		if (!this._disposed) {
-			this._disposed = true;
-			this.map.clear();
-			this._onDidChangeCollection.fire();
-		}
-	}
-
-	protected _checkDisposed() {
-		if (this._disposed) {
-			throw new Error('EnvironmentVariableCollection has already been disposed');
-		}
-	}
 }
 
 export class WorkerExtHostTerminalService extends BaseExtHostTerminalService {

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

@@ -23,7 +23,6 @@ import { getMainProcessParentEnv } from 'vs/workbench/contrib/terminal/node/term
 import { BaseExtHostTerminalService, ExtHostTerminal, EnvironmentVariableCollection } from 'vs/workbench/api/common/extHostTerminalService';
 import { IExtHostRpcService } from 'vs/workbench/api/common/extHostRpcService';
 import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';
-import { dispose } from 'vs/base/common/lifecycle';
 import { serializeEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableShared';
 import { ISerializableEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariable';
 import { MergedEnvironmentVariableCollection } from 'vs/workbench/contrib/terminal/common/environmentVariableCollection';
@@ -227,22 +226,12 @@ export class ExtHostTerminalService extends BaseExtHostTerminalService {
 		this._isWorkspaceShellAllowed = isAllowed;
 	}
 
-	public getEnvironmentVariableCollection(extension: IExtensionDescription, persistent: boolean = false): vscode.EnvironmentVariableCollection {
-		let collection: EnvironmentVariableCollection | undefined;
-		if (persistent) {
-			// If persistent is specified, return the current collection if it exists
-			collection = this._environmentVariableCollections.get(extension.identifier.value);
-
-			// If persistence changed then create a new collection
-			if (collection && !collection.persistent) {
-				collection = undefined;
-			}
-		}
+	public getEnvironmentVariableCollection(extension: IExtensionDescription): vscode.EnvironmentVariableCollection {
+		let collection = this._environmentVariableCollections.get(extension.identifier.value);
 
 		if (!collection) {
-			// If not persistent, clear out the current collection and create a new one
-			dispose(this._environmentVariableCollections.get(extension.identifier.value));
-			collection = new EnvironmentVariableCollection(persistent);
+			// TODO: Disable dispose
+			collection = new EnvironmentVariableCollection();
 			this._setEnvironmentVariableCollection(extension.identifier.value, collection);
 		}
 
@@ -257,7 +246,7 @@ export class ExtHostTerminalService extends BaseExtHostTerminalService {
 	public $initEnvironmentVariableCollections(collections: [string, ISerializableEnvironmentVariableCollection][]): void {
 		collections.forEach(entry => {
 			const extensionIdentifier = entry[0];
-			const collection = new EnvironmentVariableCollection(true, entry[1]);
+			const collection = new EnvironmentVariableCollection(entry[1]);
 			this._setEnvironmentVariableCollection(extensionIdentifier, collection);
 		});
 	}