Skip to content

Commit d2b1660

Browse files
committed
.env file should not clear shellStartup env var
1 parent b4f6631 commit d2b1660

File tree

2 files changed

+38
-29
lines changed

2 files changed

+38
-29
lines changed

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,16 @@
33

44
import * as fse from 'fs-extra';
55
import * as path from 'path';
6-
import {
7-
Disposable,
8-
EnvironmentVariableScope,
9-
GlobalEnvironmentVariableCollection,
10-
window,
11-
workspace,
12-
WorkspaceFolder,
13-
} from 'vscode';
6+
import { Disposable, EnvironmentVariableScope, GlobalEnvironmentVariableCollection, WorkspaceFolder } from 'vscode';
147
import { traceError, traceVerbose } from '../../common/logging';
158
import { resolveVariables } from '../../common/utils/internalVariables';
16-
import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
9+
import { showInformationMessage } from '../../common/window.apis';
10+
import {
11+
getConfiguration,
12+
getWorkspaceFolder,
13+
getWorkspaceFolders,
14+
onDidChangeConfiguration,
15+
} from '../../common/workspace.apis';
1716
import { EnvVarManager } from '../execution/envVariableManager';
1817

1918
/**
@@ -65,7 +64,7 @@ export class TerminalEnvVarInjector implements Disposable {
6564

6665
// Only show notification when env vars change and we have an env file but injection is disabled
6766
if (!useEnvFile && envFilePath) {
68-
window.showInformationMessage(
67+
showInformationMessage(
6968
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
7069
);
7170
}
@@ -83,7 +82,7 @@ export class TerminalEnvVarInjector implements Disposable {
8382

8483
// Listen for changes to the python.envFile setting
8584
this.disposables.push(
86-
workspace.onDidChangeConfiguration((e) => {
85+
onDidChangeConfiguration((e) => {
8786
if (e.affectsConfiguration('python.envFile')) {
8887
traceVerbose(
8988
'TerminalEnvVarInjector: python.envFile setting changed, updating environment variables',
@@ -113,7 +112,7 @@ export class TerminalEnvVarInjector implements Disposable {
113112
} else {
114113
// No provided workspace - update all workspaces
115114

116-
const workspaceFolders = workspace.workspaceFolders;
115+
const workspaceFolders = getWorkspaceFolders();
117116
if (!workspaceFolders || workspaceFolders.length === 0) {
118117
traceVerbose('TerminalEnvVarInjector: No workspace folders found, skipping env var injection');
119118
return;
@@ -136,7 +135,8 @@ export class TerminalEnvVarInjector implements Disposable {
136135
*/
137136
private async injectEnvironmentVariablesForWorkspace(workspaceFolder: WorkspaceFolder): Promise<void> {
138137
const workspaceUri = workspaceFolder.uri;
139-
const workspaceKey = workspaceUri.fsPath;
138+
// Use path.resolve() for safe cross-platform map key (Windows \ vs POSIX /)
139+
const workspaceKey = path.resolve(workspaceUri.fsPath);
140140

141141
try {
142142
const envVars = await this.envVarManager.getEnvironmentVariables(workspaceUri);
@@ -210,14 +210,25 @@ export class TerminalEnvVarInjector implements Disposable {
210210

211211
/**
212212
* Dispose of the injector and clean up resources.
213+
* Only clears the .env variables this injector has tracked, preserving
214+
* shell activation variables (VSCODE_PYTHON_*_ACTIVATE) set by
215+
* ShellStartupActivationVariablesManager on the same shared collection.
213216
*/
214217
dispose(): void {
215218
traceVerbose('TerminalEnvVarInjector: Disposing');
216219
this.disposables.forEach((disposable) => disposable.dispose());
217220
this.disposables = [];
218221

219-
// Clear all environment variables from the collection
220-
this.envVarCollection.clear();
222+
// Only remove the .env keys we tracked — do NOT call envVarCollection.clear().
223+
// The collection is shared with ShellStartupActivationVariablesManager which
224+
// contributes VSCODE_PYTHON_*_ACTIVATE variables that must survive disposal.
225+
const workspaceFolders = getWorkspaceFolders();
226+
if (workspaceFolders) {
227+
for (const folder of workspaceFolders) {
228+
const scope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder: folder });
229+
this.clearTrackedEnvVariables(scope, path.resolve(folder.uri.fsPath));
230+
}
231+
}
221232
}
222233

223234
private getEnvironmentVariableCollectionScoped(scope: EnvironmentVariableScope = {}) {
@@ -226,12 +237,14 @@ export class TerminalEnvVarInjector implements Disposable {
226237
}
227238

228239
/**
229-
* Clear all environment variables for a workspace.
240+
* Clear .env variables for a workspace when the .env file is deleted.
241+
* Only removes tracked keys, preserving shell activation variables
242+
* (VSCODE_PYTHON_*_ACTIVATE) set by ShellStartupActivationVariablesManager.
230243
*/
231244
private clearWorkspaceVariables(workspaceFolder: WorkspaceFolder): void {
232245
try {
233246
const scope = this.getEnvironmentVariableCollectionScoped({ workspaceFolder });
234-
scope.clear();
247+
this.clearTrackedEnvVariables(scope, path.resolve(workspaceFolder.uri.fsPath));
235248
} catch (error) {
236249
traceError(`Failed to clear environment variables for workspace ${workspaceFolder.uri.fsPath}:`, error);
237250
}

src/test/features/terminalEnvVarInjector.unit.test.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
Uri,
1212
WorkspaceConfiguration,
1313
WorkspaceFolder,
14-
workspace,
1514
} from 'vscode';
1615
import * as workspaceApis from '../../common/workspace.apis';
1716
import { EnvVarManager } from '../../features/execution/envVariableManager';
@@ -61,16 +60,10 @@ suite('TerminalEnvVarInjector', () => {
6160
envVarManager = typeMoq.Mock.ofType<EnvVarManager>();
6261

6362
workspaceFoldersValue = [testWorkspaceFolder];
64-
Object.defineProperty(workspace, 'workspaceFolders', {
65-
get: () => workspaceFoldersValue,
66-
configurable: true,
67-
});
6863

69-
// Mock workspace.onDidChangeConfiguration to return a proper disposable
70-
Object.defineProperty(workspace, 'onDidChangeConfiguration', {
71-
value: () => new Disposable(() => {}),
72-
configurable: true,
73-
});
64+
// Stub workspace.apis wrappers so TerminalEnvVarInjector uses mockable functions
65+
sinon.stub(workspaceApis, 'getWorkspaceFolders').callsFake(() => workspaceFoldersValue);
66+
sinon.stub(workspaceApis, 'onDidChangeConfiguration').returns(new Disposable(() => {}));
7467

7568
mockScopedCollection = {
7669
clear: sinon.stub(),
@@ -108,10 +101,13 @@ suite('TerminalEnvVarInjector', () => {
108101
sinon.assert.match(injector, sinon.match.object);
109102
});
110103

111-
test('should dispose cleanly', () => {
104+
test('should dispose without clearing the entire collection (preserves shell activation vars)', () => {
112105
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
113106
injector.dispose();
114-
envVarCollection.verify((c) => c.clear(), typeMoq.Times.atLeastOnce());
107+
// dispose() must NOT call envVarCollection.clear() because the collection
108+
// is shared with ShellStartupActivationVariablesManager which contributes
109+
// VSCODE_PYTHON_*_ACTIVATE variables that must survive extension deactivation.
110+
envVarCollection.verify((c) => c.clear(), typeMoq.Times.never());
115111
});
116112

117113
test('should register environment variable change event handler', () => {

0 commit comments

Comments
 (0)