Skip to content

Commit f658e5e

Browse files
committed
Add "Open Settings" option to environment file notification
1 parent 941fc45 commit f658e5e

File tree

3 files changed

+72
-13
lines changed

3 files changed

+72
-13
lines changed

src/common/localize.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export namespace Common {
1616
export const quickCreate = l10n.t('Quick Create');
1717
export const installPython = l10n.t('Install Python');
1818
export const dontShowAgain = l10n.t("Don't Show Again");
19+
export const openSettings = l10n.t('Open Settings');
1920
}
2021

2122
export namespace WorkbenchStrings {

src/features/terminal/terminalEnvVarInjector.ts

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,20 @@
44
import * as fse from 'fs-extra';
55
import * as path from 'path';
66
import {
7+
commands,
78
Disposable,
89
EnvironmentVariableScope,
910
GlobalEnvironmentVariableCollection,
1011
workspace,
1112
WorkspaceFolder,
1213
} from 'vscode';
14+
import { Common } from '../../common/localize';
1315
import { traceError, traceVerbose } from '../../common/logging';
16+
import { getGlobalPersistentState } from '../../common/persistentState';
1417
import { resolveVariables } from '../../common/utils/internalVariables';
18+
import * as windowApis from '../../common/window.apis';
1519
import { getConfiguration, getWorkspaceFolder } from '../../common/workspace.apis';
1620
import { EnvVarManager } from '../execution/envVariableManager';
17-
import { getGlobalPersistentState } from '../../common/persistentState';
18-
import { Common } from '../../common/localize';
19-
import * as windowApis from '../../common/window.apis';
2021

2122
/**
2223
* Manages injection of workspace-specific environment variables into VS Code terminals
@@ -192,7 +193,10 @@ export class TerminalEnvVarInjector implements Disposable {
192193
*/
193194
private async showEnvFileNotification(): Promise<void> {
194195
const state = await getGlobalPersistentState();
195-
const dontShowAgain = await state.get<boolean>(TerminalEnvVarInjector.DONT_SHOW_ENV_FILE_NOTIFICATION_KEY, false);
196+
const dontShowAgain = await state.get<boolean>(
197+
TerminalEnvVarInjector.DONT_SHOW_ENV_FILE_NOTIFICATION_KEY,
198+
false,
199+
);
196200

197201
if (dontShowAgain) {
198202
traceVerbose('TerminalEnvVarInjector: Env file notification suppressed by user preference');
@@ -201,10 +205,14 @@ export class TerminalEnvVarInjector implements Disposable {
201205

202206
const result = await windowApis.showInformationMessage(
203207
'An environment file is configured but terminal environment injection is disabled. Enable "python.terminal.useEnvFile" to use environment variables from .env files in terminals.',
208+
Common.openSettings,
204209
Common.dontShowAgain,
205210
);
206211

207-
if (result === Common.dontShowAgain) {
212+
if (result === Common.openSettings) {
213+
await commands.executeCommand('workbench.action.openSettings', 'python.terminal.useEnvFile');
214+
traceVerbose('TerminalEnvVarInjector: User opened settings for useEnvFile');
215+
} else if (result === Common.dontShowAgain) {
208216
await state.set(TerminalEnvVarInjector.DONT_SHOW_ENV_FILE_NOTIFICATION_KEY, true);
209217
traceVerbose('TerminalEnvVarInjector: User chose to not show env file notification again');
210218
}

src/test/features/terminalEnvVarInjectorNotification.unit.test.ts

Lines changed: 58 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,13 @@
44
import * as assert from 'node:assert';
55
import * as sinon from 'sinon';
66
import * as typeMoq from 'typemoq';
7-
import { GlobalEnvironmentVariableCollection, Uri, workspace } from 'vscode';
8-
import { EnvVarManager } from '../../features/execution/envVariableManager';
9-
import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
7+
import { commands, GlobalEnvironmentVariableCollection, Uri, workspace } from 'vscode';
8+
import { Common } from '../../common/localize';
109
import * as persistentState from '../../common/persistentState';
11-
import * as workspaceApis from '../../common/workspace.apis';
1210
import * as windowApis from '../../common/window.apis';
13-
import { Common } from '../../common/localize';
11+
import * as workspaceApis from '../../common/workspace.apis';
12+
import { EnvVarManager } from '../../features/execution/envVariableManager';
13+
import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
1414

1515
interface MockScopedCollection {
1616
clear: sinon.SinonStub;
@@ -27,6 +27,7 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
2727
let mockGetConfiguration: sinon.SinonStub;
2828
let mockGetWorkspaceFolder: sinon.SinonStub;
2929
let mockShowInformationMessage: sinon.SinonStub;
30+
let mockExecuteCommand: sinon.SinonStub;
3031
let envVarChangeHandler: (args: { uri?: Uri; changeType?: number }) => void;
3132
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3233
let workspaceFoldersStub: any;
@@ -72,6 +73,9 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
7273
// Setup showInformationMessage mock
7374
mockShowInformationMessage = sinon.stub(windowApis, 'showInformationMessage').resolves(undefined);
7475

76+
// Setup executeCommand mock
77+
mockExecuteCommand = sinon.stub(commands, 'executeCommand').resolves();
78+
7579
// Setup environment variable change event handler - will be overridden in tests
7680
envVarManager
7781
.setup((m) => m.onDidChangeEnvironmentVariables)
@@ -96,15 +100,15 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
96100
// Setup environment variable change handler to capture it
97101
envVarManager.reset();
98102
envVarCollection.reset();
99-
103+
100104
// Re-setup scoped collection after reset
101105
envVarCollection
102106
.setup((x) => x.getScoped(typeMoq.It.isAny()))
103107
.returns(
104108
() => mockScopedCollection as unknown as ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
105109
);
106110
envVarCollection.setup((x) => x.clear()).returns(() => {});
107-
111+
108112
// Setup event handler to capture it
109113
envVarManager
110114
.setup((m) => m.onDidChangeEnvironmentVariables)
@@ -131,7 +135,7 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
131135
envVarChangeHandler({ uri: testUri });
132136
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async notification
133137

134-
// Assert - notification should be shown
138+
// Assert - notification should be shown with both buttons
135139
assert.ok(mockShowInformationMessage.called, 'showInformationMessage should be called');
136140
const notificationCall = mockShowInformationMessage.getCall(0);
137141
assert.ok(
@@ -140,6 +144,11 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
140144
);
141145
assert.strictEqual(
142146
notificationCall.args[1],
147+
Common.openSettings,
148+
'Notification should have "Open Settings" button',
149+
);
150+
assert.strictEqual(
151+
notificationCall.args[2],
143152
Common.dontShowAgain,
144153
'Notification should have "Don\'t Show Again" button',
145154
);
@@ -219,6 +228,47 @@ suite('TerminalEnvVarInjector Notification Tests', () => {
219228
assert.strictEqual(setCall.args[1], true, 'Should set state to true');
220229
});
221230

231+
test('should open settings when user clicks "Open Settings" button', async () => {
232+
// Arrange - user clicks the "Open Settings" button
233+
mockState.get.resolves(false);
234+
mockShowInformationMessage.resolves(Common.openSettings);
235+
236+
// Setup environment variable change handler to capture it
237+
envVarManager.reset();
238+
envVarManager
239+
.setup((m) => m.onDidChangeEnvironmentVariables)
240+
.returns((handler) => {
241+
envVarChangeHandler = handler;
242+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
243+
return { dispose: () => {} } as any;
244+
});
245+
246+
const mockConfig = {
247+
get: sinon.stub(),
248+
};
249+
mockConfig.get.withArgs('terminal.useEnvFile', false).returns(false);
250+
mockConfig.get.withArgs('envFile').returns('.env');
251+
mockGetConfiguration.returns(mockConfig);
252+
253+
const testUri = Uri.file('/workspace');
254+
mockGetWorkspaceFolder.returns({ uri: testUri });
255+
256+
// Act - create injector and trigger env var change
257+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
258+
await new Promise((resolve) => setTimeout(resolve, 10)); // Allow async initialization
259+
260+
envVarChangeHandler({ uri: testUri });
261+
await new Promise((resolve) => setTimeout(resolve, 50)); // Allow async notification and command execution
262+
263+
// Assert - executeCommand should be called to open settings
264+
assert.ok(mockExecuteCommand.called, 'executeCommand should be called');
265+
const commandCall = mockExecuteCommand.getCall(0);
266+
assert.strictEqual(commandCall.args[0], 'workbench.action.openSettings', 'Should open settings');
267+
assert.strictEqual(commandCall.args[1], 'python.terminal.useEnvFile', 'Should open useEnvFile setting');
268+
// State should NOT be set when clicking "Open Settings"
269+
assert.ok(!mockState.set.called, 'state.set should not be called when opening settings');
270+
});
271+
222272
test('should not show notification when useEnvFile is true', async () => {
223273
// Arrange - useEnvFile is enabled
224274
mockState.get.resolves(false);

0 commit comments

Comments
 (0)