diff --git a/package.json b/package.json index eb6cb461..31887629 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,12 @@ "%python-envs.terminal.autoActivationType.off%" ], "scope": "machine" + }, + "python.terminal.useEnvFile": { + "type": "boolean", + "description": "%python-envs.terminal.useEnvFile.description%", + "default": false, + "scope": "resource" } } }, diff --git a/package.nls.json b/package.nls.json index d7461323..2f55796f 100644 --- a/package.nls.json +++ b/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" } diff --git a/src/features/terminal/terminalEnvVarInjector.ts b/src/features/terminal/terminalEnvVarInjector.ts index 4bf48039..12a95a79 100644 --- a/src/features/terminal/terminalEnvVarInjector.ts +++ b/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,8 @@ 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'; /** @@ -21,6 +23,7 @@ import { EnvVarManager } from '../execution/envVariableManager'; */ export class TerminalEnvVarInjector implements Disposable { private disposables: Disposable[] = []; + private readonly previousEnvFileState = new Map(); constructor( private readonly envVarCollection: GlobalEnvironmentVariableCollection, @@ -35,6 +38,21 @@ export class TerminalEnvVarInjector implements Disposable { private async initialize(): Promise { traceVerbose('TerminalEnvVarInjector: Initializing environment variable injection'); + // Initialize previous envFile state for all workspaces + const workspaceFolders = workspace.workspaceFolders; + if (workspaceFolders) { + for (const folder of workspaceFolders) { + this.updatePreviousEnvFileState(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) => { @@ -70,6 +88,81 @@ export class TerminalEnvVarInjector implements Disposable { await this.updateEnvironmentVariables(); } + /** + * Handle configuration changes and show notifications when python.envFile is set. + */ + private handleConfigurationChange(event: ConfigurationChangeEvent): void { + const workspaceFolders = workspace.workspaceFolders; + if (!workspaceFolders) { + return; + } + + for (const folder of workspaceFolders) { + if (event.affectsConfiguration('python.envFile', folder.uri)) { + + const folderKey = folder.uri.toString(); + const previousEnvFile = this.previousEnvFileState.get(folderKey); + const currentEnvFile = this.getCurrentEnvFile(folder); + + // Show notification if envFile was just set and useEnvFile is not true + if (!previousEnvFile && currentEnvFile && !this.getCurrentUseEnvFile(folder)) { + this.showEnvFileSetNotification(); + } + + this.previousEnvFileState.set(folderKey, currentEnvFile); + } + + // Still need to update environment variables when either setting changes + if (event.affectsConfiguration('python.terminal.useEnvFile', folder.uri) || + event.affectsConfiguration('python.envFile', folder.uri)) { + + this.updateEnvironmentVariables(folder).catch((error) => { + traceError('Failed to update environment variables after configuration change:', error); + }); + } + } + } + + /** + * Get current envFile setting for a workspace. + */ + private getCurrentEnvFile(workspaceFolder: WorkspaceFolder): string | undefined { + const config = getConfiguration('python', workspaceFolder.uri); + return config.get('envFile'); + } + + /** + * Get current useEnvFile setting for a workspace. + */ + private getCurrentUseEnvFile(workspaceFolder: WorkspaceFolder): boolean { + const config = getConfiguration('python', workspaceFolder.uri); + return config.get('terminal.useEnvFile', false); + } + + /** + * Update the previous envFile state for a workspace. + */ + private updatePreviousEnvFileState(workspaceFolder: WorkspaceFolder): void { + const folderKey = workspaceFolder.uri.toString(); + this.previousEnvFileState.set(folderKey, this.getCurrentEnvFile(workspaceFolder)); + } + + /** + * Show notification when envFile is set but useEnvFile is not enabled. + */ + private showEnvFileSetNotification(): void { + const 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.'; + + showInformationMessage(message, 'Open Settings').then((selection) => { + if (selection === 'Open Settings') { + // Open VS Code settings to the useEnvFile setting + import('vscode').then(vscode => { + vscode.commands.executeCommand('workbench.action.openSettings', 'python.terminal.useEnvFile'); + }); + } + }); + } + /** * Update environment variables in the terminal collection. */ @@ -109,37 +202,50 @@ export class TerminalEnvVarInjector implements Disposable { private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise { const workspaceUri = workspaceFolder.uri; try { - const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri); - + // Check if environment variable injection is enabled + const config = getConfiguration('python', workspaceUri); + const useEnvFile = config.get('terminal.useEnvFile', false); + // use scoped environment variable collection const envVarScope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder }); envVarScope.clear(); // Clear existing variables for this workspace - // Track which .env file is being used for logging - const config = getConfiguration('python', workspaceUri); - const envFilePath = config.get('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}`); - } else { + // Only inject if useEnvFile is true + if (useEnvFile) { traceVerbose( - `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`, + `TerminalEnvVarInjector: Environment variable injection enabled 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); + const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri); + + // Track which .env file is being used for logging + const envFilePath = config.get('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}`); + + 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 { - envVarScope.replace(key, value); + traceVerbose( + `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`, + ); } + } else { + traceVerbose( + `TerminalEnvVarInjector: Environment variable injection disabled for workspace: ${workspaceUri.fsPath}`, + ); } } catch (error) { traceError( @@ -154,7 +260,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 diff --git a/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts b/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts index 4b009e99..bc3acfe1 100644 --- a/src/test/features/terminalEnvVarInjectorBasic.unit.test.ts +++ b/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(); @@ -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)