Repository navigation
fix(lsp): offer Run and Evaluate code lenses only on model files on disk - #1838
Merged
Merged
Conversation
The VS Code runner reads the element's file from disk and refuses any lens whose URI is not a file: URI, so a Run or Evaluate lens anywhere else could only fail. CodeLens now emits the run command only for a file: document, and a notebook cell, answered from the notebook's own file: document, drops its run lenses since the kernel runs a cell. Reference-count lenses are unchanged. Co-Authored-By: jason.han <hanhuijun@gmail.com>
Contributor
Author
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
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.
What and why
Follow-up to #1833. The server offered Run/Evaluate code lenses on documents the VS Code runner cannot read:
run()ineditors/vscode/src/run.tsrefuses any lens whoseuriis not afile:URI ("… is not declared in a file on disk, so sysml cannot read it."), so on those documents every Run/Evaluate lens was a button that could only fail.internal/frontend/lsp/codelens.goCodeLens: therunCommandlens is emitted only whenparams.TextDocument.URIhas thefilescheme (onDisk). Reference-count lenses are unchanged.internal/frontend/lsp/notebook_requests.gocellRequest(textDocument/codeLens): drops lenses whoseCommand.Command == runElementCommand. The kernel runs a cell, so a cell gets only its reference-count lenses, which still resolve throughcodeLens/resolvein the cell's terms.run.tskeeps its client-side check unchanged, as a defensive fallback.Which guard removes the cell lenses. Guard 1 alone does not cover cells.
cellRequestcallsCodeLenswithnameToURI(nb.name), which isuri.File(...)of the notebook's path, so it is always afile:URI. With only guard 1 applied, the new notebook test failed: the cell's answer still hadRun Go/Evaluate Arealenses with"uri":"file:///…/model.ipynb". TherunElementCommandfilter incellRequestis what removes them, so it is not redundant.Which URIs reach
CodeLensfrom VS Code.editors/vscode/src/selector.tsdocumentSelectorregisters{scheme: "file"},{scheme: stdlibScheme}(library documents, whichCodeLensalready answers with nothing) and{notebook: "*"}cells. It does not includeuntitled, so from VS Code an untitled editor never reaches the server, and a cell's request reachesCodeLensthroughcellRequestwith the notebook'sfile:URI. The scheme guard is still right for other LSP clients that send non-file URIs (untitled:, virtual file systems), and the new test locks it in withuntitled:Untitled-1.Docs:
docs/reference/lsp.md(the codeLens-on-a-cell sentence),docs/guide/12-jupyter.md(what a cell has),docs/guide/08-editors.md(the running-an-element paragraph and the code-lens feature row) andeditors/vscode/README.md(notebook cells, "Running an element from a code lens") now say that Run/Evaluate lenses are offered only for model files on disk, that the kernel runs a cell, and that reference-count lenses work in cells. Changelog:changes/unreleased/notebook-run-lenses.fixed.md.How it was verified
New tests in
internal/frontend/lsp:TestNotebookCellLensesCountReferencesAndRunNothing(notebook_test.go): a cell withpart def A,action def Goandcalc def Area, plus a second cell using them. The cell'stextDocument/codeLensanswer has reference lenses onP::A,P::GoandP::Areaand noopensysml.runElementlens, andcodeLens/resolveonP::Agives1 referencelocated in the second cell. It fails with guard 1 alone (see above).TestCodeLensRunsOnlyFilesOnDisk(editor_facts_test.go):editorFactsModelopened asuntitled:Untitled-1gets reference lenses onDemo::Fall,Demo::Rover,Demo::Charge,Demo::MissionandDemo::Positive, and no command lens. With the guard forced off it fails (lens "Evaluate Fall" (opensysml.runElement) on a document not on disk, …).TestCodeLensOffersReferenceCountsAndRunCommands,TestCodeLensResolveCountsReferences,TestCodeLensTellsOverloadsApart,TestRunServesSignatureHelpCodeLensAndInlayHints) are unchanged and pass, so afile:document still gets Run/Evaluate.No TypeScript was touched, so the extension's
npm testwas not run.Checklist
make lintpasses locally; the touched packages' tests pass (go test ./internal/frontend/...,./tests/hygiene/...); the fullmake testis left to CIchanges/unreleased/<slug>.<section>.md, not as an edit toCHANGELOG.mdmake docs-countsrun if a gate count moved (compliance rows need nothing: the census is counted at docs build). No gate count moved (doc-counts -checkis current)F4,K5) in the body, docs, or changelog