Skip to content

Commit 628c729

Browse files
committed
Preserve per-script routing after PR6 merge
Keep exact script-project settings authoritative while routing active inline selections ahead of containing-project defaults, and retain strict PEP 440 validation for persisted associations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1a9f6ba1-9bd3-4664-bc25-a0d34d7a2e91
1 parent 97c4b4d commit 628c729

4 files changed

Lines changed: 124 additions & 15 deletions

File tree

src/features/envManagers.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
EditAllManagerSettings,
3939
getDefaultEnvManagerSetting,
4040
getDefaultPkgManagerSetting,
41+
getProjectEnvironmentManagerSetting,
4142
setAllManagerSettings,
4243
} from './settings/settingHelpers';
4344

@@ -195,10 +196,11 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
195196
* Returns the environment manager for the given context.
196197
*
197198
* Priority:
198-
* 1. Use the default from settings (user-configured takes precedence)
199-
* 2. If no user-configured setting, fall back to cached environment's manager
200-
* 3. If context is a string (manager ID), return that manager directly
201-
* 4. If context is a PythonEnvironment, return its manager
199+
* 1. Use an exact per-script project setting.
200+
* 2. Use a cached per-script inline selection.
201+
* 3. Use the containing project or default setting.
202+
* 4. Fall back to the cached project/global environment's manager.
203+
* 5. If context is a string or PythonEnvironment, return its manager directly.
202204
*/
203205
public getEnvironmentManager(context: EnvironmentManagerScope): InternalEnvironmentManager | undefined {
204206
if (this._environmentManagers.size === 0) {
@@ -207,7 +209,31 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
207209
}
208210

209211
if (context === undefined || context instanceof Uri) {
210-
// First check settings - user-configured settings always take priority
212+
const project = context ? this.pm.get(context) : undefined;
213+
if (
214+
context instanceof Uri &&
215+
project &&
216+
normalizePath(project.uri.fsPath) === normalizePath(context.fsPath)
217+
) {
218+
const exactManagerId = getProjectEnvironmentManagerSetting(this.pm, context);
219+
const exactManager = exactManagerId
220+
? this._environmentManagers.get(exactManagerId)
221+
: undefined;
222+
if (exactManager) {
223+
return exactManager;
224+
}
225+
}
226+
227+
if (context instanceof Uri) {
228+
const inlineEnv = this._activeSelection.get(this.getInlineScriptSelectionKey(context));
229+
if (inlineEnv?.envId.managerId === INLINE_SCRIPT_MANAGER_ID) {
230+
const inlineManager = this._environmentManagers.get(INLINE_SCRIPT_MANAGER_ID);
231+
if (inlineManager) {
232+
return inlineManager;
233+
}
234+
}
235+
}
236+
211237
const defaultEnvManagerId = getDefaultEnvManagerSetting(this.pm, context);
212238
if (defaultEnvManagerId !== undefined) {
213239
const settingsManager = this._environmentManagers.get(defaultEnvManagerId);
@@ -216,13 +242,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
216242
}
217243
}
218244

219-
// Fall back to cached environment's manager if no user-configured settings
220-
const project = context ? this.pm.get(context) : undefined;
221-
const cachedEnv =
222-
(context instanceof Uri
223-
? this._activeSelection.get(this.getInlineScriptSelectionKey(context))
224-
: undefined) ??
225-
this._activeSelection.get(project ? project.uri.toString() : 'global');
245+
const cachedEnv = this._activeSelection.get(project ? project.uri.toString() : 'global');
226246
if (cachedEnv) {
227247
const cachedManager = this._environmentManagers.get(cachedEnv.envId.managerId);
228248
if (cachedManager) {
@@ -343,6 +363,9 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
343363
const project = scope ? this.pm.get(scope) : undefined;
344364
const key = this.getActiveSelectionKey(scope, manager, project);
345365
await manager.set(scope, environment);
366+
if (scope instanceof Uri) {
367+
this.clearInlineActiveSelection(scope, manager);
368+
}
346369
this.bumpSelectionRevision(key);
347370

348371
// Only persist to settings when explicitly requested
@@ -422,6 +445,7 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
422445
if (Array.isArray(scope) && scope.every((s) => s instanceof Uri)) {
423446
await manager.set(scope, environment);
424447
scope.forEach((uri) => {
448+
this.clearInlineActiveSelection(uri, manager);
425449
const project = this.pm.get(uri);
426450
this.bumpSelectionRevision(this.getActiveSelectionKey(uri, manager, project));
427451
});
@@ -670,6 +694,16 @@ export class PythonEnvironmentManagers implements EnvironmentManagers {
670694
return `inline-script:${normalizePath(scope.fsPath)}`;
671695
}
672696

697+
private clearInlineActiveSelection(scope: Uri, manager: InternalEnvironmentManager): void {
698+
if (manager.id === INLINE_SCRIPT_MANAGER_ID) {
699+
return;
700+
}
701+
const key = this.getInlineScriptSelectionKey(scope);
702+
if (this._activeSelection.delete(key)) {
703+
this.bumpSelectionRevision(key);
704+
}
705+
}
706+
673707
private canPersistManagerSettingForScope(
674708
scope: Uri,
675709
manager: InternalEnvironmentManager,

src/features/settings/settingHelpers.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ function getSettings(
2828
return undefined;
2929
}
3030

31+
export function getProjectEnvironmentManagerSetting(
32+
wm: PythonProjectManager,
33+
scope: Uri,
34+
): string | undefined {
35+
const config = workspaceApis.getConfiguration('python-envs', scope);
36+
const setting = getSettings(wm, config, scope)?.envManager;
37+
return setting ? setting : undefined;
38+
}
39+
3140
let DEFAULT_ENV_MANAGER_BROKEN = false;
3241
let hasShownDefaultEnvManagerBrokenWarn = false;
3342

src/test/features/envManagers.lastKnown.unit.test.ts

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
3232
let envManagers: PythonEnvironmentManagers;
3333
let projectManager: typeMoq.IMock<PythonProjectManager>;
3434
let projectsByUri: Map<string, PythonProject>;
35+
let defaultManagerId: string;
36+
let exactManagerSettings: Map<string, string>;
3537

3638
function makeEnv(id: string): PythonEnvironment {
3739
const envId: PythonEnvironmentId = { id, managerId: 'test-manager' };
@@ -61,11 +63,16 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
6163
projectManager = typeMoq.Mock.ofType<PythonProjectManager>();
6264
setupNonThenable(projectManager);
6365
projectsByUri = new Map();
66+
exactManagerSettings = new Map();
6467
projectManager
6568
.setup((pm) => pm.get(typeMoq.It.isAny()))
6669
.returns((uri) => projectsByUri.get(uri.toString()));
6770

6871
envManagers = new PythonEnvironmentManagers(projectManager.object);
72+
sinon.stub(settingHelpers, 'getDefaultEnvManagerSetting').callsFake(() => defaultManagerId);
73+
sinon
74+
.stub(settingHelpers, 'getProjectEnvironmentManagerSetting')
75+
.callsFake((_manager, uri) => exactManagerSettings.get(uri.toString()));
6976
});
7077

7178
teardown(() => {
@@ -93,10 +100,10 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
93100
refresh: async () => undefined,
94101
} as unknown as EnvironmentManager;
95102

103+
const managerIndex = envManagers.managers.length;
96104
envManagers.registerEnvironmentManager(manager);
97-
const id = envManagers.managers[0].id;
98-
// Force the default environment manager (used for undefined/global scope) to resolve to ours.
99-
sinon.stub(settingHelpers, 'getDefaultEnvManagerSetting').returns(id);
105+
const id = envManagers.managers[managerIndex].id;
106+
defaultManagerId = id;
100107
return id;
101108
}
102109

@@ -250,6 +257,53 @@ suite('PythonEnvironmentManagers getLastKnownEnvironment', () => {
250257
assert.strictEqual(envManagers.getLastKnownEnvironment(secondUri), second);
251258
});
252259

260+
test('routes an active inline-script selection before the containing project default', async () => {
261+
const script = Uri.file('/workspace/project/script.py');
262+
projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') });
263+
const defaultId = registerManager(async () => makeEnv('default'), async () => undefined, 'venv');
264+
let inlineEnvironment: PythonEnvironment;
265+
const inlineId = registerManager(async () => inlineEnvironment, async () => undefined, 'inline-script');
266+
inlineEnvironment = { ...makeEnv('inline'), envId: { id: 'inline', managerId: inlineId } };
267+
defaultManagerId = defaultId;
268+
269+
await envManagers.setEnvironment(script, inlineEnvironment, false);
270+
271+
assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, inlineId);
272+
assert.strictEqual(await envManagers.getEnvironment(script), inlineEnvironment);
273+
});
274+
275+
test('lets an exact script project setting override an active inline selection', async () => {
276+
const script = Uri.file('/workspace/script.py');
277+
projectsByUri.set(script.toString(), { name: 'script.py', uri: script });
278+
const selectedId = registerManager(async () => makeEnv('selected'), async () => undefined, 'venv');
279+
let inlineEnvironment: PythonEnvironment;
280+
const inlineId = registerManager(async () => inlineEnvironment, async () => undefined, 'inline-script');
281+
inlineEnvironment = { ...makeEnv('inline'), envId: { id: 'inline', managerId: inlineId } };
282+
defaultManagerId = selectedId;
283+
284+
await envManagers.setEnvironment(script, inlineEnvironment, false);
285+
exactManagerSettings.set(script.toString(), selectedId);
286+
287+
assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, selectedId);
288+
});
289+
290+
test('clears active inline routing after selecting a different manager', async () => {
291+
const script = Uri.file('/workspace/project/script.py');
292+
projectsByUri.set(script.toString(), { name: 'project', uri: Uri.file('/workspace/project') });
293+
let selectedEnvironment: PythonEnvironment;
294+
const selectedId = registerManager(async () => selectedEnvironment, async () => undefined, 'venv');
295+
let inlineEnvironment: PythonEnvironment;
296+
const inlineId = registerManager(async () => inlineEnvironment, async () => undefined, 'inline-script');
297+
selectedEnvironment = { ...makeEnv('selected'), envId: { id: 'selected', managerId: selectedId } };
298+
inlineEnvironment = { ...makeEnv('inline'), envId: { id: 'inline', managerId: inlineId } };
299+
defaultManagerId = selectedId;
300+
301+
await envManagers.setEnvironment(script, inlineEnvironment, false);
302+
await envManagers.setEnvironment(script, selectedEnvironment, false);
303+
304+
assert.strictEqual(envManagers.getEnvironmentManager(script)?.id, selectedId);
305+
});
306+
253307
test('does not persist an inline-script manager for the containing project', async () => {
254308
const script = Uri.file('/workspace/project/script.py');
255309
const containingProject = { name: 'project', uri: Uri.file('/workspace/project') };

src/test/managers/builtin/inlineScript/envManager.unit.test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,6 +1631,18 @@ suite('InlineScriptEnvManager', () => {
16311631
assert.strictEqual(await manager.get(uri), environment);
16321632
});
16331633

1634+
test('uses full PEP 440 semantics when validating a retained association', async () => {
1635+
const uri = scriptUri();
1636+
const environment = {
1637+
...(await createOwnedEnvironment()),
1638+
version: '3.15.0',
1639+
};
1640+
await manager.set(uri, environment);
1641+
readMetadataStub.resolves({ ...VALID_METADATA, requiresPython: '!=3.15.0rc2' });
1642+
1643+
assert.strictEqual(await manager.get(uri), environment);
1644+
});
1645+
16341646
test('does not resolve or discard an association when metadata is absent or unreadable', async () => {
16351647
const uri = scriptUri();
16361648
const environment = await createOwnedEnvironment();

0 commit comments

Comments
 (0)