Skip to content

Commit 33bf91f

Browse files
feat: confirm inline-script env setup with a short-lived CodeLens (#1790)
## Problem Setting up a PEP 723 inline-script environment signals success only by the setup CodeLens **disappearing** — which is indistinguishable from the lens never having been offered in the first place. It also leaves the chosen base interpreter invisible, which matters when `requires-python` matches several installed Pythons, or when one was installed on demand via `uv python install`. ## Change For five seconds after setup succeeds, the (now hidden) setup lens is replaced by a passive confirmation anchored at the `# /// script` block: > `Script environment ready (Python 3.12.4)` then it expires on its own. The version comes from `shortenVersionString(environment.version)` — the same helper `getPythonInfo` uses for `script env (3.12.4)` — so the lens and the environment's own name cannot disagree. An empty command id renders the title as plain, non-clickable text: this is a statement, not an action. ### No extra reflow The line is already occupied by the setup lens at that moment, so the confirmation does not add a shift — it delays the single existing one by five seconds. This was the main objection to an earlier draft, and it turned out not to apply. ## Scope The entire behavioural change is one new row in `provideCodeLenses`: | `isDirty` | metadata | `shouldRoute` | confirmation live | Before | After | |---|---|---|---|---|---| | yes | — | — | — | `[]` | `[]` | | no | none | — | — | `[]` | `[]` | | no | yes | yes | **yes** | `[]` | **confirmation** | | no | yes | yes | no | `[]` | `[]` | | no | yes | no | — | setup lens | setup lens | Everything else is byte-identical: the dirty guard, the metadata check, `shouldRoute` semantics, and the setup lens title, command, arguments and anchor. `routingRegistry.ts`, `envManager.ts`, `window.apis.ts`, settings and `package.json` are untouched. Both setup surfaces — the CodeLens and the unresolved-import quick fix from #1788 — show the confirmation, since both route through `python-envs.setupInlineScriptEnv`. The bulk command deliberately does not: it already ends with its own `Set up {0} of {1} ...` summary and can process scripts that are not open. ## Implementation notes - `noteEnvironmentReady` cancels any existing timer for the script first, so re-running setup restarts the window rather than inheriting a nearly-expired one. - Entries are keyed by `getInlineScriptRoutingKey`, so on Windows `C:\App.py` and `c:\app.py` share one entry. - `dispose()` clears every pending timer; a test asserts nothing fires afterwards. - Confirmations are in-memory and per-window, deliberately not persisted — a confirmation is about an action you just took. ## Known limitation If the file is edited **while** the environment builds, `getSavedMetadataForPersistence` returns `{}` for the dirty document, so `updateValidatedStateForSelection` leaves `shouldRoute` false and the confirmation is unreachable until the next save — and lost entirely if that takes longer than five seconds. Showing it anyway would mean confirming an association that is not validated, so the miss is preferable to the lie. Expiry also depends on VS Code re-querying after `onDidChangeCodeLenses`. That is normally immediate, but the lens can outlast five seconds slightly under load. It always clears. ## Tests Six new provider tests on `sinon.useFakeTimers()` (no wall-clock dependency): shows with version, omits version when unresolved, expires and fires exactly one refresh, routed-but-not-just-set-up shows nothing, hidden while dirty, and no timer leak past `dispose()`. Three handler tests cover the callback firing on success and staying silent when creation returns nothing or throws. The five pre-existing CodeLens tests are unmodified and still pass — including *"hides the CodeLens once a validated association makes the script routeable"*, which is the proof that the default routed path is unchanged. `npm run lint` OK, `npm run compile-tests` OK, `npm run unittest` OK (2331 passing, 6 pending, 0 failing) > `discovers a build that completes after the short retry window` is a pre-existing flake unrelated to this change — verified by stashing these changes and running it on a clean tree, where it failed 2 of 4 runs. **Nothing here is user-visible by default**: the whole surface stays behind the undeclared internal flag `python-envs.inlineScripts.enabled`, which defaults to `false`. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 3742822 commit 33bf91f

6 files changed

Lines changed: 175 additions & 17 deletions

File tree

docs/managing-python-projects.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,8 @@ When you create a script, the extension generates a single `.py` file with PEP 7
9999

100100
An inline-script environment is built from the script's `# /// script` block and stored in the extension's cache, where it is shared by every script with the same dependencies and base interpreter. Because editing one would silently change the others, these environments are not user-managed: the Python Environments views do not offer install, uninstall, or version-change actions for them. Their package list remains visible.
101101

102+
A CodeLens above the `# /// script` block offers **Set up environment for this script**, and the same action is available as a quick fix on an unresolved import. For a few seconds after setup succeeds it is replaced by a **Script environment ready (Python X.Y.Z)** confirmation naming the Python that was selected — useful when `requires-python` matches several installed versions, or when one was installed on demand. The confirmation is plain text rather than a clickable action, and it expires on its own; at every other time the setup CodeLens behaves exactly as before.
103+
102104
Setup records which distributions it installed. If that record and the environment's contents later disagree — for example after installing a package into it from a terminal — every script sharing the environment needs setup again. Saving or reopening a script does not repair it; use the script's setup action to rebuild from its declared dependencies.
103105

104106
Once a mismatch is confirmed during an environment lookup, the affected scripts' setup actions return without requiring a save.

src/common/localize.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,13 @@ export namespace InlineScriptStrings {
4444

4545
export const diagnosticSource = l10n.t('Python Environments');
4646

47+
export function environmentReady(version: string | undefined): string {
48+
const shown = version?.trim();
49+
return shown
50+
? l10n.t('Script environment ready (Python {0})', shown)
51+
: l10n.t('Script environment ready');
52+
}
53+
4754
export const unterminatedBlock = l10n.t(
4855
"This '# /// script' block is missing its closing '# ///' marker, so its inline script metadata is ignored.",
4956
);

src/features/inlineScript/codeLens.ts

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,12 @@ import {
1111
languages,
1212
Range,
1313
TextDocument,
14+
Uri,
1415
} from 'vscode';
15-
import { InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry';
16+
import { getInlineScriptRoutingKey, InlineScriptRoutingRegistry } from '../../common/inlineScript/routingRegistry';
17+
import { InlineScriptStrings } from '../../common/localize';
18+
19+
export const READY_CONFIRMATION_TIMEOUT_MS = 5000;
1620

1721
/**
1822
* Shows a single "Set up environment for this script" CodeLens above a `.py` file's PEP 723
@@ -29,10 +33,16 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
2933
private readonly _onDidChangeCodeLenses = new EventEmitter<void>();
3034
public readonly onDidChangeCodeLenses = this._onDidChangeCodeLenses.event;
3135
private readonly subscriptions: Disposable[] = [];
36+
private readonly readyConfirmations = new Map<
37+
string,
38+
{ readonly version: string | undefined; readonly timer: ReturnType<typeof setTimeout> }
39+
>();
40+
private disposed = false;
3241

3342
constructor(
3443
private readonly routing: InlineScriptRoutingRegistry,
3544
private readonly setupCommand: string,
45+
private readonly confirmationTimeoutMs: number = READY_CONFIRMATION_TIMEOUT_MS,
3646
) {
3747
this.subscriptions.push(
3848
this.routing.onDidChangeRouteability(() => this._onDidChangeCodeLenses.fire()),
@@ -47,6 +57,22 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
4757
);
4858
}
4959

60+
public noteEnvironmentReady(uri: Uri, version: string | undefined): void {
61+
const key = getInlineScriptRoutingKey(uri);
62+
if (this.disposed || !key) {
63+
return;
64+
}
65+
this.clearConfirmation(key);
66+
this.readyConfirmations.set(key, {
67+
version,
68+
timer: setTimeout(() => {
69+
this.readyConfirmations.delete(key);
70+
this._onDidChangeCodeLenses.fire();
71+
}, this.confirmationTimeoutMs),
72+
});
73+
this._onDidChangeCodeLenses.fire();
74+
}
75+
5076
public provideCodeLenses(document: TextDocument, _token: CancellationToken): CodeLens[] {
5177
if (document.isDirty) {
5278
// The association is validated against the saved file (the manager refuses to validate a
@@ -60,13 +86,24 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
6086
// No saved PEP 723 metadata (or it is currently being edited).
6187
return [];
6288
}
63-
if (this.routing.shouldRoute(uri)) {
64-
// A validated inline-script environment matching the current metadata already exists.
65-
return [];
66-
}
6789
const offset = metadata.sourceRange?.start ?? metadata.range.start;
6890
const position = document.positionAt(offset);
6991
const range = new Range(position, position);
92+
if (this.routing.shouldRoute(uri)) {
93+
// A validated inline-script environment matching the current metadata already exists.
94+
const key = getInlineScriptRoutingKey(uri);
95+
const confirmation = key ? this.readyConfirmations.get(key) : undefined;
96+
if (!confirmation) {
97+
return [];
98+
}
99+
// An empty command id renders the title as plain, non-clickable text.
100+
return [
101+
new CodeLens(range, {
102+
title: InlineScriptStrings.environmentReady(confirmation.version),
103+
command: '',
104+
}),
105+
];
106+
}
70107
return [
71108
new CodeLens(range, {
72109
title: l10n.t('Set up environment for this script'),
@@ -77,21 +114,38 @@ export class InlineScriptCodeLensProvider implements CodeLensProvider, Disposabl
77114
}
78115

79116
public dispose(): void {
117+
this.disposed = true;
80118
this.subscriptions.forEach((s) => s.dispose());
81119
this.subscriptions.length = 0;
120+
this.readyConfirmations.forEach((confirmation) => clearTimeout(confirmation.timer));
121+
this.readyConfirmations.clear();
82122
this._onDidChangeCodeLenses.dispose();
83123
}
124+
125+
private clearConfirmation(key: string): void {
126+
const existing = this.readyConfirmations.get(key);
127+
if (existing) {
128+
clearTimeout(existing.timer);
129+
this.readyConfirmations.delete(key);
130+
}
131+
}
84132
}
85133

86134
/**
87135
* Register the inline-script CodeLens provider for local `.py` files. Only called when the PEP 723
88136
* inline-script feature flag is enabled, so it is a no-op for everyone else.
89137
*/
90-
export function registerInlineScriptCodeLens(routing: InlineScriptRoutingRegistry, setupCommand: string): Disposable {
138+
export function registerInlineScriptCodeLens(
139+
routing: InlineScriptRoutingRegistry,
140+
setupCommand: string,
141+
): { readonly disposable: Disposable; readonly provider: InlineScriptCodeLensProvider } {
91142
const provider = new InlineScriptCodeLensProvider(routing, setupCommand);
92143
const registration = languages.registerCodeLensProvider({ scheme: 'file', language: 'python' }, provider);
93-
return new Disposable(() => {
94-
registration.dispose();
95-
provider.dispose();
96-
});
144+
return {
145+
provider,
146+
disposable: new Disposable(() => {
147+
registration.dispose();
148+
provider.dispose();
149+
}),
150+
};
97151
}

src/features/inlineScript/setupEnvironment.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
} from '../../common/window.apis';
1818
import { asRelativePath, findFiles, getOpenTextDocuments } from '../../common/workspace.apis';
1919
import { EnvironmentManagers } from '../../internal.api';
20+
import { shortenVersionString } from '../../managers/common/utils';
2021
import { registerInlineScriptCodeLens } from './codeLens';
2122
import { promptUpdateExtensionsForInlineScripts } from './extensionVersionCheck';
2223
import { registerInlineScriptSetupCodeAction } from './setupCodeAction';
@@ -134,6 +135,7 @@ async function saveScriptBeforeSetup(scriptUri: Uri, routing: InlineScriptRoutin
134135
export function setupInlineScriptEnvironmentHandler(
135136
em: EnvironmentManagers,
136137
routing: InlineScriptRoutingRegistry,
138+
onEnvironmentReady?: (scriptUri: Uri, version: string | undefined) => void,
137139
): (scriptUri?: Uri) => Promise<void> {
138140
return async (scriptUri?: Uri): Promise<void> => {
139141
const uri = scriptUri ?? window.activeTextEditor?.document.uri;
@@ -164,6 +166,7 @@ export function setupInlineScriptEnvironmentHandler(
164166
notifyInlineScriptSetupOutcome(uri, routing);
165167
return;
166168
}
169+
onEnvironmentReady?.(uri, shortenVersionString(environment.version));
167170
// Kept out of the try: the environment is already set up, so a failure in this follow-up
168171
// must not be reported to the user as a setup failure.
169172
await promptUpdateExtensionsForInlineScripts().catch((error) =>
@@ -362,10 +365,16 @@ async function filterInlineScriptFiles(files: readonly Uri[]): Promise<Uri[]> {
362365
* palette-gated behind the flag.
363366
*/
364367
export function registerInlineScriptUx(em: EnvironmentManagers, routing: InlineScriptRoutingRegistry): Disposable[] {
368+
const codeLens = registerInlineScriptCodeLens(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND);
365369
return [
366-
registerInlineScriptCodeLens(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND),
370+
codeLens.disposable,
367371
registerInlineScriptSetupCodeAction(routing, SETUP_INLINE_SCRIPT_ENV_COMMAND),
368-
commands.registerCommand(SETUP_INLINE_SCRIPT_ENV_COMMAND, setupInlineScriptEnvironmentHandler(em, routing)),
372+
commands.registerCommand(
373+
SETUP_INLINE_SCRIPT_ENV_COMMAND,
374+
setupInlineScriptEnvironmentHandler(em, routing, (uri, version) =>
375+
codeLens.provider.noteEnvironmentReady(uri, version),
376+
),
377+
),
369378
commands.registerCommand(SETUP_INLINE_SCRIPT_ENVS_COMMAND, () =>
370379
setUpInlineScriptEnvironmentsInWorkspace(em, routing),
371380
),

src/test/features/inlineScript/codeLens.unit.test.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
// Licensed under the MIT License.
33

44
import assert from 'assert';
5+
import * as sinon from 'sinon';
56
import { Position, TextDocument, Uri } from 'vscode';
67
import { InlineScriptMetadata } from '../../../common/inlineScript/metadata';
78
import { InlineScriptRoutingRegistry } from '../../../common/inlineScript/routingRegistry';
8-
import { InlineScriptCodeLensProvider } from '../../../features/inlineScript/codeLens';
9+
import { InlineScriptCodeLensProvider, READY_CONFIRMATION_TIMEOUT_MS } from '../../../features/inlineScript/codeLens';
910

1011
const SETUP_COMMAND = 'python-envs.setupInlineScriptEnv';
1112

@@ -83,4 +84,65 @@ suite('Inline script CodeLens provider', () => {
8384
sub.dispose();
8485
assert.ok(fireCount >= 1, 'onDidChangeCodeLenses should fire when routing state changes');
8586
});
87+
88+
suite('post-setup confirmation', () => {
89+
let clock: sinon.SinonFakeTimers;
90+
91+
setup(() => {
92+
clock = sinon.useFakeTimers();
93+
routing.setMetadata(scriptUri, makeMetadata());
94+
routing.setValidatedAssociation(scriptUri, true);
95+
});
96+
97+
teardown(() => clock.restore());
98+
99+
test('replaces the hidden setup lens with a non-clickable confirmation naming the version', () => {
100+
provider.noteEnvironmentReady(scriptUri, '3.12.4');
101+
102+
const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never);
103+
104+
assert.strictEqual(lenses.length, 1);
105+
assert.strictEqual(lenses[0].command?.title, 'Script environment ready (Python 3.12.4)');
106+
assert.strictEqual(lenses[0].command?.command, '', 'the confirmation must not be clickable');
107+
});
108+
109+
test('omits the version when none was resolved', () => {
110+
provider.noteEnvironmentReady(scriptUri, undefined);
111+
112+
const lenses = provider.provideCodeLenses(makeDocument(scriptUri), {} as never);
113+
114+
assert.strictEqual(lenses.length, 1);
115+
assert.strictEqual(lenses[0].command?.title, 'Script environment ready');
116+
});
117+
118+
test('expires on its own and refreshes so the lens disappears', () => {
119+
provider.noteEnvironmentReady(scriptUri, '3.12.4');
120+
let fireCount = 0;
121+
const sub = provider.onDidChangeCodeLenses(() => (fireCount += 1));
122+
123+
clock.tick(READY_CONFIRMATION_TIMEOUT_MS + 1);
124+
sub.dispose();
125+
126+
assert.strictEqual(fireCount, 1, 'expiry must refresh the lenses');
127+
assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0);
128+
});
129+
130+
test('shows nothing for a routed script that was not just set up', () => {
131+
assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri), {} as never).length, 0);
132+
});
133+
134+
test('stays hidden while the document is dirty', () => {
135+
provider.noteEnvironmentReady(scriptUri, '3.12.4');
136+
137+
assert.strictEqual(provider.provideCodeLenses(makeDocument(scriptUri, true), {} as never).length, 0);
138+
});
139+
140+
test('does not leak timers past disposal', () => {
141+
provider.noteEnvironmentReady(scriptUri, '3.12.4');
142+
143+
provider.dispose();
144+
145+
assert.doesNotThrow(() => clock.tick(READY_CONFIRMATION_TIMEOUT_MS + 1));
146+
});
147+
});
86148
});

src/test/features/inlineScript/setupEnvironment.unit.test.ts

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,7 @@ suite('setupInlineScriptEnvironmentHandler', () => {
327327
let errorStub: sinon.SinonStub;
328328
let saveStub: sinon.SinonStub;
329329
let promptStub: sinon.SinonStub;
330+
let readySpy: sinon.SinonStub;
330331

331332
setup(() => {
332333
em = typemoq.Mock.ofType<EnvironmentManagers>();
@@ -338,6 +339,7 @@ suite('setupInlineScriptEnvironmentHandler', () => {
338339
errorStub = sinon.stub(winapi, 'showErrorMessage').resolves(undefined);
339340
sinon.stub(winapi, 'showInformationMessage').resolves(undefined);
340341
sinon.stub(winapi, 'showWarningMessage').resolves(undefined);
342+
readySpy = sinon.stub();
341343
promptStub = sinon.stub(extensionVersionCheck, 'promptUpdateExtensionsForInlineScripts').resolves();
342344
saveStub = sinon.stub().resolves(true);
343345
});
@@ -358,8 +360,7 @@ suite('setupInlineScriptEnvironmentHandler', () => {
358360
return env;
359361
}
360362

361-
test('saves a dirty document before setup, because setup reads the block from disk', async () => {
362-
openDirtyDocument();
363+
test('saves a dirty document before setup, because setup reads the block from disk', async () => { openDirtyDocument();
363364
expectEnvironmentCreated();
364365

365366
await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri);
@@ -417,12 +418,35 @@ suite('setupInlineScriptEnvironmentHandler', () => {
417418
sinon.assert.calledOnce(errorStub);
418419
});
419420

420-
test('does not report a failing companion-extension prompt as a setup failure', async () => {
421-
expectEnvironmentCreated();
421+
test('does not report a failing companion-extension prompt as a setup failure', async () => { expectEnvironmentCreated();
422422
promptStub.rejects(new Error('boom'));
423423

424424
await setupInlineScriptEnvironmentHandler(em.object, routing)(scriptUri);
425425

426426
sinon.assert.notCalled(errorStub);
427427
});
428+
429+
test('reports the ready environment and its Python version once setup succeeds', async () => {
430+
expectEnvironmentCreated();
431+
432+
await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri);
433+
434+
sinon.assert.calledOnceWithExactly(readySpy, scriptUri, '3.12.0');
435+
});
436+
437+
test('reports no ready environment when setup produced none', async () => {
438+
manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.resolve(undefined));
439+
440+
await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri);
441+
442+
sinon.assert.notCalled(readySpy);
443+
});
444+
445+
test('reports no ready environment when setup throws', async () => {
446+
manager.setup((m) => m.create(scriptUri, undefined)).returns(() => Promise.reject(new Error('boom')));
447+
448+
await setupInlineScriptEnvironmentHandler(em.object, routing, readySpy)(scriptUri);
449+
450+
sinon.assert.notCalled(readySpy);
451+
});
428452
});

0 commit comments

Comments
 (0)