Skip to content

Commit fecb873

Browse files
feat: add "Don't Show Again" button to env file notification
Adds a "Don't Show Again" button to the information notification that appears when an environment file is configured but terminal environment injection is disabled. The preference is persisted in global state so it survives between sessions. - Add `Common.dontShowAgain` localized string - Add `ENV_FILE_NOTIFICATION_DONT_SHOW_KEY` persistent state key - Use `showInformationMessage` wrapper instead of direct `window` API - Extract notification logic to `showEnvFileNotification()` method - Add 5 unit tests covering the new behavior Fixes #419 Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Agent-Logs-Url: https://github.com/microsoft/vscode-python-environments/sessions/d9efb122-775e-4477-9005-10a94641311d
1 parent 22fd881 commit fecb873

File tree

3 files changed

+158
-6
lines changed

3 files changed

+158
-6
lines changed

src/common/localize.ts

Lines changed: 1 addition & 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
}
1920

2021
export namespace WorkbenchStrings {

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 29 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 { 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,26 @@ 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(
226+
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
227+
Common.dontShowAgain,
228+
);
229+
if (result === Common.dontShowAgain) {
230+
await state.set(ENV_FILE_NOTIFICATION_DONT_SHOW_KEY, true);
231+
traceLog('User selected "Don\'t Show Again" for env file notification');
232+
}
233+
}
234+
211235
/**
212236
* Dispose of the injector and clean up resources.
213237
*/

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)