Skip to content

Commit bc8d754

Browse files
eleanorjboydeleanorjboyd
authored
Merge branch 'main' into bumpy-gopher
2 parents 51922a7 + fb936a1 commit bc8d754

File tree

6 files changed

+293
-111
lines changed

6 files changed

+293
-111
lines changed

‎src/features/terminal/shells/fish/fishStartup.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ async function isFishInstalled(): Promise<boolean> {
1919
}
2020
}
2121

22-
async function getFishProfile(): Promise<string> {
23-
const homeDir = os.homedir();
24-
// Fish configuration is typically at ~/.config/fish/config.fish
25-
const profilePath = path.join(homeDir, '.config', 'fish', 'config.fish');
22+
/**
23+
* Resolve the Fish configuration profile path, honoring XDG_CONFIG_HOME when set.
24+
*/
25+
export async function getFishProfile(): Promise<string> {
26+
const xdgConfigHome = process.env.XDG_CONFIG_HOME?.trim() ?? '';
27+
const baseConfigDir = xdgConfigHome.length > 0 ? xdgConfigHome : path.join(os.homedir(), '.config');
28+
// Fish configuration is typically at $XDG_CONFIG_HOME/fish/config.fish or ~/.config/fish/config.fish
29+
const profilePath = path.join(baseConfigDir, 'fish', 'config.fish');
2630
traceInfo(`SHELL: fish profile found at: ${profilePath}`);
2731
return profilePath;
2832
}

‎src/features/terminal/utils.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { onDidChangeTerminalShellIntegration, onDidWriteTerminalData } from '../
77
import { getConfiguration, getWorkspaceFolders } from '../../common/workspace.apis';
88

99
export const SHELL_INTEGRATION_TIMEOUT = 500; // 0.5 seconds
10-
export const SHELL_INTEGRATION_POLL_INTERVAL = 20; // 0.02 seconds
1110

1211
/**
1312
* Use `terminal.integrated.shellIntegration.timeout` setting if available.

‎src/managers/pyenv/pyenvUtils.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ export async function getPyenv(native?: NativePythonFinder): Promise<string | un
115115
await state.set(PYENV_PATH_KEY, undefined);
116116
}
117117

118-
const pyenvBin = isWindows() ? 'pyenv.exe' : 'pyenv';
118+
// pyenv-win provides pyenv.bat, not pyenv.exe
119+
// See: https://github.com/pyenv-win/pyenv-win
120+
const pyenvBin = isWindows() ? 'pyenv.bat' : 'pyenv';
119121
const pyenvRoot = process.env.PYENV_ROOT;
120122
if (pyenvRoot) {
121123
const pyenvPath = path.join(pyenvRoot, 'bin', pyenvBin);
@@ -269,7 +271,7 @@ export async function resolvePyenvPath(
269271
): Promise<PythonEnvironment | undefined> {
270272
try {
271273
const e = await nativeFinder.resolve(fsPath);
272-
if (e.kind !== NativePythonEnvironmentKind.pyenv) {
274+
if (e.kind !== NativePythonEnvironmentKind.pyenv && e.kind !== NativePythonEnvironmentKind.pyenvVirtualEnv) {
273275
return undefined;
274276
}
275277
const pyenv = await getPyenv(nativeFinder);

‎src/test/features/terminal/shells/fish/fishStartup.unit.test.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
import * as assert from 'assert';
2+
import * as os from 'os';
3+
import * as path from 'path';
4+
import * as sinon from 'sinon';
5+
6+
import { getFishProfile } from '../../../../../features/terminal/shells/fish/fishStartup';
7+
8+
suite('Fish Startup', () => {
9+
let originalXdgConfigHome: string | undefined;
10+
11+
setup(() => {
12+
originalXdgConfigHome = process.env.XDG_CONFIG_HOME;
13+
});
14+
15+
teardown(() => {
16+
if (originalXdgConfigHome === undefined) {
17+
delete process.env.XDG_CONFIG_HOME;
18+
} else {
19+
process.env.XDG_CONFIG_HOME = originalXdgConfigHome;
20+
}
21+
sinon.restore();
22+
});
23+
24+
test('getFishProfile uses XDG_CONFIG_HOME when set', async () => {
25+
const xdgConfigHome = path.join('test', 'xdg');
26+
process.env.XDG_CONFIG_HOME = xdgConfigHome;
27+
28+
const profilePath = await getFishProfile();
29+
30+
assert.strictEqual(profilePath, path.join(xdgConfigHome, 'fish', 'config.fish'));
31+
});
32+
33+
test('getFishProfile falls back to ~/.config when XDG_CONFIG_HOME is empty', async () => {
34+
process.env.XDG_CONFIG_HOME = ' ';
35+
const homeDir = os.homedir();
36+
37+
const profilePath = await getFishProfile();
38+
39+
assert.strictEqual(profilePath, path.join(homeDir, '.config', 'fish', 'config.fish'));
40+
});
41+
});

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

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
import * as assert from 'assert';
5+
import * as sinon from 'sinon';
6+
import * as typeMoq from 'typemoq';
7+
import {
8+
Disposable,
9+
Event,
10+
GlobalEnvironmentVariableCollection,
11+
Uri,
12+
WorkspaceConfiguration,
13+
WorkspaceFolder,
14+
workspace,
15+
} from 'vscode';
16+
import * as workspaceApis from '../../common/workspace.apis';
17+
import { EnvVarManager } from '../../features/execution/envVariableManager';
18+
import { TerminalEnvVarInjector } from '../../features/terminal/terminalEnvVarInjector';
19+
20+
interface MockScopedCollection {
21+
clear: sinon.SinonStub;
22+
replace: sinon.SinonStub;
23+
delete: sinon.SinonStub;
24+
}
25+
26+
function createMockConfig(settings: { useEnvFile?: boolean; envFilePath?: string }): Partial<WorkspaceConfiguration> {
27+
return {
28+
get: <T>(key: string, defaultValue?: T): T | undefined => {
29+
if (key === 'terminal.useEnvFile') {
30+
return (settings.useEnvFile ?? false) as T;
31+
}
32+
if (key === 'envFile') {
33+
return settings.envFilePath as T;
34+
}
35+
return defaultValue;
36+
},
37+
};
38+
}
39+
40+
function createMockWorkspaceFolder(fsPath: string, name: string, index: number): WorkspaceFolder {
41+
return { uri: Uri.file(fsPath), name, index };
42+
}
43+
44+
function createMockEvent<T>(): Event<T> {
45+
return (_listener: (e: T) => void): Disposable => new Disposable(() => {});
46+
}
47+
48+
suite('TerminalEnvVarInjector', () => {
49+
let envVarCollection: typeMoq.IMock<GlobalEnvironmentVariableCollection>;
50+
let envVarManager: typeMoq.IMock<EnvVarManager>;
51+
let injector: TerminalEnvVarInjector;
52+
let mockScopedCollection: MockScopedCollection;
53+
let getConfigurationStub: sinon.SinonStub;
54+
let workspaceFoldersValue: readonly WorkspaceFolder[] | undefined;
55+
56+
const testWorkspacePath = '/test/workspace';
57+
const testWorkspaceFolder = createMockWorkspaceFolder(testWorkspacePath, 'test', 0);
58+
59+
setup(() => {
60+
envVarCollection = typeMoq.Mock.ofType<GlobalEnvironmentVariableCollection>();
61+
envVarManager = typeMoq.Mock.ofType<EnvVarManager>();
62+
63+
workspaceFoldersValue = [testWorkspaceFolder];
64+
Object.defineProperty(workspace, 'workspaceFolders', {
65+
get: () => workspaceFoldersValue,
66+
configurable: true,
67+
});
68+
69+
// Mock workspace.onDidChangeConfiguration to return a proper disposable
70+
Object.defineProperty(workspace, 'onDidChangeConfiguration', {
71+
value: () => new Disposable(() => {}),
72+
configurable: true,
73+
});
74+
75+
mockScopedCollection = {
76+
clear: sinon.stub(),
77+
replace: sinon.stub(),
78+
delete: sinon.stub(),
79+
};
80+
81+
envVarCollection
82+
.setup((x) => x.getScoped(typeMoq.It.isAny()))
83+
.returns(
84+
() => mockScopedCollection as unknown as ReturnType<GlobalEnvironmentVariableCollection['getScoped']>,
85+
);
86+
envVarCollection.setup((x) => x.clear()).returns(() => {});
87+
88+
envVarManager
89+
.setup((m) => m.onDidChangeEnvironmentVariables)
90+
.returns(() => createMockEvent());
91+
92+
getConfigurationStub = sinon.stub(workspaceApis, 'getConfiguration');
93+
getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration);
94+
});
95+
96+
teardown(() => {
97+
sinon.restore();
98+
try {
99+
injector?.dispose();
100+
} catch {
101+
// Ignore disposal errors
102+
}
103+
});
104+
105+
suite('Basic functionality', () => {
106+
test('should initialize without errors', () => {
107+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
108+
sinon.assert.match(injector, sinon.match.object);
109+
});
110+
111+
test('should dispose cleanly', () => {
112+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
113+
injector.dispose();
114+
envVarCollection.verify((c) => c.clear(), typeMoq.Times.atLeastOnce());
115+
});
116+
117+
test('should register environment variable change event handler', () => {
118+
let eventHandlerRegistered = false;
119+
envVarManager.reset();
120+
envVarManager
121+
.setup((m) => m.onDidChangeEnvironmentVariables)
122+
.returns(() => {
123+
eventHandlerRegistered = true;
124+
return createMockEvent();
125+
});
126+
127+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
128+
sinon.assert.match(eventHandlerRegistered, true);
129+
});
130+
});
131+
132+
suite('useEnvFile=false', () => {
133+
test('should NOT inject env vars when useEnvFile is false', async () => {
134+
getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration);
135+
envVarManager
136+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
137+
.returns(() => Promise.resolve({ TEST_VAR: 'test_value', API_KEY: 'secret123' }));
138+
139+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
140+
await new Promise((resolve) => setTimeout(resolve, 50));
141+
142+
assert.strictEqual(mockScopedCollection.replace.called, false);
143+
});
144+
145+
test('should NOT inject when useEnvFile is false even with python.envFile configured', async () => {
146+
getConfigurationStub.returns(
147+
createMockConfig({
148+
useEnvFile: false,
149+
envFilePath: '${workspaceFolder}/.env',
150+
}) as WorkspaceConfiguration,
151+
);
152+
envVarManager
153+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
154+
.returns(() => Promise.resolve({ DATABASE_URL: 'postgres://localhost/db' }));
155+
156+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
157+
await new Promise((resolve) => setTimeout(resolve, 50));
158+
159+
assert.strictEqual(mockScopedCollection.replace.called, false);
160+
});
161+
162+
test('should NOT inject when useEnvFile is false with multiple workspace folders', async () => {
163+
const workspace1 = createMockWorkspaceFolder('/workspace1', 'workspace1', 0);
164+
const workspace2 = createMockWorkspaceFolder('/workspace2', 'workspace2', 1);
165+
workspaceFoldersValue = [workspace1, workspace2];
166+
167+
getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration);
168+
envVarManager
169+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
170+
.returns(() => Promise.resolve({ VAR1: 'value1' }));
171+
172+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
173+
await new Promise((resolve) => setTimeout(resolve, 100));
174+
175+
assert.strictEqual(mockScopedCollection.replace.called, false);
176+
});
177+
178+
test('should handle no workspace folders gracefully', async () => {
179+
workspaceFoldersValue = [];
180+
getConfigurationStub.returns(createMockConfig({ useEnvFile: false }) as WorkspaceConfiguration);
181+
envVarManager
182+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
183+
.returns(() => Promise.resolve({ VAR: 'value' }));
184+
185+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
186+
await new Promise((resolve) => setTimeout(resolve, 50));
187+
188+
assert.strictEqual(mockScopedCollection.replace.called, false);
189+
});
190+
});
191+
192+
suite('python.envFile compatibility', () => {
193+
test('python.envFile has no effect when useEnvFile is false', async () => {
194+
getConfigurationStub.returns(
195+
createMockConfig({
196+
useEnvFile: false,
197+
envFilePath: '${workspaceFolder}/.env',
198+
}) as WorkspaceConfiguration,
199+
);
200+
envVarManager
201+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
202+
.returns(() => Promise.resolve({ PRODUCTION_API_KEY: 'prod_key_123' }));
203+
204+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
205+
await new Promise((resolve) => setTimeout(resolve, 50));
206+
207+
assert.strictEqual(mockScopedCollection.replace.called, false);
208+
});
209+
210+
test('different envFile paths should not matter when useEnvFile is false', async () => {
211+
const pathConfigs = [undefined, '', '.env', '${workspaceFolder}/.env', '/absolute/path/.env'];
212+
213+
for (const envFilePath of pathConfigs) {
214+
mockScopedCollection.replace.resetHistory();
215+
getConfigurationStub.returns(
216+
createMockConfig({ useEnvFile: false, envFilePath }) as WorkspaceConfiguration,
217+
);
218+
219+
envVarManager.reset();
220+
envVarManager
221+
.setup((m) => m.onDidChangeEnvironmentVariables)
222+
.returns(() => createMockEvent());
223+
envVarManager
224+
.setup((m) => m.getEnvironmentVariables(typeMoq.It.isAny()))
225+
.returns(() => Promise.resolve({ VAR: 'value' }));
226+
227+
injector = new TerminalEnvVarInjector(envVarCollection.object, envVarManager.object);
228+
await new Promise((resolve) => setTimeout(resolve, 50));
229+
230+
assert.strictEqual(mockScopedCollection.replace.called, false, `Failed for envFilePath="${envFilePath}"`);
231+
232+
try {
233+
injector.dispose();
234+
} catch {
235+
// Ignore
236+
}
237+
}
238+
});
239+
});
240+
});

0 commit comments

Comments
 (0)