Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Try using a unique temp file for vdocs #585

Merged
merged 7 commits into from
Oct 29, 2024
Merged

Try using a unique temp file for vdocs #585

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
26907dc
Use a unique temp file for vdocs
juliasilge Oct 24, 2024
784f147
Try to fix `paths` filter for CI
juliasilge Oct 24, 2024
b5df580
Merge branch 'main' into try-unique-temp-file-for-vdoc
juliasilge Oct 25, 2024
d7965c2
Whitespaaaaaaace
juliasilge Oct 25, 2024
7abf8f7
Consistently clean up vdocs
lionel- Oct 25, 2024
74a30bc
Update changelog
juliasilge Oct 29, 2024
e0c9a64
Merge branch 'main' into try-unique-temp-file-for-vdoc
juliasilge Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 2 additions & 9 deletions apps/vscode/src/lsp/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import {
virtualDocUri,
} from "../vdoc/vdoc";
import { activateVirtualDocEmbeddedContent } from "../vdoc/vdoc-content";
import { deactivateVirtualDocTempFiles, isLanguageVirtualDoc } from "../vdoc/vdoc-tempfile";
import { vdocCompletions } from "../vdoc/vdoc-completion";

import {
Expand Down Expand Up @@ -160,8 +159,6 @@ export async function activateLsp(
}

export function deactivate(): Thenable<void> | undefined {
deactivateVirtualDocTempFiles();

if (!client) {
return undefined;
}
Expand Down Expand Up @@ -289,8 +286,7 @@ function embeddedGoToDefinitionProvider(engine: MarkdownEngine) {
adjustedPosition(vdoc.language, position)
);
const resolveLocation = (location: Location) => {
if (isLanguageVirtualDoc(vdoc.language, location.uri) ||
location.uri.toString() === vdocUri.uri.toString()) {
if (location.uri.toString() === vdocUri.uri.toString()) {
return new Location(
document.uri,
unadjustedRange(vdoc.language, location.range)
Expand All @@ -300,8 +296,7 @@ function embeddedGoToDefinitionProvider(engine: MarkdownEngine) {
}
};
const resolveLocationLink = (location: LocationLink) => {
if (isLanguageVirtualDoc(vdoc.language, location.targetUri) ||
location.targetUri.toString() === vdocUri.uri.toString()) {
if (location.targetUri.toString() === vdocUri.uri.toString()) {
const locationLink: LocationLink = {
targetRange: unadjustedRange(vdoc.language, location.targetRange),
originSelectionRange: location.originSelectionRange
Expand Down Expand Up @@ -349,5 +344,3 @@ function isWithinYamlComment(doc: TextDocument, pos: Position) {
const line = doc.lineAt(pos.line).text;
return !!line.match(/^\s*#\s*\| /);
}


74 changes: 23 additions & 51 deletions apps/vscode/src/vdoc/vdoc-tempfile.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
* vdoc-tempfile.ts
*
* Copyright (C) 2022 by Posit Software, PBC
* Copyright (C) 2022-2024 by Posit Software, PBC
* Copyright (c) 2019 Takashi Tamura
*
* Unless you have received this program directly from Posit Software pursuant
Expand All @@ -17,6 +17,7 @@
import * as fs from "fs";
import * as path from "path";
import * as tmp from "tmp";
import * as uuid from "uuid";
import {
commands,
Hover,
Expand All @@ -27,42 +28,27 @@ import {
WorkspaceEdit,
} from "vscode";
import { VirtualDoc, VirtualDocUri } from "./vdoc";
import { EmbeddedLanguage } from "./languages";

// one virtual doc per language file extension
const languageVirtualDocs = new Map<String, TextDocument>();

export async function virtualDocUriFromTempFile(
virtualDoc: VirtualDoc,
docPath: string,
virtualDoc: VirtualDoc,
docPath: string,
local: boolean
) : Promise<VirtualDocUri> {
): Promise<VirtualDocUri> {
const newVirtualDocUri = (doc: TextDocument) =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New constructor for a vdoc-uri with a cleanup function.

<VirtualDocUri>{
uri: doc.uri,
cleanup: async () => await deleteDocument(doc),
};

// if this is local then create it alongside the docPath and return a cleanup
// function to remove it when the action is completed.
// if this is local then create it alongside the docPath and return a cleanup
// function to remove it when the action is completed.
if (local || virtualDoc.language.localTempFile) {
const ext = virtualDoc.language.extension;
const vdocPath = path.join(path.dirname(docPath), `.vdoc.${ext}`);
fs.writeFileSync(vdocPath, virtualDoc.content);
const vdocUri = Uri.file(vdocPath);
const doc = await workspace.openTextDocument(vdocUri);
return {
uri: doc.uri,
cleanup: async () => await deleteDocument(doc)
};
}

// do we have an existing document?
const langVdoc = languageVirtualDocs.get(virtualDoc.language.extension);
if (langVdoc && !langVdoc.isClosed) {
if (langVdoc.getText() === virtualDoc.content) {
// if its content is identical to what's passed in then just return it
return { uri: langVdoc.uri };
Comment on lines -58 to -60
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an optimisation for when multiple LSP requests are performed in a row but the underlying file hasn't changed. We removed it to simplify the lifetime management of the tempfiles. We now always create a new tempfile for each request and each caller is in charge of cleaning up the file (we double-checked that all callers currently do clean them up).

} else {
// otherwise remove it (it will get recreated below)
await deleteDocument(langVdoc);
languageVirtualDocs.delete(virtualDoc.language.extension);
}
return newVirtualDocUri(doc);
}

// write the virtual doc as a temp file
Expand All @@ -71,31 +57,17 @@ export async function virtualDocUriFromTempFile(
// open the document and save a reference to it
const vdocUri = Uri.file(vdocTempFile);
const doc = await workspace.openTextDocument(vdocUri);
languageVirtualDocs.set(virtualDoc.language.extension, doc);

// if this is the first time getting a virtual doc for this
// language then execute a dummy request to cause it to load
if (!langVdoc) {
await commands.executeCommand<Hover[]>(
"vscode.executeHoverProvider",
vdocUri,
new Position(0, 0)
);
}

// return the uri
return { uri: doc.uri };
}

// delete any vdocs left open
export async function deactivateVirtualDocTempFiles() {
languageVirtualDocs.forEach(async (doc) => {
await deleteDocument(doc);
});
}
// TODO: Reevaluate whether this is necessary. Old comment:
// > if this is the first time getting a virtual doc for this
// > language then execute a dummy request to cause it to load
await commands.executeCommand<Hover[]>(
"vscode.executeHoverProvider",
vdocUri,
new Position(0, 0)
);

export function isLanguageVirtualDoc(langauge: EmbeddedLanguage, uri: Uri) {
return languageVirtualDocs.get(langauge.extension)?.uri.toString() === uri.toString();
return newVirtualDocUri(doc);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now cleaning up the tempfiles in this code path too.

Previously it would only be deleted manually when a change was detected in the contents.

}

// delete a document
Expand All @@ -121,7 +93,7 @@ function createVirtualDocTempFile(virtualDoc: VirtualDoc) {
if (!fs.existsSync(dir)) {
fs.mkdirSync(dir);
}
const tmpPath = path.join(vdocTempDir, ext, ".intellisense." + ext);
const tmpPath = path.join(vdocTempDir, ext, ".intellisense." + uuid.v4() + "." + ext);
fs.writeFileSync(tmpPath, virtualDoc.content);

return tmpPath;
Expand Down