Move EnvironmentVariableCollection API into ExtensionContext (#96061)

* Move env var collection to ext context

* Remove dispose, fix persistent passing

* Fire collection change on persistence change

* Fix tests by forcing activation and getting ctx

* chore: bump js-debug

Co-authored-by: Connor Peet <connor@peet.io>
This commit is contained in:
Daniel Imms 2020-04-24 16:45:30 -07:00 committed by GitHub
parent 00ec9390a4
commit 7f5bada046
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 73 additions and 79 deletions

View file

@ -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;
}

View file

@ -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~');

View file

@ -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"
}
}
]
}

View file

@ -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

View file

@ -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;
},

View file

@ -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);
}
});
});
}

View file

@ -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 {

View file

@ -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);
});
}