Implement configuration change listeners and smart notifications for env settings

Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>

Author: Copilot

package.json

@@ -106,7 +106,7 @@
                 },
                 "python.terminal.useEnvFile": {
                     "type": "boolean",
-                    "description": "Controls whether environment variables from .env files and python.envFile setting are injected into terminals",
+                    "description": "%python-envs.terminal.useEnvFile.description%",
                     "default": false,
                     "scope": "resource"
                 }

package.nls.json

@@ -36,5 +36,6 @@
     "python-envs.terminal.deactivate.title": "Deactivate Environment in Current Terminal",
     "python-envs.uninstallPackage.title": "Uninstall Package",
     "python-envs.revealProjectInExplorer.title": "Reveal Project in Explorer",
-    "python-envs.runPetInTerminal.title": "Run Python Environment Tool (PET) in Terminal"
+    "python-envs.runPetInTerminal.title": "Run Python Environment Tool (PET) in Terminal",
+    "python-envs.terminal.useEnvFile.description": "Controls whether environment variables from .env files and python.envFile setting are injected into terminals"
 }

src/features/terminal/terminalEnvVarInjector.ts

@@ -4,6 +4,7 @@
 import * as fse from 'fs-extra';
 import * as path from 'path';
 import {
+    ConfigurationChangeEvent,
     Disposable,
     EnvironmentVariableScope,
     GlobalEnvironmentVariableCollection,
@@ -12,7 +13,7 @@ import {
 } from 'vscode';
 import { traceError, traceVerbose } from '../../common/logging';
 import { resolveVariables } from '../../common/utils/internalVariables';
-import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
+import { getConfiguration, getWorkspaceFolder, onDidChangeConfiguration } from '../../common/workspace.apis';
 import { showInformationMessage } from '../../common/window.apis';
 import { EnvVarManager } from '../execution/envVariableManager';
 
@@ -22,6 +23,7 @@ import { EnvVarManager } from '../execution/envVariableManager';
  */
 export class TerminalEnvVarInjector implements Disposable {
     private disposables: Disposable[] = [];
+    private readonly previousSettingsState = new Map<string, { useEnvFile: boolean | undefined; envFile: string | undefined }>();
 
     constructor(
         private readonly envVarCollection: GlobalEnvironmentVariableCollection,
@@ -36,6 +38,21 @@ export class TerminalEnvVarInjector implements Disposable {
     private async initialize(): Promise<void> {
         traceVerbose('TerminalEnvVarInjector: Initializing environment variable injection');
 
+        // Initialize previous settings state for all workspaces
+        const workspaceFolders = workspace.workspaceFolders;
+        if (workspaceFolders) {
+            for (const folder of workspaceFolders) {
+                this.updatePreviousSettingsState(folder);
+            }
+        }
+
+        // Listen for configuration changes to show notifications when settings change
+        this.disposables.push(
+            onDidChangeConfiguration((event: ConfigurationChangeEvent) => {
+                this.handleConfigurationChange(event);
+            }),
+        );
+
         // Listen for environment variable changes from the manager
         this.disposables.push(
             this.envVarManager.onDidChangeEnvironmentVariables((args) => {
@@ -71,6 +88,109 @@ export class TerminalEnvVarInjector implements Disposable {
         await this.updateEnvironmentVariables();
     }
 
+    /**
+     * Handle configuration changes and show notifications when relevant settings change.
+     */
+    private handleConfigurationChange(event: ConfigurationChangeEvent): void {
+        const workspaceFolders = workspace.workspaceFolders;
+        if (!workspaceFolders) {
+            return;
+        }
+
+        for (const folder of workspaceFolders) {
+            if (event.affectsConfiguration('python.terminal.useEnvFile', folder.uri) ||
+                event.affectsConfiguration('python.envFile', folder.uri)) {
+                
+                const folderKey = folder.uri.toString();
+                const previousState = this.previousSettingsState.get(folderKey);
+                const currentState = this.getCurrentSettingsState(folder);
+
+                if (previousState && this.shouldShowNotification(previousState, currentState)) {
+                    this.showSettingsChangeNotification(currentState);
+                }
+
+                this.previousSettingsState.set(folderKey, currentState);
+                
+                // Update environment variables for this workspace
+                this.updateEnvironmentVariables(folder).catch((error) => {
+                    traceError('Failed to update environment variables after configuration change:', error);
+                });
+            }
+        }
+    }
+
+    /**
+     * Get current settings state for a workspace.
+     */
+    private getCurrentSettingsState(workspaceFolder: WorkspaceFolder): { useEnvFile: boolean | undefined; envFile: string | undefined } {
+        const config = getConfiguration('python', workspaceFolder.uri);
+        return {
+            useEnvFile: config.get<boolean>('terminal.useEnvFile'),
+            envFile: config.get<string>('envFile')
+        };
+    }
+
+    /**
+     * Update the previous settings state for a workspace.
+     */
+    private updatePreviousSettingsState(workspaceFolder: WorkspaceFolder): void {
+        const folderKey = workspaceFolder.uri.toString();
+        this.previousSettingsState.set(folderKey, this.getCurrentSettingsState(workspaceFolder));
+    }
+
+    /**
+     * Determine if we should show a notification based on settings changes.
+     */
+    private shouldShowNotification(
+        previousState: { useEnvFile: boolean | undefined; envFile: string | undefined },
+        currentState: { useEnvFile: boolean | undefined; envFile: string | undefined }
+    ): boolean {
+        // Show notification if:
+        // 1. useEnvFile changed from undefined/false to true
+        // 2. envFile changed from undefined to set (any string value)
+        // And ensure both settings are configured correctly
+
+        const useEnvFileChanged = !previousState.useEnvFile && currentState.useEnvFile;
+        const envFileChanged = !previousState.envFile && !!currentState.envFile;
+
+        if (useEnvFileChanged || envFileChanged) {
+            // If one is set but the other isn't, show notification
+            if ((currentState.envFile && !currentState.useEnvFile) || 
+                (currentState.useEnvFile && !currentState.envFile)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Show notification about settings configuration.
+     */
+    private showSettingsChangeNotification(currentState: { useEnvFile: boolean | undefined; envFile: string | undefined }): void {
+        let message: string;
+        
+        if (currentState.envFile && !currentState.useEnvFile) {
+            message = 'The python.envFile setting is configured but will not take effect in terminals. Enable the "python.terminal.useEnvFile" setting to use environment variables from .env files in terminals.';
+        } else if (currentState.useEnvFile && !currentState.envFile) {
+            message = 'The python.terminal.useEnvFile setting is enabled. Consider setting "python.envFile" to specify a custom .env file path, or ensure a .env file exists in your workspace root.';
+        } else {
+            return; // Both are properly configured, no notification needed
+        }
+
+        showInformationMessage(message, 'Open Settings').then((selection) => {
+            if (selection === 'Open Settings') {
+                // Open VS Code settings to the relevant setting
+                import('vscode').then(vscode => {
+                    const settingToOpen = currentState.envFile && !currentState.useEnvFile 
+                        ? 'python.terminal.useEnvFile'
+                        : 'python.envFile';
+                    vscode.commands.executeCommand('workbench.action.openSettings', settingToOpen);
+                });
+            }
+        });
+    }
+
     /**
      * Update environment variables in the terminal collection.
      */
@@ -113,64 +233,47 @@ export class TerminalEnvVarInjector implements Disposable {
             // Check if environment variable injection is enabled
             const config = getConfiguration('python', workspaceUri);
             const useEnvFile = config.get<boolean>('terminal.useEnvFile', false);
-            const envFilePath = config.get<string>('envFile');
 
             // use scoped environment variable collection
             const envVarScope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder });
             envVarScope.clear(); // Clear existing variables for this workspace
 
-            // Check if python.envFile is set but useEnvFile is false (default) and show notification
-            if (!useEnvFile && envFilePath) {
+            // Only inject if useEnvFile is true
+            if (useEnvFile) {
                 traceVerbose(
-                    `TerminalEnvVarInjector: python.envFile is set but python.terminal.useEnvFile is false for workspace: ${workspaceUri.fsPath}`,
+                    `TerminalEnvVarInjector: Environment variable injection enabled for workspace: ${workspaceUri.fsPath}`,
                 );
-                
-                // Show information message to user
-                showInformationMessage(
-                    'The python.envFile setting is configured but will not take effect in terminals. Enable the "python.terminal.useEnvFile" setting to use environment variables from .env files in terminals.',
-                    'Open Settings'
-                ).then((selection) => {
-                    if (selection === 'Open Settings') {
-                        // Open VS Code settings to the python.terminal.useEnvFile setting
-                        import('vscode').then(vscode => {
-                            vscode.commands.executeCommand('workbench.action.openSettings', 'python.terminal.useEnvFile');
-                        });
-                    }
-                });
-            }
-
-            if (!useEnvFile) {
-                traceVerbose(
-                    `TerminalEnvVarInjector: Environment variable injection disabled for workspace: ${workspaceUri.fsPath}`,
-                );
-                return; // Injection is disabled
-            }
 
-            const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri);
+                const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri);
 
-            // Track which .env file is being used for logging
-            const resolvedEnvFilePath: string | undefined = envFilePath
-                ? path.resolve(resolveVariables(envFilePath, workspaceUri))
-                : undefined;
-            const defaultEnvFilePath: string = path.join(workspaceUri.fsPath, '.env');
+                // Track which .env file is being used for logging
+                const envFilePath = config.get<string>('envFile');
+                const resolvedEnvFilePath: string | undefined = envFilePath
+                    ? path.resolve(resolveVariables(envFilePath, workspaceUri))
+                    : undefined;
+                const defaultEnvFilePath: string = path.join(workspaceUri.fsPath, '.env');
 
-            let activeEnvFilePath: string = resolvedEnvFilePath || defaultEnvFilePath;
-            if (activeEnvFilePath && (await fse.pathExists(activeEnvFilePath))) {
-                traceVerbose(`TerminalEnvVarInjector: Using env file: ${activeEnvFilePath}`);
+                let activeEnvFilePath: string = resolvedEnvFilePath || defaultEnvFilePath;
+                if (activeEnvFilePath && (await fse.pathExists(activeEnvFilePath))) {
+                    traceVerbose(`TerminalEnvVarInjector: Using env file: ${activeEnvFilePath}`);
+                    
+                    for (const [key, value] of Object.entries(envVars)) {
+                        if (value === undefined) {
+                            // Remove the environment variable if the value is undefined
+                            envVarScope.delete(key);
+                        } else {
+                            envVarScope.replace(key, value);
+                        }
+                    }
+                } else {
+                    traceVerbose(
+                        `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`,
+                    );
+                }
             } else {
                 traceVerbose(
-                    `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`,
+                    `TerminalEnvVarInjector: Environment variable injection disabled for workspace: ${workspaceUri.fsPath}`,
                 );
-                return; // No .env file to inject
-            }
-
-            for (const [key, value] of Object.entries(envVars)) {
-                if (value === undefined) {
-                    // Remove the environment variable if the value is undefined
-                    envVarScope.delete(key);
-                } else {
-                    envVarScope.replace(key, value);
-                }
             }
         } catch (error) {
             traceError(
@@ -185,7 +288,7 @@ export class TerminalEnvVarInjector implements Disposable {
      */
     dispose(): void {
         traceVerbose('TerminalEnvVarInjector: Disposing');
-        this.disposables.forEach((disposable) => disposable.dispose());
+        this.disposables.forEach((disposable) => disposable?.dispose());
         this.disposables = [];
 
         // Clear all environment variables from the collection

src/test/features/terminalEnvVarInjectorBasic.unit.test.ts

@@ -3,9 +3,10 @@
 
 import * as sinon from 'sinon';
 import * as typeMoq from 'typemoq';
-import { GlobalEnvironmentVariableCollection, workspace } from 'vscode';
+import { GlobalEnvironmentVariableCollection, workspace, EnvironmentVariableCollection } from 'vscode';
 import { EnvVarManager } from '../../features/execution/envVariableManager';
 import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
+import * as workspaceApi from '../../common/workspace.apis';
 
 interface MockScopedCollection {
     clear: sinon.SinonStub;
@@ -20,6 +21,7 @@ suite('TerminalEnvVarInjector Basic Tests', () => {
     let mockScopedCollection: MockScopedCollection;
     // eslint-disable-next-line @typescript-eslint/no-explicit-any
     let workspaceFoldersStub: any;
+    let onDidChangeConfigurationStub: sinon.SinonStub;
 
     setup(() => {
         envVarCollection = typeMoq.Mock.ofType<GlobalEnvironmentVariableCollection>();
@@ -40,9 +42,15 @@ suite('TerminalEnvVarInjector Basic Tests', () => {
         };
 
         // Setup environment variable collection to return scoped collection
-        envVarCollection.setup((x) => x.getScoped(typeMoq.It.isAny())).returns(() => mockScopedCollection as any);
+        envVarCollection.setup((x) => x.getScoped(typeMoq.It.isAny())).returns(() => mockScopedCollection as unknown as EnvironmentVariableCollection);
         envVarCollection.setup((x) => x.clear()).returns(() => {});
 
+        // Mock onDidChangeConfiguration to return a disposable
+        onDidChangeConfigurationStub = sinon.stub(workspaceApi, 'onDidChangeConfiguration');
+        onDidChangeConfigurationStub.returns({
+            dispose: () => {}
+        });
+
         // Setup minimal mocks for event subscriptions
         envVarManager
             .setup((m) => m.onDidChangeEnvironmentVariables)