-
Notifications
You must be signed in to change notification settings - Fork 62
In Positron, use virtual notebook (in memory) for LSP features instead of vdoc (on disk) #1115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
be7d747
bbec2bb
4bf8f9c
b37b319
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| /* | ||
| * native-features.ts | ||
| * | ||
| * Copyright (C) 2026 by Posit Software, PBC | ||
| */ | ||
|
Comment on lines
+1
to
+5
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice if you add a brief overview comment of what "native features" are. Is this the entry point for them? |
||
|
|
||
| import { commands, LogOutputChannel, workspace } from "vscode"; | ||
| import { tryAcquirePositronApi } from "@posit-dev/positron"; | ||
|
|
||
| import { EmbeddedLanguage } from "../vdoc/languages"; | ||
|
|
||
| /** | ||
| * The Positron setting that turns the virtual notebook on. Contributed by | ||
| * Positron core, not by this extension, so it is read through the full | ||
| * configuration rather than the `quarto` section we contribute. | ||
| */ | ||
| export const kNativeFeaturesSetting = "quarto.embeddedLanguageFeatures.native"; | ||
|
|
||
| /** | ||
| * Commands whose presence says this host carries the virtual notebook (see | ||
| * {@link detectNativeEmbeddedFeatures}). | ||
| * | ||
| * These are the INTERNAL ids, and the code calls the public | ||
| * `positron.executeQuartoCell*` ones. The internal ids are what a probe can | ||
| * see. The public ones are API commands, registered inside the extension host | ||
| * and deliberately never mirrored into the registry that `getCommands` reads, | ||
| * so they do not appear there at all. Neither does any | ||
| * `vscode.executeDocumentSymbolProvider`-style command, for the same reason. | ||
| * The internal commands are registered in the workbench, so they are visible, | ||
| * as long as the probe does not filter underscore-prefixed ids. | ||
| */ | ||
| const kNativeFeatureCommands = [ | ||
| "_executeQuartoCellSymbolProvider", | ||
| "_executeQuartoCellFormattingProvider", | ||
| "_executeQuartoCellRangeFormattingProvider", | ||
| ]; | ||
|
|
||
| /** | ||
| * Languages Positron is verified to serve natively. Matched against | ||
| * {@link EmbeddedLanguage.ids}, so an alias of a listed language counts too. | ||
| * | ||
| * Add one language at a time, once its cell providers have been verified end to | ||
| * end: a document that is not covered here keeps its virtual document, which is | ||
| * the safe direction. | ||
| */ | ||
| const kNativeLanguages = new Set(["r", "python"]); | ||
|
|
||
| let nativeAvailable = false; | ||
|
|
||
| /** | ||
| * Determine if this host can serve embedded language features natively. | ||
| * | ||
| * Capability detection is command presence rather than a Positron API flag or a | ||
| * version comparison. Positron registers these commands unconditionally: with | ||
| * the setting off there are no cells and they answer empty, so their presence | ||
| * tracks "this build can serve natively" exactly. Vanilla VS Code and older | ||
| * Positron builds have no such commands, so a user who pastes the setting key | ||
| * into their own `settings.json` there stays on virtual documents. | ||
| * | ||
| * Must be awaited during activation, before any gate can be consulted. | ||
| * | ||
| */ | ||
| export async function detectNativeEmbeddedFeatures( | ||
| outputChannel?: LogOutputChannel | ||
| ): Promise<void> { | ||
| if (!tryAcquirePositronApi()) { | ||
| nativeAvailable = false; | ||
| return; | ||
| } | ||
|
|
||
| // `false` keeps the underscore-prefixed ids we are looking for | ||
| const all = await commands.getCommands(false); | ||
| nativeAvailable = kNativeFeatureCommands.every((command) => | ||
| all.includes(command) | ||
| ); | ||
|
Comment on lines
+66
to
+75
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. consider pulling out a function like const isNativeAvailable = async () => {
if (!tryAcquirePositronApi()) return false;
const all = await commands.getCommands(false);
return kNativeFeatureCommands.every((command) =>
all.includes(command)
);
} |
||
|
|
||
| if (nativeAvailable) { | ||
| outputChannel?.info( | ||
| "[NativeFeatures] Host serves Quarto cell language features. " + | ||
| `The extension stands down for ${[...kNativeLanguages].join(", ")} ` + | ||
| `while ${kNativeFeaturesSetting} is on.` | ||
| ); | ||
| } else if ( | ||
| workspace.getConfiguration().get<boolean>(kNativeFeaturesSetting) === true | ||
| ) { | ||
| outputChannel?.warn( | ||
| `[NativeFeatures] ${kNativeFeaturesSetting} is on, but this host has no ` + | ||
| "Quarto cell commands. Serving embedded language features from virtual " + | ||
| "documents, which can duplicate what the host provides." | ||
| ); | ||
|
Comment on lines
+78
to
+90
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nice logs. These were very helpful in testing. |
||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Whether a language is one we let the host serve natively. Pure, so the | ||
| * language set can be tested without an extension host. | ||
| */ | ||
| export function isNativeEmbeddedLanguage(language: EmbeddedLanguage): boolean { | ||
| return language.ids.some((id) => kNativeLanguages.has(id)); | ||
| } | ||
|
|
||
| /** | ||
| * Whether the host serves embedded language features for `language`, meaning | ||
| * this extension should stand down and not serve them from a virtual document. | ||
| * | ||
| * Pass no language to ask about the document as a whole, which is what the | ||
| * whole-document commands (symbols, formatting) cover. | ||
| * | ||
| * The setting is read live on every call so that toggling it takes effect | ||
| * without a window reload. The statement range and help topic registrations in | ||
| * `lsp/client.ts` follow the setting live too, via a configuration listener. | ||
| * | ||
| * A gated pull feature answers `undefined` rather than delegating to the Quarto | ||
| * language server with `next()`. The server has nothing real to say about a code | ||
| * cell: it declares the signature help, definition, and semantic tokens | ||
| * capabilities only so that the client can intercept them with middleware, and | ||
| * its handlers answer null (see `apps/lsp/src/middleware.ts`). For semantic | ||
| * tokens delegating is worse than pointless, because the server's empty token | ||
| * stream counts as an answer and would suppress the host's own provider. | ||
| */ | ||
| export function useNativeEmbeddedFeatures(language?: EmbeddedLanguage): boolean { | ||
| if (!nativeAvailable) { | ||
| return false; | ||
| } | ||
| if (workspace.getConfiguration().get<boolean>(kNativeFeaturesSetting) !== true) { | ||
| return false; | ||
| } | ||
| return language === undefined || isNativeEmbeddedLanguage(language); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| /* | ||
| * cell-symbols.ts | ||
| * | ||
| * Copyright (C) 2026 by Posit Software, PBC | ||
| */ | ||
|
|
||
| import { | ||
| commands, | ||
| DocumentSymbol, | ||
| Range, | ||
| SymbolKind, | ||
| Uri, | ||
| } from "vscode"; | ||
|
|
||
| /** | ||
| * One code cell's symbols, as answered by | ||
| * `positron.executeQuartoCellSymbolProvider`. | ||
| */ | ||
| export interface QuartoCellSymbols { | ||
| /** The cell's code span in source coordinates, fences excluded. */ | ||
| readonly range: Range; | ||
|
|
||
| /** Already in source coordinates. Never empty. */ | ||
| readonly symbols: DocumentSymbol[]; | ||
| } | ||
|
|
||
| /** | ||
| * The symbols of every code cell in a Quarto document, grouped by cell. | ||
| * | ||
| * One request for the whole document, so callers walking a symbol tree should | ||
| * ask once and then look cells up by range with {@link nestCellSymbols}. | ||
| * | ||
| * Answers `[]` for every unservable state: a host without the command, a | ||
| * document with no cells, and a document whose cells have no language server | ||
| * attached yet. That last case is why the caller must gate on | ||
| * `useNativeEmbeddedFeatures()` rather than treat an empty answer as a reason to | ||
| * fall back, and it needs no retry: when a server does register, the editor | ||
| * re-requests document symbols on its own. | ||
| */ | ||
| export async function quartoCellSymbols( | ||
| uri: Uri | ||
| ): Promise<QuartoCellSymbols[]> { | ||
| try { | ||
| const cells = await commands.executeCommand<QuartoCellSymbols[] | undefined>( | ||
| "positron.executeQuartoCellSymbolProvider", | ||
| uri | ||
| ); | ||
| return cells ?? []; | ||
| } catch (error) { | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Nests each cell's symbols under the chunk symbol it came from. | ||
| * | ||
| * Chunks are matched to cells by range containment: a chunk symbol's range | ||
| * covers its fences, so the cell's code span sits inside it. Chunks are the | ||
| * `SymbolKind.Function` symbols the Quarto language server's `toc.ts` emits, | ||
| * which is the same marker the virtual document path uses. | ||
| * | ||
| * Symbols the language server already nested under a chunk are kept, and a | ||
| * chunk with no matching cell is left as it is. | ||
| */ | ||
| export function nestCellSymbols( | ||
| symbols: DocumentSymbol[], | ||
| cells: readonly QuartoCellSymbols[] | ||
| ): DocumentSymbol[] { | ||
| for (const symbol of symbols) { | ||
| if (symbol.kind === SymbolKind.Function) { | ||
| const cell = cells.find((candidate) => | ||
| symbol.range.contains(candidate.range) | ||
| ); | ||
| if (cell) { | ||
| symbol.children = [...symbol.children, ...cell.symbols]; | ||
| } | ||
| } else { | ||
| symbol.children = nestCellSymbols(symbol.children, cells); | ||
| } | ||
| } | ||
|
|
||
| return symbols; | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -68,6 +68,8 @@ import { LspInitializationOptions, QuartoContext } from "quarto-core"; | |||||
| import { lspClientTransport } from "core-node"; | ||||||
| import { JsonRpcRequestTransport } from "core"; | ||||||
| import { extensionHost } from "../host"; | ||||||
| import { kNativeFeaturesSetting, useNativeEmbeddedFeatures } from "../host/native-features"; | ||||||
| import { nestCellSymbols, quartoCellSymbols } from "./cell-symbols"; | ||||||
| import semver from "semver"; | ||||||
| import { EmbeddedLanguage } from "../vdoc/languages"; | ||||||
| import { SymbolInformation } from "vscode"; | ||||||
|
|
@@ -157,8 +159,36 @@ export function activateLsp( | |||||
| if (config.get("cells.signatureHelp.enabled", true)) { | ||||||
| middleware.provideSignatureHelp = embeddedSignatureHelpProvider(engine); | ||||||
| } | ||||||
| extensionHost().registerStatementRangeProvider(engine); | ||||||
| extensionHost().registerHelpTopicProvider(engine); | ||||||
| // Statement range and help topic are single-answer features: whichever | ||||||
| // provider registered last owns Cmd+Enter and F1. When the host serves cells | ||||||
| // natively we must not compete with it, so these registrations follow the | ||||||
| // setting live rather than being made once. Disposing on enable hands the | ||||||
| // features to the host; re-registering on disable wins the race because this | ||||||
| // registration is then the most recent. | ||||||
| let hostProviders: Disposable[] = []; | ||||||
| const registerHostProviders = () => { | ||||||
| hostProviders = [ | ||||||
| extensionHost().registerStatementRangeProvider(engine), | ||||||
| extensionHost().registerHelpTopicProvider(engine), | ||||||
| ]; | ||||||
| }; | ||||||
| if (!useNativeEmbeddedFeatures()) { | ||||||
| registerHostProviders(); | ||||||
| } | ||||||
| context.subscriptions.push( | ||||||
| new Disposable(() => hostProviders.forEach((d) => d.dispose())), | ||||||
| workspace.onDidChangeConfiguration((e) => { | ||||||
| if (!e.affectsConfiguration(kNativeFeaturesSetting)) { | ||||||
| return; | ||||||
| } | ||||||
| if (useNativeEmbeddedFeatures()) { | ||||||
| hostProviders.forEach((d) => d.dispose()); | ||||||
| hostProviders = []; | ||||||
| } else if (hostProviders.length === 0) { | ||||||
| registerHostProviders(); | ||||||
| } | ||||||
| }) | ||||||
| ); | ||||||
|
|
||||||
| // create client options | ||||||
| const initializationOptions: LspInitializationOptions = { | ||||||
|
|
@@ -328,6 +358,11 @@ function embeddedCodeCompletionProvider(engine: MarkdownEngine) { | |||||
| const vdoc = await virtualDoc(document, position, engine); | ||||||
|
|
||||||
| if (vdoc && !isWithinYamlComment(document, position)) { | ||||||
| // stand down when the host serves this language's cells itself | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if (useNativeEmbeddedFeatures(vdoc.language)) { | ||||||
| return undefined; | ||||||
| } | ||||||
|
|
||||||
| // if there is a trigger character make sure the language supports it | ||||||
| const language = vdoc.language; | ||||||
| if (context.triggerCharacter) { | ||||||
|
|
@@ -372,6 +407,10 @@ function embeddedHoverProvider(engine: MarkdownEngine) { | |||||
|
|
||||||
| const vdoc = await virtualDoc(document, position, engine); | ||||||
| if (vdoc) { | ||||||
| if (useNativeEmbeddedFeatures(vdoc.language)) { | ||||||
| return undefined; | ||||||
| } | ||||||
|
|
||||||
| return await withVirtualDocUri(vdoc, document.uri, "hover", async (uri: Uri) => { | ||||||
| try { | ||||||
| return await getHover(uri, vdoc.language, position); | ||||||
|
|
@@ -396,6 +435,10 @@ function embeddedSignatureHelpProvider(engine: MarkdownEngine) { | |||||
| ) => { | ||||||
| const vdoc = await virtualDoc(document, position, engine); | ||||||
| if (vdoc) { | ||||||
| if (useNativeEmbeddedFeatures(vdoc.language)) { | ||||||
| return undefined; | ||||||
| } | ||||||
|
|
||||||
| return await withVirtualDocUri(vdoc, document.uri, "signature", async (uri: Uri) => { | ||||||
| try { | ||||||
| return await getSignatureHelpHover(uri, vdoc.language, position, context.triggerCharacter); | ||||||
|
|
@@ -418,6 +461,10 @@ function embeddedGoToDefinitionProvider(engine: MarkdownEngine) { | |||||
| ): Promise<Definition | LocationLink[] | null | undefined> => { | ||||||
| const vdoc = await virtualDoc(document, position, engine); | ||||||
| if (vdoc) { | ||||||
| if (useNativeEmbeddedFeatures(vdoc.language)) { | ||||||
| return undefined; | ||||||
| } | ||||||
|
|
||||||
| return await withVirtualDocUri(vdoc, document.uri, "definition", async (uri: Uri) => { | ||||||
| try { | ||||||
| const definitions = await commands.executeCommand< | ||||||
|
|
@@ -508,6 +555,15 @@ function embeddedDocumentSymbolProvider(engine: MarkdownEngine) { | |||||
| // I don't think we actually ever get SymbolInformation[] here, but I'm not certain | ||||||
| // so this is defensively coded. | ||||||
| if (baseSymbols.length > 0 && isDocumentSymbol(baseSymbols[0])) { | ||||||
| // When the host serves the cells, one command answers for the whole | ||||||
| // document, so it is fetched once per request and the chunks are matched | ||||||
| // to it by range. | ||||||
| if (useNativeEmbeddedFeatures()) { | ||||||
| const cells = await quartoCellSymbols(document.uri); | ||||||
| if (token.isCancellationRequested) return baseSymbols; | ||||||
| return nestCellSymbols(baseSymbols as DocumentSymbol[], cells); | ||||||
| } | ||||||
|
|
||||||
| const enhanced = await enhanceSymbolsWithCodeCellContent( | ||||||
| document, | ||||||
| baseSymbols as DocumentSymbol[], | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer a more directly descriptive term than "native-features", like "positron-supported-features" or "positron-provided-features".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This goes for everywhere where the term "native" is used.