Update proposed API for env collection

package.json

@@ -46,7 +46,7 @@
         "theme": "dark"
     },
     "engines": {
-        "vscode": "^1.82.0-20230809"
+        "vscode": "^1.82.0-20230823"
     },
     "enableTelemetry": false,
     "keywords": [

src/client/interpreter/activation/terminalEnvVarCollectionService.ts

@@ -3,7 +3,14 @@
 
 import * as path from 'path';
 import { inject, injectable } from 'inversify';
-import { ProgressOptions, ProgressLocation, MarkdownString, WorkspaceFolder } from 'vscode';
+import {
+    ProgressOptions,
+    ProgressLocation,
+    MarkdownString,
+    WorkspaceFolder,
+    GlobalEnvironmentVariableCollection,
+    EnvironmentVariableScope,
+} from 'vscode';
 import { pathExists } from 'fs-extra';
 import { IExtensionActivationService } from '../../activation/types';
 import { IApplicationShell, IApplicationEnvironment, IWorkspaceService } from '../../common/application/types';
@@ -20,7 +27,7 @@ import {
 } from '../../common/types';
 import { Deferred, createDeferred } from '../../common/utils/async';
 import { Interpreters } from '../../common/utils/localize';
-import { traceDecoratorVerbose, traceVerbose, traceWarn } from '../../logging';
+import { traceDecoratorVerbose, traceError, traceVerbose, traceWarn } from '../../logging';
 import { IInterpreterService } from '../contracts';
 import { defaultShells } from './service';
 import { IEnvironmentActivationService, ITerminalEnvVarCollectionService } from './types';
@@ -61,52 +68,56 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
     ) {}
 
     public async activate(resource: Resource): Promise<void> {
-        if (!inTerminalEnvVarExperiment(this.experimentService)) {
-            this.context.environmentVariableCollection.clear();
-            await this.handleMicroVenv(resource);
+        try {
+            if (!inTerminalEnvVarExperiment(this.experimentService)) {
+                this.context.environmentVariableCollection.clear();
+                await this.handleMicroVenv(resource);
+                if (!this.registeredOnce) {
+                    this.interpreterService.onDidChangeInterpreter(
+                        async (r) => {
+                            await this.handleMicroVenv(r);
+                        },
+                        this,
+                        this.disposables,
+                    );
+                    this.registeredOnce = true;
+                }
+                return;
+            }
             if (!this.registeredOnce) {
                 this.interpreterService.onDidChangeInterpreter(
                     async (r) => {
-                        await this.handleMicroVenv(r);
+                        this.showProgress();
+                        await this._applyCollection(r).ignoreErrors();
+                        this.hideProgress();
+                    },
+                    this,
+                    this.disposables,
+                );
+                this.applicationEnvironment.onDidChangeShell(
+                    async (shell: string) => {
+                        this.showProgress();
+                        this.processEnvVars = undefined;
+                        // Pass in the shell where known instead of relying on the application environment, because of bug
+                        // on VSCode: https://github.com/microsoft/vscode/issues/160694
+                        await this._applyCollection(undefined, shell).ignoreErrors();
+                        this.hideProgress();
                     },
                     this,
                     this.disposables,
                 );
                 this.registeredOnce = true;
             }
-            return;
-        }
-        if (!this.registeredOnce) {
-            this.interpreterService.onDidChangeInterpreter(
-                async (r) => {
-                    this.showProgress();
-                    await this._applyCollection(r).ignoreErrors();
-                    this.hideProgress();
-                },
-                this,
-                this.disposables,
-            );
-            this.applicationEnvironment.onDidChangeShell(
-                async (shell: string) => {
-                    this.showProgress();
-                    this.processEnvVars = undefined;
-                    // Pass in the shell where known instead of relying on the application environment, because of bug
-                    // on VSCode: https://github.com/microsoft/vscode/issues/160694
-                    await this._applyCollection(undefined, shell).ignoreErrors();
-                    this.hideProgress();
-                },
-                this,
-                this.disposables,
-            );
-            this.registeredOnce = true;
+            this._applyCollection(resource).ignoreErrors();
+        } catch (ex) {
+            traceError(`Activating terminal env collection failed`, ex);
         }
-        this._applyCollection(resource).ignoreErrors();
     }
 
     public async _applyCollection(resource: Resource, shell = this.applicationEnvironment.shell): Promise<void> {
         const workspaceFolder = this.getWorkspaceFolder(resource);
         const settings = this.configurationService.getSettings(resource);
-        const envVarCollection = this.context.getEnvironmentVariableCollection({ workspaceFolder });
+        const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
         // Clear any previously set env vars from collection
         envVarCollection.clear();
         if (!settings.terminal.activateEnvironment) {
@@ -221,6 +232,13 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                 // PS1 should be set but no PS1 was set.
                 return;
             }
+            const config = this.workspaceService
+                .getConfiguration('terminal')
+                .get<boolean>('integrated.shellIntegration.enabled');
+            if (!config) {
+                traceVerbose('PS1 is not set when shell integration is disabled.');
+                return;
+            }
         }
         this.terminalPromptIsCorrect(resource);
     }
@@ -232,7 +250,7 @@ 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.context.getEnvironmentVariableCollection({ workspaceFolder });
+                    const envVarCollection = this.getEnvironmentVariableCollection({ workspaceFolder });
                     const pathVarName = getSearchPathEnvVarNames()[0];
                     envVarCollection.replace(
                         'PATH',
@@ -241,13 +259,18 @@ export class TerminalEnvVarCollectionService implements IExtensionActivationServ
                     );
                     return;
                 }
-                this.context.getEnvironmentVariableCollection({ workspaceFolder }).clear();
+                this.getEnvironmentVariableCollection({ workspaceFolder }).clear();
             }
         } catch (ex) {
             traceWarn(`Microvenv failed as it is using proposed API which is constantly changing`, ex);
         }
     }
 
+    private getEnvironmentVariableCollection(scope: EnvironmentVariableScope = {}) {
+        const envVarCollection = this.context.environmentVariableCollection as GlobalEnvironmentVariableCollection;
+        return envVarCollection.getScoped(scope);
+    }
+
     private getWorkspaceFolder(resource: Resource): WorkspaceFolder | undefined {
         let workspaceFolder = this.workspaceService.getWorkspaceFolder(resource);
         if (

src/test/interpreters/activation/terminalEnvVarCollectionService.unit.test.ts

@@ -9,9 +9,10 @@ import { mock, instance, when, anything, verify, reset } from 'ts-mockito';
 import {
     EnvironmentVariableCollection,
     EnvironmentVariableMutatorOptions,
-    EnvironmentVariableScope,
+    GlobalEnvironmentVariableCollection,
     ProgressLocation,
     Uri,
+    WorkspaceConfiguration,
     WorkspaceFolder,
 } from 'vscode';
 import {
@@ -44,12 +45,12 @@ suite('Terminal Environment Variable Collection Service', () => {
     let context: IExtensionContext;
     let shell: IApplicationShell;
     let experimentService: IExperimentService;
-    let collection: EnvironmentVariableCollection & {
-        getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
-    };
+    let collection: EnvironmentVariableCollection;
+    let globalCollection: GlobalEnvironmentVariableCollection;
     let applicationEnvironment: IApplicationEnvironment;
     let environmentActivationService: IEnvironmentActivationService;
     let workspaceService: IWorkspaceService;
+    let workspaceConfig: WorkspaceConfiguration;
     let terminalEnvVarCollectionService: TerminalEnvVarCollectionService;
     const progressOptions = {
         location: ProgressLocation.Window,
@@ -62,19 +63,20 @@ suite('Terminal Environment Variable Collection Service', () => {
 
     setup(() => {
         workspaceService = mock<IWorkspaceService>();
+        workspaceConfig = mock<WorkspaceConfiguration>();
         when(workspaceService.getWorkspaceFolder(anything())).thenReturn(undefined);
         when(workspaceService.workspaceFolders).thenReturn(undefined);
+        when(workspaceService.getConfiguration('terminal')).thenReturn(instance(workspaceConfig));
+        when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(true);
         platform = mock<IPlatformService>();
         when(platform.osType).thenReturn(getOSType());
         interpreterService = mock<IInterpreterService>();
         context = mock<IExtensionContext>();
         shell = mock<IApplicationShell>();
-        collection = mock<
-            EnvironmentVariableCollection & {
-                getScopedEnvironmentVariableCollection(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
-            }
-        >();
-        when(context.getEnvironmentVariableCollection(anything())).thenReturn(instance(collection));
+        globalCollection = mock<GlobalEnvironmentVariableCollection>();
+        collection = mock<EnvironmentVariableCollection>();
+        when(context.environmentVariableCollection).thenReturn(instance(globalCollection));
+        when(globalCollection.getScoped(anything())).thenReturn(instance(collection));
         experimentService = mock<IExperimentService>();
         when(experimentService.inExperimentSync(TerminalEnvVarActivation.experiment)).thenReturn(true);
         applicationEnvironment = mock<IApplicationEnvironment>();
@@ -291,6 +293,34 @@ suite('Terminal Environment Variable Collection Service', () => {
         expect(result).to.equal(true);
     });
 
+    test('Correct track that prompt was set for PS1 if shell integration is disabled', async () => {
+        reset(workspaceConfig);
+        when(workspaceConfig.get<boolean>('integrated.shellIntegration.enabled')).thenReturn(false);
+        when(platform.osType).thenReturn(OSType.Linux);
+        const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };
+        const ps1Shell = 'bash';
+        const resource = Uri.file('a');
+        const workspaceFolder: WorkspaceFolder = {
+            uri: Uri.file('workspacePath'),
+            name: 'workspace1',
+            index: 0,
+        };
+        when(interpreterService.getActiveInterpreter(resource)).thenResolve(({
+            type: PythonEnvType.Virtual,
+        } as unknown) as PythonEnvironment);
+        when(workspaceService.getWorkspaceFolder(resource)).thenReturn(workspaceFolder);
+        when(
+            environmentActivationService.getActivatedEnvironmentVariables(resource, undefined, undefined, ps1Shell),
+        ).thenResolve(envVars);
+        when(collection.replace(anything(), anything(), anything())).thenReturn();
+
+        await terminalEnvVarCollectionService._applyCollection(resource, ps1Shell);
+
+        const result = terminalEnvVarCollectionService.isTerminalPromptSetCorrectly(resource);
+
+        expect(result).to.equal(false);
+    });
+
     test('Correct track that prompt was not set for non-Windows zsh where PS1 is set', async () => {
         when(platform.osType).thenReturn(OSType.Linux);
         const envVars: NodeJS.ProcessEnv = { VIRTUAL_ENV: 'prefix/to/venv', PS1: '(.venv)', ...process.env };

types/vscode.proposed.envCollectionWorkspace.d.ts

@@ -4,30 +4,34 @@
  *--------------------------------------------------------------------------------------------*/
 
 declare module 'vscode' {
+    // https://github.com/microsoft/vscode/issues/171173
 
-	// https://github.com/microsoft/vscode/issues/182069
+    // export interface ExtensionContext {
+    //     /**
+    //      * Gets the extension's global environment variable collection for this workspace, enabling changes to be
+    //      * applied to terminal environment variables.
+    //      */
+    //     readonly environmentVariableCollection: GlobalEnvironmentVariableCollection;
+    // }
 
-	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 interface GlobalEnvironmentVariableCollection extends EnvironmentVariableCollection {
+        /**
+         * Gets scope-specific environment variable collection for the extension. This enables alterations to
+         * terminal environment variables solely within the designated scope, and is applied in addition to (and
+         * after) the global collection.
+         *
+         * Each object obtained through this method is isolated and does not impact objects for other scopes,
+         * including the global collection.
+         *
+         * @param scope The scope to which the environment variable collection applies to.
+         */
+        getScoped(scope: EnvironmentVariableScope): EnvironmentVariableCollection;
+    }
 
-	export type EnvironmentVariableScope = {
-		/**
-		* Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
-		*/
-		workspaceFolder?: WorkspaceFolder;
-	};
+    export type EnvironmentVariableScope = {
+        /**
+         * Any specific workspace folder to get collection for. If unspecified, collection applicable to all workspace folders is returned.
+         */
+        workspaceFolder?: WorkspaceFolder;
+    };
 }