Skip to content

Commit 5e4a386

Browse files
Fix issue where terminal environment variables are not removed when they are commented out or deleted from .env files. (#1131)
Fixes #936 The problem this PR fixed is when users comment out or remove a variable from their .env file: ``` API_KEY=secret # DEBUG=true ← commented out ``` The DEBUG variable remained in terminal environments because: 1. `dotenv.parse()` ignores commented lines (seems the right behavior per dotenv spec) 2. The returned object only contained active variables: `{ API_KEY: 'secret' }` 3. The old code only iterated over keys in `envVars` and it never saw `DEBUG` 4. The `delete()` call was never reached for removed/commented variables Solution is to call `clear()` before re-injecting variables, which makes the fix stays at the terminal injection layer rather than modifying the parsing layer, avoiding impacting on other consumers. I also noticed some other issues with setting watcher, currently there's no file watcher watching the settings change, so a reload will be required when toggling the setting. We can consider adding a config listener. --------- Co-authored-by: Anthony Kim <62267334+anthonykim1@users.noreply.github.com>
1 parent 1f7b7ef commit 5e4a386

File tree

1 file changed

+45
-5
lines changed

1 file changed

+45
-5
lines changed

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import { EnvVarManager } from '../execution/envVariableManager';
2222
*/
2323
export class TerminalEnvVarInjector implements Disposable {
2424
private disposables: Disposable[] = [];
25+
// Track which .env variables we've set for each workspace to avoid clearing shell activation variables
26+
private envVarKeys: Map<string, Set<string>> = new Map();
2527

2628
constructor(
2729
private readonly envVarCollection: GlobalEnvironmentVariableCollection,
@@ -134,6 +136,8 @@ export class TerminalEnvVarInjector implements Disposable {
134136
*/
135137
private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise<void> {
136138
const workspaceUri = workspaceFolder.uri;
139+
const workspaceKey = workspaceUri.fsPath;
140+
137141
try {
138142
const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri);
139143

@@ -149,6 +153,8 @@ export class TerminalEnvVarInjector implements Disposable {
149153
traceVerbose(
150154
`TerminalEnvVarInjector: Env file injection disabled for workspace: ${workspaceUri.fsPath}`,
151155
);
156+
// Clear only the .env variables we previously set, not shell activation variables
157+
this.clearTrackedEnvVariables(envVarScope, workspaceKey);
152158
return;
153159
}
154160

@@ -165,17 +171,35 @@ export class TerminalEnvVarInjector implements Disposable {
165171
traceVerbose(
166172
`TerminalEnvVarInjector: No .env file found for workspace: ${workspaceUri.fsPath}, not injecting environment variables.`,
167173
);
168-
return; // No .env file to inject
174+
// Clear only the .env variables we previously set, not shell activation variables
175+
this.clearTrackedEnvVariables(envVarScope, workspaceKey);
176+
return;
169177
}
170178

171-
for (const [key, value] of Object.entries(envVars)) {
172-
if (value === undefined) {
173-
// Remove the environment variable if the value is undefined
179+
// Get previously tracked keys for this workspace
180+
const previousKeys = this.envVarKeys.get(workspaceKey) || new Set<string>();
181+
const currentKeys = new Set<string>();
182+
183+
// Delete variables that were previously set but are no longer in the .env file.
184+
// This ensures that when variables are commented out or removed from .env,
185+
// they are properly removed from the terminal environment without affecting
186+
// shell activation variables set by ShellStartupActivationVariablesManager.
187+
for (const key of previousKeys) {
188+
if (!(key in envVars)) {
174189
envVarScope.delete(key);
175-
} else {
190+
}
191+
}
192+
193+
// Set/update current variables
194+
for (const [key, value] of Object.entries(envVars)) {
195+
if (value !== undefined) {
176196
envVarScope.replace(key, value);
197+
currentKeys.add(key);
177198
}
178199
}
200+
201+
// Update tracking with current keys
202+
this.envVarKeys.set(workspaceKey, currentKeys);
179203
} catch (error) {
180204
traceError(
181205
`TerminalEnvVarInjector: Error injecting environment variables for workspace ${workspaceUri.fsPath}:`,
@@ -212,4 +236,20 @@ export class TerminalEnvVarInjector implements Disposable {
212236
traceError(`Failed to clear environment variables for workspace ${workspaceFolder.uri.fsPath}:`, error);
213237
}
214238
}
239+
240+
/**
241+
* Clear only the .env variables we've tracked, not shell activation variables.
242+
*/
243+
private clearTrackedEnvVariables(
244+
envVarScope: ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
245+
workspaceKey: string,
246+
): void {
247+
const trackedKeys = this.envVarKeys.get(workspaceKey);
248+
if (trackedKeys) {
249+
for (const key of trackedKeys) {
250+
envVarScope.delete(key);
251+
}
252+
this.envVarKeys.delete(workspaceKey);
253+
}
254+
}
215255
}

0 commit comments

Comments
 (0)