feat: offer inline-script env setup as an unresolved-import quick fix - #31
Closed
StellaHuang95 wants to merge 1 commit into
Closed
StellaHuang95 wants to merge 1 commit into
StellaHuang95 wants to merge 1 commit into
Conversation
The only discovery surface for PEP 723 inline-script environment setup was
a CodeLens, and codeLens.ts hides it whenever the document is dirty. At the
moment a user types `import requests` and sees a red squiggle, the CodeLens
is gone, so users who do not already know about PEP 723 never find the
feature.
Register a CodeActionProvider that offers "Set up this script's Python
environment" when an unresolved-import diagnostic sits in a `.py` file that
declares a `# /// script` block and has no inline-script environment yet.
The provider parses the in-memory buffer, so it works while dirty, and the
command handler saves the document before setup because the environment
manager reads metadata from disk.
Gates run cheapest-first, before any parsing, because provideCodeActions can
fire on cursor movement: feature flag, then a matching diagnostic, then the
routing key, then shouldRoute, and only then the header parse.
Diagnostics are matched on `code` only, never `source` (Pyrefly-backed
Pylance reports its source as the literal "pylance + pyrefly"). Pyright, Ty,
Pyrefly and mypy dialects are all covered, including the {value, target}
object form of Diagnostic.code. reportMissingModuleSource is included
deliberately, unlike in Pylance's own isMissingImportDiagnostic: a stub
without source means the package is not installed, which setup fixes.
The action deliberately leaves `diagnostics` and `isPreferred` unset. Setup
installs the block's declared dependencies verbatim and may not resolve the
import at all, so the action must not claim to fix the diagnostic or
pre-empt a real import fix. The title promises environment setup, nothing
more.
Setup also now seeds routing metadata after saving a dirty document. Without
it a just-typed block goes from no metadata to an identity while `create`
runs, which setUpInlineScriptEnvironment reads as a concurrent edit and
silently skips the association. The companion-extension prompt moves out of
the setup try block so a failure there is no longer reported to the user as
a setup failure.
The feature stays behind the internal python-envs.inlineScripts.enabled
flag, so none of this is user-visible yet.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Owner
Author
|
Reopening against the upstream repo (microsoft/vscode-python-environments) instead. Superseded. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The PEP 723 inline-script feature has exactly one discovery surface — a CodeLens — and
provideCodeLensesreturns[]whenever the document is dirty (src/features/inlineScript/codeLens.ts:51).So the CodeLens is absent at the one moment a user most needs it: right after typing
import requestsand seeing the red squiggle. Users who don't already know PEP 723 never discover the feature at all.Change
Register a
CodeActionProvider(src/features/inlineScript/setupCodeAction.ts) that offers "Set up this script's Python environment" as a quick fix when:.pythat could carry an inline-script environment,routing.shouldRoute(uri)is false), and# /// scriptblock.Gates are ordered cheapest-first because
provideCodeActionscan fire on cursor movement — thecontext.diagnosticsscan happens before any parsing, so the common case (no unresolved import) costs nothing but a short array scan.Metadata is parsed from the in-memory buffer, so unlike the CodeLens this works while the document is dirty.
Honest labeling — wording and mechanics
The title says what the action does, not that the squiggle will clear. It may well not: the unresolved module might be undeclared in the block, or declared under a different distribution name (
PILvspillow). Correspondingly:action.diagnosticsis left unset — populating it tells VS Code the action resolves those diagnostics and opts it into fix-all affordances.action.isPreferredis left unset, so it never pre-empts a genuine import fix such as "add import".There is deliberately no import-name → package-name mapping. Setup installs the block's declared
dependenciesverbatim; the import name is never an input.Diagnostic codes
Matched on
codeonly, neversource— Pyrefly-backed Pylance reports its source as the literal stringpylance + pyrefly, so any source allow-list would be wrong somewhere.Diagnostic.codeisstring | number | {value, target}; the union is normalized and lowercased before comparison.reportMissingImports,reportMissingModuleSourceunresolved-import,possibly-missing-importmissing-import,missing-source,missing-source-for-stubsimport-not-found,import-untypedreportMissingModuleSourceis included on purposePylance's own
isMissingImportDiagnostic()excludes it — and rightly so for their fix, since a stub-resolved module is already spelled correctly and has nothing for a "change spelling" action to suggest. For us the meaning is the opposite kind of useful: a stub was found but the source was not, i.e. the package isn't installed — precisely what setting up the environment fixes.Reappears after a block edit — intentional
Once the script is set up the action disappears. If the user then edits the
# /// scriptblock (e.g. to add the dependency that was missing), it comes back by itself:InlineScriptRoutingRegistry.setMetadataresetsvalidatedAssociationtofalsewhen the metadata identity changes (src/common/inlineScript/routingRegistry.ts:74-75). This is desired behavior and is covered by a test.Save before setup — and a race it closes
setUpInlineScriptEnvironmentresolves the block from disk viareadInlineScriptMetadataFromFile. Since the quick fix is offered on a dirty buffer, the handler now saves first.Saving alone wasn't enough.
setUpInlineScriptEnvironmentcompares the metadata identity before and aftercreate, and skips the association if it changed mid-setup. For a block the user had just typed, routing metadata is stillundefined— the detector's own save handler would then populate it duringcreate, the identity would goundefined → X, and the association would be silently skipped. The user would wait for an environment and get nothing.saveScriptBeforeSetup()closes this by re-reading the file it just wrote and seedingrouting.setMetadata(). The detector's later write computes the same identity from the same bytes, so the identity stays stable.Tests
19 tests in
setupCodeAction.unit.test.tsplus a 6-testsetupInlineScriptEnvironmentHandlersuite insetupEnvironment.unit.test.ts. Coverage includes every gating branch, all four diagnostic dialects, the{value, target}object form, case-insensitivity, thepylance + pyreflysource,diagnostics/isPreferredleft unset, reappear-after-block-edit, in-memory parse while dirty, and the header byte bound.Guard tests verified to fail without their guards
Per the convention established in microsoft#1772, each guard was removed one at a time and the covering test confirmed to fail — not merely to pass today. All 11 were detected:
.py) gateshouldRoute) gateaction.diagnosticsleft unsetaction.isPreferredleft unsetTwo findings this produced, both worth calling out because they were caught by the mutation run rather than by the tests passing:
promptUpdateExtensionsForInlineScripts()originally sat inside the setuptry, so a prompt failure showed the user a "setup failed" message even though the environment had been created successfully. It's now outside the try with its own.catch, mirroring what the bulk path already did.does not run setup when the document could not be savedoriginally still passed with the guard removed:manager.createwas unconfigured, sosetEnvironmentwas never called either way, and the error message it asserted was being emitted by the notify path instead. It now configures a would-succeed setup and assertsmanager.createwas never called plus the specific save-failure message.Notes
python-envs.setupInlineScriptEnvremains out ofpackage.jsonby design; it's invoked only by the two UI surfaces.ms-python.pythonor Pylance are needed.isInlineScriptsFeatureEnabled()is re-read per call in the provider, whereas registration is latched at activation. Flipping the setting mid-session without a reload makes the code action go inert while the CodeLens keeps showing. That's the safer direction, and the cheapest-first gate ordering requires the per-call check.Validation
npm run lint✓ ·npm run compile-tests✓ ·npm run unittest✓ (2323 passing, 6 pending, 0 failing)Nothing here is user-visible: the whole surface stays behind the undeclared internal flag
python-envs.inlineScripts.enabled, which defaults tofalse.