From b2414a95f678eaf18c159e3e2c96a266b0ac3a67 Mon Sep 17 00:00:00 2001 From: isabelizimm Date: Wed, 13 Nov 2024 10:51:57 -0500 Subject: [PATCH 1/5] do not use GlobalEnvironmentVariableCollection --- .../envCollectionActivation/service.ts | 53 +++++++++++++------ ...rminalEnvVarCollectionService.unit.test.ts | 6 ++- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts index 77e478b3577..ddde6ed5d8d 100644 --- a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts +++ b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts @@ -6,10 +6,13 @@ import { inject, injectable } from 'inversify'; import { MarkdownString, WorkspaceFolder, - GlobalEnvironmentVariableCollection, - EnvironmentVariableScope, + // --- Start Positron --- + // GlobalEnvironmentVariableCollection, + // EnvironmentVariableScope, + // --- End Positron --- EnvironmentVariableMutatorOptions, ProgressLocation, + EnvironmentVariableCollection, } from 'vscode'; import { pathExists, normCase } from '../../common/platform/fs-paths'; import { IExtensionActivationService } from '../../activation/types'; @@ -169,7 +172,10 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private async _applyCollectionImpl(resource: Resource, shell = this.applicationEnvironment.shell): Promise { const workspaceFolder = this.getWorkspaceFolder(resource); const settings = this.configurationService.getSettings(resource); - const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder }); + // --- Start Positron --- + // remove workspace folder scope to avoid overwriting other extensions' env vars + const envVarCollection = this.getEnvironmentVariableCollection(); + // --- End Positron --- if (!settings.terminal.activateEnvironment) { envVarCollection.clear(); traceVerbose('Activating environments in terminal is disabled for', resource?.fsPath); @@ -365,9 +371,12 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ private async handleMicroVenv(resource: Resource) { try { const settings = this.configurationService.getSettings(resource); - const workspaceFolder = this.getWorkspaceFolder(resource); + // --- Start Positron --- + // remove workspace folder scope to avoid overwriting other extensions' env vars + // const workspaceFolder = this.getWorkspaceFolder(resource); if (!settings.terminal.activateEnvironment) { - this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); + this.getEnvironmentVariableCollection().clear(); + // --- End Positron --- traceVerbose( 'Do not activate microvenv as activating environments in terminal is disabled for', resource?.fsPath, @@ -378,7 +387,9 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ if (interpreter?.envType === EnvironmentType.Venv) { const activatePath = path.join(path.dirname(interpreter.path), 'activate'); if (!(await pathExists(activatePath))) { - const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder }); + // --- Start Positron --- + // remove workspace folder scope to avoid overwriting other extensions' env vars + const envVarCollection = this.getEnvironmentVariableCollection(); const pathVarName = getSearchPathEnvVarNames()[0]; envVarCollection.replace( 'PATH', @@ -387,7 +398,8 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ ); return; } - this.getEnvironmentVariableCollection({ workspaceFolder }).clear(); + this.getEnvironmentVariableCollection().clear(); + // --- End Positron --- } } catch (ex) { traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex); @@ -400,18 +412,27 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available. return isActive ? { - applyAtShellIntegration: true, - applyAtProcessCreation: false, - } + applyAtShellIntegration: true, + applyAtProcessCreation: false, + } : { - applyAtShellIntegration: true, // Takes care of false negatives in case manual integration is being used. - applyAtProcessCreation: true, - }; + applyAtShellIntegration: true, // Takes care of false negatives in case manual integration is being used. + applyAtProcessCreation: true, + }; } - private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) { - const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection; - return envVarCollection.getScoped(scope); + // --- Start Positron --- + // Global Environment Variable Collections will overwrite other additions to the same environment variable + // from other extensions, eg, Quarto + + private getEnvironmentVariableCollection() { + // private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) { + // const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection; + // return envVarCollection.getScoped(scope); + + const envVarCollection = this.context.environmentVariableCollection as EnvironmentVariableCollection; + return envVarCollection; + // --- End Positron --- } private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined { diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index faa95a08a3c..11542578270 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -80,8 +80,10 @@ suite('Terminal Environment Variable Collection Service', () => { when(shellIntegrationService.isWorking()).thenResolve(true); globalCollection = mock(); collection = mock(); - when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); - when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); + // --- Start Positron --- + when(context.environmentVariableCollection as EnvironmentVariableCollection).thenReturn(instance(collection)); + //when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); + // --- End Positron --- experimentService = mock(); when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); applicationEnvironment = mock(); From c5ccca8c8f32197da8e580286b689c00f0772b7e Mon Sep 17 00:00:00 2001 From: isabelizimm Date: Wed, 13 Nov 2024 12:13:40 -0500 Subject: [PATCH 2/5] lint --- .../terminals/envCollectionActivation/service.ts | 14 +++++++------- .../terminalEnvVarCollectionService.unit.test.ts | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts index ddde6ed5d8d..5b131497504 100644 --- a/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts +++ b/extensions/positron-python/src/client/terminals/envCollectionActivation/service.ts @@ -9,10 +9,10 @@ import { // --- Start Positron --- // GlobalEnvironmentVariableCollection, // EnvironmentVariableScope, + EnvironmentVariableCollection, // --- End Positron --- EnvironmentVariableMutatorOptions, ProgressLocation, - EnvironmentVariableCollection, } from 'vscode'; import { pathExists, normCase } from '../../common/platform/fs-paths'; import { IExtensionActivationService } from '../../activation/types'; @@ -412,13 +412,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ // TODO: Stop prepending altogether once https://github.com/microsoft/vscode/issues/145234 is available. return isActive ? { - applyAtShellIntegration: true, - applyAtProcessCreation: false, - } + applyAtShellIntegration: true, + applyAtProcessCreation: false, + } : { - applyAtShellIntegration: true, // Takes care of false negatives in case manual integration is being used. - applyAtProcessCreation: true, - }; + applyAtShellIntegration: true, // Takes care of false negatives in case manual integration is being used. + applyAtProcessCreation: true, + }; } // --- Start Positron --- diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 11542578270..0c1a18b2aef 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -82,7 +82,7 @@ suite('Terminal Environment Variable Collection Service', () => { collection = mock(); // --- Start Positron --- when(context.environmentVariableCollection as EnvironmentVariableCollection).thenReturn(instance(collection)); - //when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); + // when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); // --- End Positron --- experimentService = mock(); when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true); From 2072c9f2590adee7b8098bfa3d1923f63c4dfa30 Mon Sep 17 00:00:00 2001 From: isabelizimm Date: Wed, 13 Nov 2024 12:41:16 -0500 Subject: [PATCH 3/5] update tests to pass preCompile --- .../terminalEnvVarCollectionService.unit.test.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 0c1a18b2aef..e7cc85606d6 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -9,7 +9,9 @@ import { mock, instance, when, anything, verify, reset } from 'ts-mockito'; import { EnvironmentVariableCollection, EnvironmentVariableMutatorOptions, - GlobalEnvironmentVariableCollection, + // --- Start Positron --- + // GlobalEnvironmentVariableCollection, + // --- End Positron --- ProgressLocation, Uri, WorkspaceFolder, @@ -47,7 +49,9 @@ suite('Terminal Environment Variable Collection Service', () => { let shell: IApplicationShell; let experimentService: IExperimentService; let collection: EnvironmentVariableCollection; - let globalCollection: GlobalEnvironmentVariableCollection; + // --- Start Positron --- + //let globalCollection: GlobalEnvironmentVariableCollection; + // --- End Positron --- let applicationEnvironment: IApplicationEnvironment; let environmentActivationService: IEnvironmentActivationService; let workspaceService: IWorkspaceService; @@ -78,9 +82,9 @@ suite('Terminal Environment Variable Collection Service', () => { const envVarProvider = mock(); shellIntegrationService = mock(); when(shellIntegrationService.isWorking()).thenResolve(true); - globalCollection = mock(); - collection = mock(); // --- Start Positron --- + //globalCollection = mock(); + collection = mock(); when(context.environmentVariableCollection as EnvironmentVariableCollection).thenReturn(instance(collection)); // when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); // --- End Positron --- @@ -144,7 +148,7 @@ suite('Terminal Environment Variable Collection Service', () => { reset(experimentService); // --- Start Positron --- // Use globalCollection instead of collection since we're using a later version of @types/vscode - when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); + //when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); // --- End Positron --- when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); From b2ef83708d6a3144a3226555cd6dc178cf3bf51e Mon Sep 17 00:00:00 2001 From: isabelizimm Date: Wed, 13 Nov 2024 13:01:53 -0500 Subject: [PATCH 4/5] relint after merge --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index bc8e026ce28..37a237dd6b3 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -50,7 +50,7 @@ suite('Terminal Environment Variable Collection Service', () => { let experimentService: IExperimentService; let collection: EnvironmentVariableCollection; // --- Start Positron --- - //let globalCollection: GlobalEnvironmentVariableCollection; + // let globalCollection: GlobalEnvironmentVariableCollection; // --- End Positron --- let applicationEnvironment: IApplicationEnvironment; let environmentActivationService: IEnvironmentActivationService; @@ -83,7 +83,7 @@ suite('Terminal Environment Variable Collection Service', () => { shellIntegrationService = mock(); when(shellIntegrationService.isWorking()).thenResolve(true); // --- Start Positron --- - //globalCollection = mock(); + // globalCollection = mock(); collection = mock(); when(context.environmentVariableCollection as EnvironmentVariableCollection).thenReturn(instance(collection)); // when(globalCollection.getScoped(anything())).thenReturn(instance(collection)); From 34fa70d2f72f432c6844b931093eed53378b1cc3 Mon Sep 17 00:00:00 2001 From: isabelizimm Date: Wed, 13 Nov 2024 14:05:33 -0500 Subject: [PATCH 5/5] remove incorrect comment --- .../activation/terminalEnvVarCollectionService.unit.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts index 37a237dd6b3..3a08d6fcd33 100644 --- a/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts +++ b/extensions/positron-python/src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts @@ -146,10 +146,6 @@ suite('Terminal Environment Variable Collection Service', () => { // --- End Positron --- reset(experimentService); - // --- Start Positron --- - // Use globalCollection instead of collection since we're using a later version of @types/vscode - // when(context.environmentVariableCollection).thenReturn(instance(globalCollection)); - // --- End Positron --- when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(false); const applyCollectionStub = sinon.stub(terminalEnvVarCollectionService, '_applyCollection'); applyCollectionStub.resolves();