From 540707d948c0f5e51cff7bf6c37ceffa2bfb6bb0 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 8 Aug 2023 21:58:11 +0000 Subject: [PATCH 1/3] Use updated API to fetch scoped env collection --- package-lock.json | 16 ++++----- package.json | 4 +-- .../terminalEnvVarCollectionService.ts | 22 ++----------- ...scode.proposed.envCollectionWorkspace.d.ts | 33 +++++++++---------- 4 files changed, 29 insertions(+), 46 deletions(-) diff --git a/package-lock.json b/package-lock.json index 01f03cb31661..16c83257adda 100644 --- a/package-lock.json +++ b/package-lock.json @@ -67,7 +67,7 @@ "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.75.0", + "@types/vscode": "^1.81.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", @@ -128,7 +128,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.79.0-20230526" + "vscode": "^1.81.0-20230808" } }, "node_modules/@aashutoshrathi/word-wrap": { @@ -1491,9 +1491,9 @@ "dev": true }, "node_modules/@types/vscode": { - "version": "1.75.1", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.75.1.tgz", - "integrity": "sha512-emg7wdsTFzdi+elvoyoA+Q8keEautdQHyY5LNmHVM4PTpY8JgOTVADrGVyXGepJ6dVW2OS5/xnLUWh+nZxvdiA==", + "version": "1.81.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.81.0.tgz", + "integrity": "sha512-YIaCwpT+O2E7WOMq0eCgBEABE++SX3Yl/O02GoMIF2DO3qAtvw7m6BXFYsxnc6XyzwZgh6/s/UG78LSSombl2w==", "dev": true }, "node_modules/@types/which": { @@ -16508,9 +16508,9 @@ "dev": true }, "@types/vscode": { - "version": "1.75.1", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.75.1.tgz", - "integrity": "sha512-emg7wdsTFzdi+elvoyoA+Q8keEautdQHyY5LNmHVM4PTpY8JgOTVADrGVyXGepJ6dVW2OS5/xnLUWh+nZxvdiA==", + "version": "1.81.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.81.0.tgz", + "integrity": "sha512-YIaCwpT+O2E7WOMq0eCgBEABE++SX3Yl/O02GoMIF2DO3qAtvw7m6BXFYsxnc6XyzwZgh6/s/UG78LSSombl2w==", "dev": true }, "@types/which": { diff --git a/package.json b/package.json index b6a4b65b42a9..9e5893a33791 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.79.0-20230526" + "vscode": "^1.81.0-20230808" }, "enableTelemetry": false, "keywords": [ @@ -2151,7 +2151,7 @@ "@types/stack-trace": "0.0.29", "@types/tmp": "^0.0.33", "@types/uuid": "^8.3.4", - "@types/vscode": "^1.75.0", + "@types/vscode": "^1.81.0", "@types/which": "^2.0.1", "@types/winreg": "^1.2.30", "@types/xml2js": "^0.4.2", diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index f4c29f03707d..45c8bcdb6466 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -3,14 +3,7 @@ import * as path from 'path'; import { inject, injectable } from 'inversify'; -import { - ProgressOptions, - ProgressLocation, - MarkdownString, - WorkspaceFolder, - EnvironmentVariableCollection, - EnvironmentVariableScope, -} from 'vscode'; +import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode'; import { pathExists } from 'fs-extra'; import { IExtensionActivationService } from '../../activation/types'; import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types'; @@ -111,7 +104,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise { const workspaceFolder = this.getWorkspaceFolder(resource); const settings = this.configurationService.getSettings(resource); - const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder); + const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder }); // Clear any previously set env vars from collection. envVarCollection.clear(); if (!settings.terminal.activateEnvironment) { @@ -170,22 +163,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ envVarCollection.description = description; } - private getEnvironmentVariableCollection(workspaceFolder?: WorkspaceFolder) { - const envVarCollection = this.context.environmentVariableCollection as EnvironmentVariableCollection & { - getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection; - }; - return workspaceFolder - ? envVarCollection.getScopedEnvironmentVariableCollection({ workspaceFolder }) - : envVarCollection; - } - private async handleMicroVenv(resource: Resource) { const workspaceFolder = this.getWorkspaceFolder(resource); const interpreter = await this.interpreterService.getActiveInterpreter(resource); if (interpreter?.envType === EnvironmentType.Venv) { const activatePath = path.join(path.dirname(interpreter.path), 'activate'); if (!(await pathExists(activatePath))) { - const envVarCollection = this.getEnvironmentVariableCollection(workspaceFolder); + const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder }); const pathVarName = getSearchPathEnvVarNames()[0]; envVarCollection.replace( 'PATH', diff --git a/types/vscode.proposed.envCollectionWorkspace.d.ts b/types/vscode.proposed.envCollectionWorkspace.d.ts index d778e53e5086..406e5b98cd47 100644 --- a/types/vscode.proposed.envCollectionWorkspace.d.ts +++ b/types/vscode.proposed.envCollectionWorkspace.d.ts @@ -7,15 +7,22 @@ declare module 'vscode' { // https://github.com/microsoft/vscode/issues/182069 - // export interface ExtensionContext { - // /** - // * Gets the extension's environment variable collection for this workspace, enabling changes - // * to be applied to terminal environment variables. - // * - // * @param scope The scope to which the environment variable collection applies to. - // */ - // readonly environmentVariableCollection: EnvironmentVariableCollection & { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection }; - // } + export interface ExtensionContext { + /** + * Gets the extension's environment variable collection for this workspace, enabling changes + * to be applied to terminal environment variables. + * + * @deprecated Use {@link getEnvironmentVariableCollection} instead. + */ + readonly environmentVariableCollection: EnvironmentVariableCollection; + /** + * Gets the extension's environment variable collection for this workspace, enabling changes + * to be applied to terminal environment variables. + * + * @param scope The scope to which the environment variable collection applies to. + */ + getEnvironmentVariableCollection(scope?: EnvironmentVariableScope): EnvironmentVariableCollection; + } export type EnvironmentVariableScope = { /** @@ -23,12 +30,4 @@ declare module 'vscode' { */ workspaceFolder?: WorkspaceFolder; }; - - export interface EnvironmentVariableCollection extends Iterable<[variable: string, mutator: EnvironmentVariableMutator]> { - /** - * A description for the environment variable collection, this will be used to describe the - * changes in the UI. - */ - description: string | MarkdownString | undefined; - } } From 2542ec343fc898fca9e7953a304a38206de40aac Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 8 Aug 2023 22:14:20 +0000 Subject: [PATCH 2/3] More updates --- package-lock.json | 2 +- package.json | 2 +- .../terminalEnvVarCollectionService.ts | 10 +++++++--- ...rminalEnvVarCollectionService.unit.test.ts | 20 +++++++++---------- 4 files changed, 18 insertions(+), 16 deletions(-) diff --git a/package-lock.json b/package-lock.json index 16c83257adda..56b94d1dd44c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -128,7 +128,7 @@ "yargs": "^15.3.1" }, "engines": { - "vscode": "^1.81.0-20230808" + "vscode": "^1.81.0-20230809" } }, "node_modules/@aashutoshrathi/word-wrap": { diff --git a/package.json b/package.json index 9e5893a33791..2d78d407a0c9 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.81.0-20230808" + "vscode": "^1.81.0-20230809" }, "enableTelemetry": false, "keywords": [ diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 45c8bcdb6466..091bf2d89c42 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -60,7 +60,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ public async activate(resource: Resource): Promise { if (!inTerminalEnvVarExperiment(this.experimentService)) { - this.context.environmentVariableCollection.clear(); + const workspaceFolder = this.getWorkspaceFolder(resource); + this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear(); await this.handleMicroVenv(resource); if (!this.registeredOnce) { this.interpreterService.onDidChangeInterpreter( @@ -153,7 +154,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } traceVerbose(`Setting environment variable ${key} in collection to ${value}`); - envVarCollection.replace(key, value, { applyAtShellIntegration: true }); + envVarCollection.replace(key, value, { + applyAtShellIntegration: true, + applyAtProcessCreation: true, + }); } } }); @@ -179,7 +183,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ return; } } - this.context.environmentVariableCollection.clear(); + this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear(); } private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined { diff --git a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 458e2b28c181..73c9e82274de 100644 --- a/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -45,7 +45,6 @@ suite('Terminal Environment Variable Collection Service', () => { let collection: EnvironmentVariableCollection & { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection; }; - let scopedCollection: EnvironmentVariableCollection; let applicationEnvironment: IApplicationEnvironment; let environmentActivationService: IEnvironmentActivationService; let workspaceService: IWorkspaceService; @@ -73,9 +72,7 @@ suite('Terminal Environment Variable Collection Service', () => { getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection; } >(); - scopedCollection = mock(); - when(collection.getScopedEnvironmentVariableCollection(anything())).thenReturn(instance(scopedCollection)); - when(context.environmentVariableCollection).thenReturn(instance(collection)); + when(context.getEnvironmentVariableCollection(anything())).thenReturn(instance(collection)); experimentService = mock(); when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); applicationEnvironment = mock(); @@ -95,7 +92,6 @@ suite('Terminal Environment Variable Collection Service', () => { pythonPath: displayPath, } as unknown) as IPythonSettings); when(collection.clear()).thenResolve(); - when(scopedCollection.clear()).thenResolve(); terminalEnvVarCollectionService = new TerminalEnvVarCollectionService( instance(platform), instance(interpreterService), @@ -253,15 +249,17 @@ suite('Terminal Environment Variable Collection Service', () => { environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, customShell), ).thenResolve(envVars); - when(scopedCollection.replace(anything(), anything(), anything())).thenCall((_e, _v, options) => { - assert.deepEqual(options, { applyAtShellIntegration: true }); - return Promise.resolve(); - }); + when(collection.replace(anything(), anything(), anything())).thenCall( + (_e, _v, options: EnvironmentVariableMutatorOptions) => { + assert.deepEqual(options, { applyAtShellIntegration: true, applyAtProcessCreation: true }); + return Promise.resolve(); + }, + ); await terminalEnvVarCollectionService._applyCollection(resource, customShell); - verify(scopedCollection.clear()).once(); - verify(scopedCollection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); + verify(collection.clear()).once(); + verify(collection.replace('CONDA_PREFIX', 'prefix/to/conda', anything())).once(); }); test('If no activated variables are returned for custom shell, fallback to using default shell', async () => { From 2b050b7fbcc3b0369f9d6f6f664f27a7bee98131 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 9 Aug 2023 23:05:13 +0000 Subject: [PATCH 3/3] Rnaodm --- .../interpreter/activation/terminalEnvVarCollectionService.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts index 091bf2d89c42..2e5c26be0222 100644 --- a/src/client/interpreter/activation/terminalEnvVarCollectionService.ts +++ b/src/client/interpreter/activation/terminalEnvVarCollectionService.ts @@ -106,7 +106,7 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ const workspaceFolder = this.getWorkspaceFolder(resource); const settings = this.configurationService.getSettings(resource); const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder }); - // Clear any previously set env vars from collection. + // Clear any previously set env vars from collection envVarCollection.clear(); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath);