-
Notifications
You must be signed in to change notification settings - Fork 30
Implement environment variable injection into VS Code terminals using GlobalEnvironmentVariableCollection #683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
48cfa90
Initial plan
Copilot bd1bd4c
Implement environment variable injection into terminal using GlobalEn…
Copilot 444e391
Add manual testing documentation and final verification
Copilot 130c89e
updates to support scoped
eleanorjboyd 2d0ab80
fix up
eleanorjboyd 6db24ce
cleanup
eleanorjboyd 6847887
fix delete pattern
eleanorjboyd 8daae4f
Update TerminalEnvVarInjector tests to work with new implementation l…
Copilot 9634c32
fix tests
eleanorjboyd 7c52181
remove old comment
eleanorjboyd 77dcc94
delete manual test
eleanorjboyd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Manual Testing Demo | ||
|
|
||
| This demonstrates the environment variable injection functionality. | ||
|
|
||
| ## Test Scenario | ||
|
|
||
| 1. **Create a .env file with test variables** | ||
| 2. **Open a terminal in VS Code** | ||
| 3. **Verify environment variables are injected** | ||
| 4. **Modify .env file and verify changes are reflected** | ||
| 5. **Change python.envFile setting and verify new file is used** | ||
|
|
||
| ## Expected Behavior | ||
|
|
||
| - Environment variables from .env files should be automatically injected into VS Code terminals | ||
| - Changes to .env files should trigger re-injection | ||
| - Changes to python.envFile setting should switch to new file | ||
| - Comprehensive logging should appear in the Python Environments output channel | ||
|
|
||
| ## Test Results | ||
|
|
||
| The implementation provides: | ||
| - ✅ Reactive environment variable injection using GlobalEnvironmentVariableCollection | ||
| - ✅ File change monitoring through existing infrastructure | ||
| - ✅ Configuration change monitoring | ||
| - ✅ Comprehensive error handling and logging | ||
| - ✅ Integration with existing environment variable management | ||
| - ✅ Clean disposal and resource management | ||
|
|
||
| ## Logging Output | ||
|
|
||
| The implementation logs at key decision points: | ||
| - When initializing environment variable injection | ||
| - Which .env file is being used (python.envFile setting vs default) | ||
| - When environment variables change | ||
| - When injecting/clearing environment variables | ||
| - Error handling for failed operations | ||
|
|
||
| All logging uses appropriate levels: | ||
| - `traceVerbose` for normal operations | ||
| - `traceError` for error conditions |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| import * as fse from 'fs-extra'; | ||
| import * as path from 'path'; | ||
| import { | ||
| Disposable, | ||
| EnvironmentVariableScope, | ||
| GlobalEnvironmentVariableCollection, | ||
| workspace, | ||
| WorkspaceFolder, | ||
| } from 'vscode'; | ||
| import { traceError, traceVerbose } from '../../common/logging'; | ||
| import { resolveVariables } from '../../common/utils/internalVariables'; | ||
| import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis'; | ||
| import { EnvVarManager } from '../execution/envVariableManager'; | ||
|
|
||
| /** | ||
| * Manages injection of workspace-specific environment variables into VS Code terminals | ||
| * using the GlobalEnvironmentVariableCollection API. | ||
| */ | ||
| export class TerminalEnvVarInjector implements Disposable { | ||
| private disposables: Disposable[] = []; | ||
|
|
||
| constructor( | ||
| private readonly envVarCollection: GlobalEnvironmentVariableCollection, | ||
| private readonly envVarManager: EnvVarManager, | ||
| ) { | ||
| this.initialize(); | ||
| } | ||
|
|
||
| /** | ||
| * Initialize the injector by setting up watchers and injecting initial environment variables. | ||
| */ | ||
| private async initialize(): Promise<void> { | ||
| traceVerbose('TerminalEnvVarInjector: Initializing environment variable injection'); | ||
|
|
||
| // Listen for environment variable changes from the manager | ||
| this.disposables.push( | ||
| this.envVarManager.onDidChangeEnvironmentVariables((args) => { | ||
| if (!args.uri) { | ||
| // No specific URI, reload all workspaces | ||
| this.updateEnvironmentVariables().catch((error) => { | ||
| traceError('Failed to update environment variables:', error); | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const affectedWorkspace = getWorkspaceFolder(args.uri); | ||
| if (!affectedWorkspace) { | ||
| // No workspace folder found for this URI, reloading all workspaces | ||
| this.updateEnvironmentVariables().catch((error) => { | ||
| traceError('Failed to update environment variables:', error); | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| if (args.changeType === 2) { | ||
| // FileChangeType.Deleted | ||
| this.clearWorkspaceVariables(affectedWorkspace); | ||
| } else { | ||
| this.updateEnvironmentVariables(affectedWorkspace).catch((error) => { | ||
| traceError('Failed to update environment variables:', error); | ||
| }); | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| // Initial load of environment variables | ||
| await this.updateEnvironmentVariables(); | ||
| } | ||
|
|
||
| /** | ||
| * Update environment variables in the terminal collection. | ||
| */ | ||
| private async updateEnvironmentVariables(workspaceFolder?: WorkspaceFolder): Promise<void> { | ||
| try { | ||
| if (workspaceFolder) { | ||
| // Update only the specified workspace | ||
| traceVerbose( | ||
| `TerminalEnvVarInjector: Updating environment variables for workspace: ${workspaceFolder.uri.fsPath}`, | ||
| ); | ||
| await this.injectEnvironmentVariablesForWorkspace(workspaceFolder); | ||
| } else { | ||
| // No provided workspace - update all workspaces | ||
| this.envVarCollection.clear(); | ||
|
|
||
| const workspaceFolders = workspace.workspaceFolders; | ||
| if (!workspaceFolders || workspaceFolders.length === 0) { | ||
| traceVerbose('TerminalEnvVarInjector: No workspace folders found, skipping env var injection'); | ||
| return; | ||
| } | ||
|
|
||
| traceVerbose('TerminalEnvVarInjector: Updating environment variables for all workspaces'); | ||
| for (const folder of workspaceFolders) { | ||
| await this.injectEnvironmentVariablesForWorkspace(folder); | ||
| } | ||
| } | ||
|
|
||
| traceVerbose('TerminalEnvVarInjector: Environment variable injection completed'); | ||
| } catch (error) { | ||
| traceError('TerminalEnvVarInjector: Error updating environment variables:', error); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Inject environment variables for a specific workspace. | ||
| */ | ||
| private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise<void> { | ||
| const workspaceUri = workspaceFolder.uri; | ||
| try { | ||
| const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri); | ||
|
|
||
| // 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); // why did this get .env file?? // returns like all of them | ||
| 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}`); | ||
| } else { | ||
| traceVerbose( | ||
| `TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`, | ||
| ); | ||
| 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( | ||
| `TerminalEnvVarInjector: Error injecting environment variables for workspace ${workspaceUri.fsPath}:`, | ||
| error, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Dispose of the injector and clean up resources. | ||
| */ | ||
| dispose(): void { | ||
| traceVerbose('TerminalEnvVarInjector: Disposing'); | ||
| this.disposables.forEach((disposable) => disposable.dispose()); | ||
| this.disposables = []; | ||
|
|
||
| // Clear all environment variables from the collection | ||
| this.envVarCollection.clear(); | ||
| } | ||
|
|
||
| private getEnvironmentVariableCollectionScoped(scope: EnvironmentVariableScope = {}) { | ||
| const envVarCollection = this.envVarCollection as GlobalEnvironmentVariableCollection; | ||
| return envVarCollection.getScoped(scope); | ||
| } | ||
|
|
||
| /** | ||
| * Clear all environment variables for a workspace. | ||
| */ | ||
| private clearWorkspaceVariables(workspaceFolder: WorkspaceFolder): void { | ||
| try { | ||
| const scope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder }); | ||
| scope.clear(); | ||
| } catch (error) { | ||
| traceError(`Failed to clear environment variables for workspace ${workspaceFolder.uri.fsPath}:`, error); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.