-
Notifications
You must be signed in to change notification settings - Fork 45
Remove virtualDocUri() as a footgun
#713
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
Changes from all commits
016af20
0da0c68
1742215
8ce0179
bc936d9
9c3050b
14ae96f
a3236ff
3aad1bb
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 |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import { | |
| LocationLink, | ||
| Definition, | ||
| LogOutputChannel, | ||
| Uri | ||
| } from "vscode"; | ||
| import { | ||
| LanguageClient, | ||
|
|
@@ -51,7 +52,7 @@ import { | |
| adjustedPosition, | ||
| unadjustedRange, | ||
| virtualDoc, | ||
| virtualDocUri, | ||
| withVirtualDocUri, | ||
| } from "../vdoc/vdoc"; | ||
| import { activateVirtualDocEmbeddedContent } from "../vdoc/vdoc-content"; | ||
| import { vdocCompletions } from "../vdoc/vdoc-completion"; | ||
|
|
@@ -225,19 +226,13 @@ function embeddedHoverProvider(engine: MarkdownEngine) { | |
|
|
||
| const vdoc = await virtualDoc(document, position, engine); | ||
| if (vdoc) { | ||
| // get uri for hover | ||
| const vdocUri = await virtualDocUri(vdoc, document.uri, "hover"); | ||
|
|
||
| // execute hover | ||
| try { | ||
| return getHover(vdocUri.uri, vdoc.language, position); | ||
| } catch (error) { | ||
| console.log(error); | ||
| } finally { | ||
| if (vdocUri.cleanup) { | ||
| await vdocUri.cleanup(); | ||
| return await withVirtualDocUri(vdoc, document.uri, "hover", async (uri: Uri) => { | ||
| try { | ||
| return await getHover(uri, vdoc.language, position); | ||
| } catch (error) { | ||
| console.log(error); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // default to server delegation | ||
|
|
@@ -255,16 +250,13 @@ function embeddedSignatureHelpProvider(engine: MarkdownEngine) { | |
| ) => { | ||
| const vdoc = await virtualDoc(document, position, engine); | ||
| if (vdoc) { | ||
| const vdocUri = await virtualDocUri(vdoc, document.uri, "signature"); | ||
| try { | ||
| return getSignatureHelpHover(vdocUri.uri, vdoc.language, position, context.triggerCharacter); | ||
| } catch (error) { | ||
| return undefined; | ||
| } finally { | ||
| if (vdocUri.cleanup) { | ||
| await vdocUri.cleanup(); | ||
| return await withVirtualDocUri(vdoc, document.uri, "signature", async (uri: Uri) => { | ||
| try { | ||
| return await getSignatureHelpHover(uri, vdoc.language, position, context.triggerCharacter); | ||
|
Collaborator
Author
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. We did not |
||
| } catch (error) { | ||
| return undefined; | ||
| } | ||
| } | ||
| }); | ||
| } else { | ||
| return await next(document, position, context, token); | ||
| } | ||
|
|
@@ -280,64 +272,61 @@ function embeddedGoToDefinitionProvider(engine: MarkdownEngine) { | |
| ): Promise<Definition | LocationLink[] | null | undefined> => { | ||
| const vdoc = await virtualDoc(document, position, engine); | ||
| if (vdoc) { | ||
| const vdocUri = await virtualDocUri(vdoc, document.uri, "definition"); | ||
|
Collaborator
Author
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. This diff looks ugly but I didn't change anything in the |
||
| try { | ||
| const definitions = await commands.executeCommand< | ||
| ProviderResult<Definition | LocationLink[]> | ||
| >( | ||
| "vscode.executeDefinitionProvider", | ||
| vdocUri.uri, | ||
| adjustedPosition(vdoc.language, position) | ||
| ); | ||
| const resolveLocation = (location: Location) => { | ||
| if (location.uri.toString() === vdocUri.uri.toString()) { | ||
| return new Location( | ||
| document.uri, | ||
| unadjustedRange(vdoc.language, location.range) | ||
| ); | ||
| } else { | ||
| return location; | ||
| } | ||
| }; | ||
| const resolveLocationLink = (location: LocationLink) => { | ||
| if (location.targetUri.toString() === vdocUri.uri.toString()) { | ||
| const locationLink: LocationLink = { | ||
| targetRange: unadjustedRange(vdoc.language, location.targetRange), | ||
| originSelectionRange: location.originSelectionRange | ||
| ? unadjustedRange(vdoc.language, location.originSelectionRange) | ||
| : undefined, | ||
| targetSelectionRange: location.targetSelectionRange | ||
| ? unadjustedRange(vdoc.language, location.targetSelectionRange) | ||
| : undefined, | ||
| targetUri: document.uri, | ||
| }; | ||
| return locationLink; | ||
| } else { | ||
| return location; | ||
| } | ||
| }; | ||
| if (definitions instanceof Location) { | ||
| return resolveLocation(definitions); | ||
| } else if (Array.isArray(definitions) && definitions.length > 0) { | ||
| if (definitions[0] instanceof Location) { | ||
| return definitions.map((definition) => | ||
| resolveLocation(definition as Location) | ||
| ); | ||
| return await withVirtualDocUri(vdoc, document.uri, "definition", async (uri: Uri) => { | ||
| try { | ||
| const definitions = await commands.executeCommand< | ||
| ProviderResult<Definition | LocationLink[]> | ||
| >( | ||
| "vscode.executeDefinitionProvider", | ||
| uri, | ||
| adjustedPosition(vdoc.language, position) | ||
| ); | ||
| const resolveLocation = (location: Location) => { | ||
| if (location.uri.toString() === uri.toString()) { | ||
| return new Location( | ||
| document.uri, | ||
| unadjustedRange(vdoc.language, location.range) | ||
| ); | ||
| } else { | ||
| return location; | ||
| } | ||
| }; | ||
| const resolveLocationLink = (location: LocationLink) => { | ||
| if (location.targetUri.toString() === uri.toString()) { | ||
| const locationLink: LocationLink = { | ||
| targetRange: unadjustedRange(vdoc.language, location.targetRange), | ||
| originSelectionRange: location.originSelectionRange | ||
| ? unadjustedRange(vdoc.language, location.originSelectionRange) | ||
| : undefined, | ||
| targetSelectionRange: location.targetSelectionRange | ||
| ? unadjustedRange(vdoc.language, location.targetSelectionRange) | ||
| : undefined, | ||
| targetUri: document.uri, | ||
| }; | ||
| return locationLink; | ||
| } else { | ||
| return location; | ||
| } | ||
| }; | ||
| if (definitions instanceof Location) { | ||
| return resolveLocation(definitions); | ||
| } else if (Array.isArray(definitions) && definitions.length > 0) { | ||
| if (definitions[0] instanceof Location) { | ||
| return definitions.map((definition) => | ||
| resolveLocation(definition as Location) | ||
| ); | ||
| } else { | ||
| return definitions.map((definition) => | ||
| resolveLocationLink(definition as LocationLink) | ||
| ); | ||
| } | ||
| } else { | ||
| return definitions.map((definition) => | ||
| resolveLocationLink(definition as LocationLink) | ||
| ); | ||
| return definitions; | ||
| } | ||
| } else { | ||
| return definitions; | ||
| } | ||
| } catch (error) { | ||
| return undefined; | ||
| } finally { | ||
| if (vdocUri.cleanup) { | ||
| await vdocUri.cleanup(); | ||
| } catch (error) { | ||
| return undefined; | ||
| } | ||
| } | ||
| }); | ||
| } else { | ||
| return await next(document, position, token); | ||
| } | ||
|
|
||
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.
We did not
awaitbefore but I'm guessing it's probably better to?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 think you are correct on this; I occasionally would observe slight glitchy-ness with the embedded help in Quarto files and I can't get it to happen now with this PR.