Skip to content

Commit 70a0087

Browse files
Copiloteleanorjboyd
andcommitted
Add "Don't show again" button to env file notification
- Added localized string 'dontShowAgain' in Common namespace - Modified TerminalEnvVarInjector to check persistent state before showing notification - Added showEnvFileNotification method with "Don't Show Again" button - Created comprehensive unit tests for the notification functionality Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com>
1 parent cc96f95 commit 70a0087

3 files changed

Lines changed: 258 additions & 3 deletions

File tree

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 & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,16 @@ import { traceError, traceVerbose } from '../../common/logging';
1515
import { resolveVariables } from '../../common/utils/internalVariables';
1616
import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
1717
import { EnvVarManager } from '../execution/envVariableManager';
18+
import { getGlobalPersistentState } from '../../common/persistentState';
19+
import { Common } from '../../common/localize';
1820

1921
/**
2022
* Manages injection of workspace-specific environment variables into VS Code terminals
2123
* using the GlobalEnvironmentVariableCollection API.
2224
*/
2325
export class TerminalEnvVarInjector implements Disposable {
2426
private disposables: Disposable[] = [];
27+
private static readonly DONT_SHOW_ENV_FILE_NOTIFICATION_KEY = 'dontShowEnvFileNotification';
2528

2629
constructor(
2730
private readonly envVarCollection: GlobalEnvironmentVariableCollection,
@@ -63,9 +66,9 @@ export class TerminalEnvVarInjector implements Disposable {
6366

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

7174
if (args.changeType === 2) {
@@ -184,6 +187,29 @@ export class TerminalEnvVarInjector implements Disposable {
184187
}
185188
}
186189

190+
/**
191+
* Show notification about environment file configuration with "Don't show again" option.
192+
*/
193+
private async showEnvFileNotification(): Promise<void> {
194+
const state = await getGlobalPersistentState();
195+
const dontShowAgain = await state.get<boolean>(TerminalEnvVarInjector.DONT_SHOW_ENV_FILE_NOTIFICATION_KEY, false);
196+
197+
if (dontShowAgain) {
198+
traceVerbose('TerminalEnvVarInjector: Env file notification suppressed by user preference');
199+
return;
200+
}
201+
202+
const result = await window.showInformationMessage(
203+
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
204+
Common.dontShowAgain,
205+
);
206+
207+
if (result === Common.dontShowAgain) {
208+
await state.set(TerminalEnvVarInjector.DONT_SHOW_ENV_FILE_NOTIFICATION_KEY, true);
209+
traceVerbose('TerminalEnvVarInjector: User chose to not show env file notification again');
210+
}
211+
}
212+
187213
/**
188214
* Dispose of the injector and clean up resources.
189215
*/
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'node:assert';
5+
import * as sinon from 'sinon';
6+
import * as typeMoq from 'typemoq';
7+
import { GlobalEnvironmentVariableCollection, Uri, window, workspace } from 'vscode';
8+
import { EnvVarManager } from '../../features/execution/envVariableManager';
9+
import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
10+
import * as persistentState from '../../common/persistentState';
11+
import * as workspaceApis from '../../common/workspace.apis';
12+
import { Common } from '../../common/localize';
13+
14+
interface MockScopedCollection {
15+
clear: sinon.SinonStub;
16+
replace: sinon.SinonStub;
17+
delete: sinon.SinonStub;
18+
}
19+
20+
suite('TerminalEnvVarInjector Notification Tests', () => {
21+
let envVarCollection: typeMoq.IMock<GlobalEnvironmentVariableCollection>;
22+
let envVarManager: typeMoq.IMock<EnvVarManager>;
23+
let injector: TerminalEnvVarInjector;
24+
let mockScopedCollection: MockScopedCollection;
25+
let mockGetGlobalPersistentState: sinon.SinonStub;
26+
let mockState: { get: sinon.SinonStub; set: sinon.SinonStub; clear: sinon.SinonStub };
27+
let mockGetConfiguration: sinon.SinonStub;
28+
let mockGetWorkspaceFolder: sinon.SinonStub;
29+
let mockShowInformationMessage: sinon.SinonStub;
30+
let envVarChangeHandler: (args: { uri?: Uri; changeType?: number }) => void;
31+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
32+
let workspaceFoldersStub: any;
33+
34+
setup(() => {
35+
envVarCollection = typeMoq.Mock.ofType<GlobalEnvironmentVariableCollection>();
36+
envVarManager = typeMoq.Mock.ofType<EnvVarManager>();
37+
38+
// Mock workspace.workspaceFolders property
39+
workspaceFoldersStub = [];
40+
Object.defineProperty(workspace, 'workspaceFolders', {
41+
get: () => workspaceFoldersStub,
42+
configurable: true,
43+
});
44+
45+
// Setup scoped collection mock
46+
mockScopedCollection = {
47+
clear: sinon.stub(),
48+
replace: sinon.stub(),
49+
delete: sinon.stub(),
50+
};
51+
52+
// Setup environment variable collection to return scoped collection
53+
envVarCollection
54+
.setup((x) => x.getScoped(typeMoq.It.isAny()))
55+
.returns(
56+
() => mockScopedCollection as unknown as ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
57+
);
58+
envVarCollection.setup((x) => x.clear()).returns(() => {});
59+
60+
// Setup persistent state mocks
61+
mockState = {
62+
get: sinon.stub(),
63+
set: sinon.stub().resolves(),
64+
clear: sinon.stub().resolves(),
65+
};
66+
mockGetGlobalPersistentState = sinon.stub(persistentState, 'getGlobalPersistentState').resolves(mockState);
67+
68+
// Setup workspace API mocks
69+
mockGetConfiguration = sinon.stub(workspaceApis, 'getConfiguration');
70+
mockGetWorkspaceFolder = sinon.stub(workspaceApis, 'getWorkspaceFolder');
71+
72+
// Setup window.showInformationMessage mock
73+
mockShowInformationMessage = sinon.stub(window, 'showInformationMessage').resolves(undefined);
74+
75+
// Setup environment variable change event handler
76+
envVarManager
77+
.setup((m) => m.onDidChangeEnvironmentVariables)
78+
.returns((handler) => {
79+
envVarChangeHandler = handler;
80+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
81+
return { dispose: () => {} } as any;
82+
});
83+
});
84+
85+
teardown(() => {
86+
sinon.restore();
87+
injector?.dispose();
88+
});
89+
90+
test('should show notification when env file exists and useEnvFile is false (first time)', async () => {
91+
// Arrange - user has not dismissed the notification before
92+
mockState.get.resolves(false);
93+
94+
const mockConfig = {
95+
get: sinon.stub(),
96+
};
97+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
98+
mockConfig.get.withArgs('envFile').returns('.env');
99+
mockGetConfiguration.returns(mockConfig);
100+
101+
const testUri = Uri.file('/workspace');
102+
mockGetWorkspaceFolder.returns({ uri: testUri });
103+
104+
// Act - create injector and trigger env var change
105+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
106+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
107+
108+
envVarChangeHandler({ uri: testUri });
109+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
110+
111+
// Assert - notification should be shown
112+
assert(mockShowInformationMessage.called, 'showInformationMessage should be called');
113+
const notificationCall = mockShowInformationMessage.getCall(0);
114+
assert(
115+
notificationCall.args[0].includes('environment file is configured'),
116+
'Notification should mention environment file',
117+
);
118+
assert.strictEqual(
119+
notificationCall.args[1],
120+
Common.dontShowAgain,
121+
'Notification should have "Don\'t Show Again" button',
122+
);
123+
});
124+
125+
test('should not show notification when user has clicked "Don\'t Show Again"', async () => {
126+
// Arrange - user has previously dismissed the notification
127+
mockState.get.resolves(true);
128+
129+
const mockConfig = {
130+
get: sinon.stub(),
131+
};
132+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
133+
mockConfig.get.withArgs('envFile').returns('.env');
134+
mockGetConfiguration.returns(mockConfig);
135+
136+
const testUri = Uri.file('/workspace');
137+
mockGetWorkspaceFolder.returns({ uri: testUri });
138+
139+
// Act - create injector and trigger env var change
140+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
141+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
142+
143+
envVarChangeHandler({ uri: testUri });
144+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
145+
146+
// Assert - notification should NOT be shown
147+
assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
148+
});
149+
150+
test('should store preference when user clicks "Don\'t Show Again"', async () => {
151+
// Arrange - user clicks the "Don't Show Again" button
152+
mockState.get.resolves(false);
153+
mockShowInformationMessage.resolves(Common.dontShowAgain);
154+
155+
const mockConfig = {
156+
get: sinon.stub(),
157+
};
158+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
159+
mockConfig.get.withArgs('envFile').returns('.env');
160+
mockGetConfiguration.returns(mockConfig);
161+
162+
const testUri = Uri.file('/workspace');
163+
mockGetWorkspaceFolder.returns({ uri: testUri });
164+
165+
// Act - create injector and trigger env var change
166+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
167+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
168+
169+
envVarChangeHandler({ uri: testUri });
170+
await new Promise((resolve) => setTimeout(resolve, 50)); // Allow async notification and state update
171+
172+
// Assert - state should be set to true
173+
assert(mockState.set.called, 'state.set should be called');
174+
const setCall = mockState.set.getCall(0);
175+
assert.strictEqual(setCall.args[0], 'dontShowEnvFileNotification', 'Should use correct state key');
176+
assert.strictEqual(setCall.args[1], true, 'Should set state to true');
177+
});
178+
179+
test('should not show notification when useEnvFile is true', async () => {
180+
// Arrange - useEnvFile is enabled
181+
mockState.get.resolves(false);
182+
183+
const mockConfig = {
184+
get: sinon.stub(),
185+
};
186+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(true); // enabled
187+
mockConfig.get.withArgs('envFile').returns('.env');
188+
mockGetConfiguration.returns(mockConfig);
189+
190+
const testUri = Uri.file('/workspace');
191+
mockGetWorkspaceFolder.returns({ uri: testUri });
192+
193+
// Act - create injector and trigger env var change
194+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
195+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
196+
197+
envVarChangeHandler({ uri: testUri });
198+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
199+
200+
// Assert - notification should NOT be shown
201+
assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
202+
});
203+
204+
test('should not show notification when envFile is not configured', async () => {
205+
// Arrange - no envFile configured
206+
mockState.get.resolves(false);
207+
208+
const mockConfig = {
209+
get: sinon.stub(),
210+
};
211+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
212+
mockConfig.get.withArgs('envFile').returns(undefined); // no env file
213+
mockGetConfiguration.returns(mockConfig);
214+
215+
const testUri = Uri.file('/workspace');
216+
mockGetWorkspaceFolder.returns({ uri: testUri });
217+
218+
// Act - create injector and trigger env var change
219+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
220+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
221+
222+
envVarChangeHandler({ uri: testUri });
223+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
224+
225+
// Assert - notification should NOT be shown
226+
assert(!mockShowInformationMessage.called, 'showInformationMessage should not be called');
227+
});
228+
});

0 commit comments

Comments
 (0)