Skip to content

Commit eff0768

Browse files
Add "Don't Show Again" to env file notification (#1393)
The env file injection notification fires on every `.env` file save with no way to suppress it, including for non-Python users who happen to use `.env` files. ### Changes - **`terminalEnvVarInjector.ts`**: Extract notification into `showEnvFileNotification()`, gate on global persistent state, offer "Don't Show Again" button that persists the dismissal via `getGlobalPersistentState()` - **`localize.ts`**: Add `Common.dontShowAgain` for reuse across features - Switch from direct `window.showInformationMessage` to the `showInformationMessage` wrapper (consistent with project convention, enables stubbing in tests) - 5 new unit tests covering show/suppress/persist paths > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `https://api.github.com/graphql` > - Triggering command: `/usr/bin/gh gh issue list --state open --limit 5 --json number,title,labels` (http block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/microsoft/vscode-python-environments/settings/copilot/coding_agent) (admins only) > > </details> <!-- START COPILOT CODING AGENT TIPS --> --- 💬 Send tasks to Copilot coding agent from [Slack](https://gh.io/cca-slack-docs) and [Teams](https://gh.io/cca-teams-docs) to turn conversations into code. Copilot posts an update in your thread when it's finished. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
1 parent 771f9c3 commit eff0768

File tree

3 files changed

+158
-6
lines changed

3 files changed

+158
-6
lines changed

src/common/localize.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export namespace Common {
1515
export const ok = l10n.t('Ok');
1616
export const quickCreate = l10n.t('Quick Create');
1717
export const installPython = l10n.t('Install Python');
18+
export const dontShowAgain = l10n.t("Don't Show Again");
1819
export const dontAskAgain = l10n.t("Don't ask again");
1920
}
2021

@@ -223,6 +224,9 @@ export namespace ActivationStrings {
223224
Commands.viewLogs,
224225
);
225226
export const activatingEnvironment = l10n.t('Activating environment');
227+
export const envFileInjectionDisabled = l10n.t(
228+
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
229+
);
226230
}
227231

228232
export namespace UvInstallStrings {

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,19 @@ import {
77
Disposable,
88
EnvironmentVariableScope,
99
GlobalEnvironmentVariableCollection,
10-
window,
1110
workspace,
1211
WorkspaceFolder,
1312
} from 'vscode';
14-
import { traceError, traceVerbose } from '../../common/logging';
13+
import { ActivationStrings, Common } from '../../common/localize';
14+
import { traceError, traceLog, traceVerbose } from '../../common/logging';
15+
import { getGlobalPersistentState } from '../../common/persistentState';
1516
import { resolveVariables } from '../../common/utils/internalVariables';
17+
import { showInformationMessage } from '../../common/window.apis';
1618
import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
1719
import { EnvVarManager } from '../execution/envVariableManager';
1820

21+
export const ENV_FILE_NOTIFICATION_DONT_SHOW_KEY = 'python-envs:terminal:ENV_FILE_NOTIFICATION_DONT_SHOW';
22+
1923
/**
2024
* Manages injection of workspace-specific environment variables into VS Code terminals
2125
* using the GlobalEnvironmentVariableCollection API.
@@ -65,9 +69,9 @@ export class TerminalEnvVarInjector implements Disposable {
6569

6670
// Only show notification when env vars change and we have an env file but injection is disabled
6771
if (!useEnvFile && envFilePath) {
68-
window.showInformationMessage(
69-
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
70-
);
72+
this.showEnvFileNotification().catch((error) => {
73+
traceError('Failed to show env file notification:', error);
74+
});
7175
}
7276

7377
if (args.changeType === 2) {
@@ -208,6 +212,23 @@ export class TerminalEnvVarInjector implements Disposable {
208212
}
209213
}
210214

215+
/**
216+
* Show a notification about env file injection being disabled, with a "Don't Show Again" option.
217+
*/
218+
private async showEnvFileNotification(): Promise<void> {
219+
const state = await getGlobalPersistentState();
220+
const dontShow = await state.get<boolean>(ENV_FILE_NOTIFICATION_DONT_SHOW_KEY);
221+
if (dontShow) {
222+
return;
223+
}
224+
225+
const result = await showInformationMessage(ActivationStrings.envFileInjectionDisabled, Common.dontShowAgain);
226+
if (result === Common.dontShowAgain) {
227+
await state.set(ENV_FILE_NOTIFICATION_DONT_SHOW_KEY, true);
228+
traceLog(`User selected "Don't Show Again" for env file notification`);
229+
}
230+
}
231+
211232
/**
212233
* Dispose of the injector and clean up resources.
213234
*/

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

Lines changed: 128 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,15 @@ import {
1313
WorkspaceFolder,
1414
workspace,
1515
} from 'vscode';
16+
import { Common } from '../../common/localize';
17+
import * as persistentState from '../../common/persistentState';
18+
import * as windowApis from '../../common/window.apis';
1619
import * as workspaceApis from '../../common/workspace.apis';
1720
import { EnvVarManager } from '../../features/execution/envVariableManager';
18-
import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
21+
import {
22+
ENV_FILE_NOTIFICATION_DONT_SHOW_KEY,
23+
TerminalEnvVarInjector,
24+
} from '../../features/terminal/terminalEnvVarInjector';
1925

2026
interface MockScopedCollection {
2127
clear: sinon.SinonStub;
@@ -307,4 +313,125 @@ suite('TerminalEnvVarInjector', () => {
307313
}
308314
});
309315
});
316+
317+
suite('env file notification with Don\'t Show Again', () => {
318+
let envChangeCallback: ((args: { uri?: Uri; changeType: number }) => void) | undefined;
319+
let mockState: { get: sinon.SinonStub; set: sinon.SinonStub; clear: sinon.SinonStub };
320+
let showInfoMessageStub: sinon.SinonStub;
321+
322+
setup(() => {
323+
mockState = {
324+
get: sinon.stub(),
325+
set: sinon.stub().resolves(),
326+
clear: sinon.stub().resolves(),
327+
};
328+
sinon.stub(persistentState, 'getGlobalPersistentState').resolves(mockState);
329+
showInfoMessageStub = sinon.stub(windowApis, 'showInformationMessage');
330+
331+
// Capture the onDidChangeEnvironmentVariables listener
332+
envVarManager.reset();
333+
envVarManager
334+
.setup((m) => m.onDidChangeEnvironmentVariables)
335+
.returns(() => {
336+
return (listener: (args: { uri?: Uri; changeType: number }) => void): Disposable => {
337+
envChangeCallback = listener;
338+
return new Disposable(() => {});
339+
};
340+
});
341+
envVarManager
342+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
343+
.returns(() => Promise.resolve({}));
344+
345+
sinon.stub(workspaceApis, 'getWorkspaceFolder').returns(testWorkspaceFolder);
346+
});
347+
348+
test('should show notification with Don\'t Show Again button when env file configured but injection disabled', async () => {
349+
getConfigurationStub.returns(
350+
createMockConfig({ useEnvFile: false, envFilePath: '${workspaceFolder}/.env' }) as WorkspaceConfiguration,
351+
);
352+
mockState.get.resolves(false);
353+
showInfoMessageStub.resolves(undefined);
354+
355+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
356+
await new Promise((resolve) => setTimeout(resolve, 50));
357+
358+
assert.ok(envChangeCallback, 'Event handler should be registered');
359+
envChangeCallback!({ uri: Uri.file(testWorkspacePath), changeType: 1 });
360+
await new Promise((resolve) => setTimeout(resolve, 50));
361+
362+
assert.ok(showInfoMessageStub.calledOnce, 'Should show notification');
363+
assert.ok(
364+
showInfoMessageStub.calledWith(sinon.match.string, Common.dontShowAgain),
365+
'Should include Don\'t Show Again button',
366+
);
367+
});
368+
369+
test('should not show notification when Don\'t Show Again was previously selected', async () => {
370+
getConfigurationStub.returns(
371+
createMockConfig({ useEnvFile: false, envFilePath: '${workspaceFolder}/.env' }) as WorkspaceConfiguration,
372+
);
373+
mockState.get.resolves(true);
374+
375+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
376+
await new Promise((resolve) => setTimeout(resolve, 50));
377+
378+
assert.ok(envChangeCallback, 'Event handler should be registered');
379+
envChangeCallback!({ uri: Uri.file(testWorkspacePath), changeType: 1 });
380+
await new Promise((resolve) => setTimeout(resolve, 50));
381+
382+
assert.ok(showInfoMessageStub.notCalled, 'Should not show notification when dismissed');
383+
});
384+
385+
test('should persist preference when Don\'t Show Again is clicked', async () => {
386+
getConfigurationStub.returns(
387+
createMockConfig({ useEnvFile: false, envFilePath: '${workspaceFolder}/.env' }) as WorkspaceConfiguration,
388+
);
389+
mockState.get.resolves(false);
390+
showInfoMessageStub.resolves(Common.dontShowAgain);
391+
392+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
393+
await new Promise((resolve) => setTimeout(resolve, 50));
394+
395+
assert.ok(envChangeCallback, 'Event handler should be registered');
396+
envChangeCallback!({ uri: Uri.file(testWorkspacePath), changeType: 1 });
397+
await new Promise((resolve) => setTimeout(resolve, 50));
398+
399+
assert.ok(
400+
mockState.set.calledWith(ENV_FILE_NOTIFICATION_DONT_SHOW_KEY, true),
401+
'Should persist Don\'t Show Again preference',
402+
);
403+
});
404+
405+
test('should not show notification when useEnvFile is true', async () => {
406+
getConfigurationStub.returns(
407+
createMockConfig({ useEnvFile: true, envFilePath: '${workspaceFolder}/.env' }) as WorkspaceConfiguration,
408+
);
409+
mockState.get.resolves(false);
410+
411+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
412+
await new Promise((resolve) => setTimeout(resolve, 50));
413+
414+
assert.ok(envChangeCallback, 'Event handler should be registered');
415+
envChangeCallback!({ uri: Uri.file(testWorkspacePath), changeType: 1 });
416+
await new Promise((resolve) => setTimeout(resolve, 50));
417+
418+
assert.ok(showInfoMessageStub.notCalled, 'Should not show notification when useEnvFile is true');
419+
});
420+
421+
test('should not show notification when no envFile is configured', async () => {
422+
getConfigurationStub.returns(
423+
createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration,
424+
);
425+
mockState.get.resolves(false);
426+
427+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
428+
await new Promise((resolve) => setTimeout(resolve, 50));
429+
430+
assert.ok(envChangeCallback, 'Event handler should be registered');
431+
envChangeCallback!({ uri: Uri.file(testWorkspacePath), changeType: 1 });
432+
await new Promise((resolve) => setTimeout(resolve, 50));
433+
434+
assert.ok(showInfoMessageStub.notCalled, 'Should not show notification when no envFile configured');
435+
});
436+
});
310437
});

0 commit comments

Comments
 (0)