From fbbf987c3affb475c2360164f4ecc1eb86e83f7b Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 9 Aug 2023 18:20:26 -0700 Subject: [PATCH] Use updated API to fetch scoped env collection (#21788) For https://github.com/microsoft/vscode/issues/171173 https://github.com/microsoft/vscode-python/issues/20822 To be merged tomorrow when latest insiders is released. Blocked on https://github.com/microsoft/vscode/pull/189979. --- package-lock.json | 16 ++++----- package.json | 4 +-- .../terminalEnvVarCollectionService.ts | 34 ++++++------------- ...rminalEnvVarCollectionService.unit.test.ts | 20 +++++------ ...scode.proposed.envCollectionWorkspace.d.ts | 33 +++++++++--------- 5 files changed, 46 insertions(+), 61 deletions(-) diff --git a/package-lock.json b/package-lock.json index 01f03cb31661..56b94d1dd44c 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-20230809" } }, "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..2d78d407a0c9 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "theme": "dark" }, "engines": { - "vscode": "^1.79.0-20230526" + "vscode": "^1.81.0-20230809" }, "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..2e5c26be0222 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'; @@ -67,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( @@ -111,8 +105,8 @@ 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); - // Clear any previously set env vars from collection. + const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder }); + // Clear any previously set env vars from collection envVarCollection.clear(); if (!settings.terminal.activateEnvironment) { traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); @@ -160,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, + }); } } }); @@ -170,22 +167,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', @@ -195,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 () => { 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; - } }