From 2f4b51cb85b00cec50393d6f36abefc5243956ee Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Mon, 25 Nov 2024 17:48:56 -0800 Subject: [PATCH 1/2] Update dynamic documents to store edits for closed documents --- src/lsptoolshost/roslynLanguageServer.ts | 6 ++ .../src/csharp/csharpProjectedDocument.ts | 46 ++++++++++++--- src/razor/src/document/IRazorDocument.ts | 1 + src/razor/src/document/razorDocument.ts | 32 +++++++++++ .../src/document/razorDocumentFactory.ts | 12 +--- .../src/document/razorDocumentManager.ts | 57 ++++++------------- .../src/dynamicFile/dynamicFileInfoHandler.ts | 54 ++++++++++++------ .../dynamicFile/dynamicFileUpdatedParams.ts | 10 ++++ .../dynamicFile/provideDynamicFileResponse.ts | 6 +- 9 files changed, 150 insertions(+), 74 deletions(-) create mode 100644 src/razor/src/document/razorDocument.ts create mode 100644 src/razor/src/dynamicFile/dynamicFileUpdatedParams.ts diff --git a/src/lsptoolshost/roslynLanguageServer.ts b/src/lsptoolshost/roslynLanguageServer.ts index 3b43487dc..e391bdf2d 100644 --- a/src/lsptoolshost/roslynLanguageServer.ts +++ b/src/lsptoolshost/roslynLanguageServer.ts @@ -78,6 +78,7 @@ import { import { registerSourceGeneratedFilesContentProvider } from './sourceGeneratedFilesContentProvider'; import { registerMiscellaneousFileNotifier } from './miscellaneousFileNotifier'; import { TelemetryEventNames } from '../shared/telemetryEventNames'; +import { RazorDynamicFileChangedParams } from '../razor/src/dynamicFile/dynamicFileUpdatedParams'; let _channel: vscode.LogOutputChannel; let _traceChannel: vscode.OutputChannel; @@ -789,6 +790,11 @@ export class RoslynLanguageServer { async (notification) => vscode.commands.executeCommand(DynamicFileInfoHandler.removeDynamicFileInfoCommand, notification) ); + vscode.commands.registerCommand( + DynamicFileInfoHandler.dynamicFileUpdatedCommand, + async (notification: RazorDynamicFileChangedParams) => + this.sendNotification('razor/dynamicFileInfoChanged', notification) + ); } // eslint-disable-next-line @typescript-eslint/promise-function-async diff --git a/src/razor/src/csharp/csharpProjectedDocument.ts b/src/razor/src/csharp/csharpProjectedDocument.ts index d74c0bb25..6e90a1de7 100644 --- a/src/razor/src/csharp/csharpProjectedDocument.ts +++ b/src/razor/src/csharp/csharpProjectedDocument.ts @@ -19,6 +19,7 @@ export class CSharpProjectedDocument implements IProjectedDocument { private resolveProvisionalEditAt: number | undefined; private ProvisionalDotPosition: Position | undefined; private hostDocumentVersion: number | null = null; + private edits: ServerTextChange[] | null = null; public constructor(public readonly uri: vscode.Uri) { this.path = getUriPath(uri); @@ -37,21 +38,38 @@ export class CSharpProjectedDocument implements IProjectedDocument { } public update(edits: ServerTextChange[], hostDocumentVersion: number) { + // Apply any stored edits if needed + if (this.edits) { + edits = this.edits.concat(edits); + this.edits = null; + } + this.removeProvisionalDot(); this.hostDocumentVersion = hostDocumentVersion; - if (edits.length === 0) { - return; + this.updateContent(edits); + } + + public storeEdits(edits: ServerTextChange[], hostDocumentVersion: number) { + this.hostDocumentVersion = hostDocumentVersion; + if (this.edits) { + this.edits = this.edits.concat(edits); + } else { + this.edits = edits; } + } - let content = this.content; - for (const edit of edits.reverse()) { - // TODO: Use a better data structure to represent the content, string concatenation is slow. - content = this.getEditedContent(edit.newText, edit.span.start, edit.span.start + edit.span.length, content); + public getAndApplyEdits() { + const edits = this.edits; + + // Make sure the internal representation of the content is updated + if (edits) { + this.updateContent(edits); } - this.setContent(content); + this.edits = null; + return edits; } public getContent() { @@ -150,4 +168,18 @@ export class CSharpProjectedDocument implements IProjectedDocument { private setContent(content: string) { this.content = content; } + + private updateContent(edits: ServerTextChange[]) { + if (edits.length === 0) { + return; + } + + let content = this.content; + for (const edit of edits.reverse()) { + // TODO: Use a better data structure to represent the content, string concatenation is slow. + content = this.getEditedContent(edit.newText, edit.span.start, edit.span.start + edit.span.length, content); + } + + this.setContent(content); + } } diff --git a/src/razor/src/document/IRazorDocument.ts b/src/razor/src/document/IRazorDocument.ts index 95557df8f..6fcd07a26 100644 --- a/src/razor/src/document/IRazorDocument.ts +++ b/src/razor/src/document/IRazorDocument.ts @@ -11,4 +11,5 @@ export interface IRazorDocument { readonly uri: vscode.Uri; readonly csharpDocument: IProjectedDocument; readonly htmlDocument: IProjectedDocument; + readonly isOpen: boolean; } diff --git a/src/razor/src/document/razorDocument.ts b/src/razor/src/document/razorDocument.ts new file mode 100644 index 000000000..1c42dce6a --- /dev/null +++ b/src/razor/src/document/razorDocument.ts @@ -0,0 +1,32 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as vscode from 'vscode'; +import { CSharpProjectedDocument } from '../csharp/csharpProjectedDocument'; +import { HtmlProjectedDocument } from '../html/htmlProjectedDocument'; +import { getUriPath } from '../uriPaths'; +import { IRazorDocument } from './IRazorDocument'; + +export class RazorDocument implements IRazorDocument { + public readonly path: string; + + constructor( + readonly uri: vscode.Uri, + readonly csharpDocument: CSharpProjectedDocument, + readonly htmlDocument: HtmlProjectedDocument + ) { + this.path = getUriPath(uri); + } + + public get isOpen(): boolean { + for (const textDocument of vscode.workspace.textDocuments) { + if (textDocument.uri.fsPath == this.uri.fsPath) { + return true; + } + } + + return false; + } +} diff --git a/src/razor/src/document/razorDocumentFactory.ts b/src/razor/src/document/razorDocumentFactory.ts index 18b8b919f..1b3f2c5db 100644 --- a/src/razor/src/document/razorDocumentFactory.ts +++ b/src/razor/src/document/razorDocumentFactory.ts @@ -11,18 +11,12 @@ import { HtmlProjectedDocumentContentProvider } from '../html/htmlProjectedDocum import { virtualCSharpSuffix, virtualHtmlSuffix } from '../razorConventions'; import { getUriPath } from '../uriPaths'; import { IRazorDocument } from './IRazorDocument'; +import { RazorDocument } from './razorDocument'; -export function createDocument(uri: vscode.Uri) { +export function createDocument(uri: vscode.Uri): IRazorDocument { const csharpDocument = createProjectedCSharpDocument(uri); const htmlDocument = createProjectedHtmlDocument(uri); - const path = getUriPath(uri); - - const document: IRazorDocument = { - uri, - path, - csharpDocument, - htmlDocument, - }; + const document = new RazorDocument(uri, csharpDocument, htmlDocument); return document; } diff --git a/src/razor/src/document/razorDocumentManager.ts b/src/razor/src/document/razorDocumentManager.ts index 066073f40..50ee8dd71 100644 --- a/src/razor/src/document/razorDocumentManager.ts +++ b/src/razor/src/document/razorDocumentManager.ts @@ -50,21 +50,12 @@ export class RazorDocumentManager implements IRazorDocumentManager { return Object.values(this.razorDocuments); } - public async getDocument(uri: vscode.Uri) { + public async getDocument(uri: vscode.Uri): Promise { const document = this._getDocument(uri); - - // VS Code closes virtual documents after some timeout if they are not open in the IDE. Since our generated C# and Html - // documents are never open in the IDE, we need to ensure that VS Code considers them open so that requests against them - // succeed. Without this, even a simple diagnostics request will fail in Roslyn if the user just opens a .razor document - // and leaves it open past the timeout. - if (this.razorDocumentGenerationInitialized) { - await this.ensureDocumentAndProjectedDocumentsOpen(document); - } - return document; } - public async getActiveDocument() { + public async getActiveDocument(): Promise { if (!vscode.window.activeTextEditor) { return null; } @@ -147,7 +138,7 @@ export class RazorDocumentManager implements IRazorDocumentManager { return vscode.Disposable.from(watcher, didCreateRegistration, didOpenRegistration, didCloseRegistration); } - private _getDocument(uri: vscode.Uri) { + private _getDocument(uri: vscode.Uri): IRazorDocument { const path = getUriPath(uri); let document = this.findDocument(path); @@ -159,7 +150,7 @@ export class RazorDocumentManager implements IRazorDocumentManager { document = this.addDocument(uri); } - return document; + return document!; } private async openDocument(uri: vscode.Uri) { @@ -182,10 +173,6 @@ export class RazorDocumentManager implements IRazorDocumentManager { await vscode.commands.executeCommand(razorInitializeCommand, pipeName); await this.serverClient.connectNamedPipe(pipeName); - for (const document of this.documents) { - await this.ensureDocumentAndProjectedDocumentsOpen(document); - } - this.onRazorInitializedEmitter.fire(); } } @@ -205,7 +192,7 @@ export class RazorDocumentManager implements IRazorDocumentManager { this.notifyDocumentChange(document, RazorDocumentChangeKind.closed); } - private addDocument(uri: vscode.Uri) { + private addDocument(uri: vscode.Uri): IRazorDocument { const path = getUriPath(uri); let document = this.findDocument(path); if (document) { @@ -261,10 +248,6 @@ export class RazorDocumentManager implements IRazorDocumentManager { ) { // We allow re-setting of the updated content from the same doc sync version in the case // of project or file import changes. - - // Make sure the document is open, because updating will cause a didChange event to fire. - await vscode.workspace.openTextDocument(document.csharpDocument.uri); - const csharpProjectedDocument = projectedDocument as CSharpProjectedDocument; // If the language server is telling us that the previous document was empty, then we should clear @@ -275,7 +258,17 @@ export class RazorDocumentManager implements IRazorDocumentManager { csharpProjectedDocument.clear(); } - csharpProjectedDocument.update(updateBufferRequest.changes, updateBufferRequest.hostDocumentVersion); + if (document.isOpen) { + // Make sure the document is open, because updating will cause a didChange event to fire. + await vscode.workspace.openTextDocument(document.csharpDocument.uri); + + csharpProjectedDocument.update(updateBufferRequest.changes, updateBufferRequest.hostDocumentVersion); + } else { + csharpProjectedDocument.storeEdits( + updateBufferRequest.changes, + updateBufferRequest.hostDocumentVersion + ); + } this.notifyDocumentChange(document, RazorDocumentChangeKind.csharpChanged); } else { @@ -342,22 +335,4 @@ export class RazorDocumentManager implements IRazorDocumentManager { this.onChangeEmitter.fire(args); } - - private async ensureDocumentAndProjectedDocumentsOpen(document: IRazorDocument) { - // vscode.workspace.openTextDocument may send a textDocument/didOpen - // request to the C# language server. We need to keep track of - // this to make sure we don't send a duplicate request later on. - const razorUri = vscode.Uri.file(document.path); - if (!this.isRazorDocumentOpenInCSharpWorkspace(razorUri)) { - this.didOpenRazorCSharpDocument(razorUri); - - // Need to tell the Razor server that the document is open, or it won't generate C# code - // for it, and our projected document will always be empty, until the user manually - // opens the razor file. - await vscode.workspace.openTextDocument(razorUri); - } - - await vscode.workspace.openTextDocument(document.csharpDocument.uri); - await vscode.workspace.openTextDocument(document.htmlDocument.uri); - } } diff --git a/src/razor/src/dynamicFile/dynamicFileInfoHandler.ts b/src/razor/src/dynamicFile/dynamicFileInfoHandler.ts index 9a91d1375..2d86a926c 100644 --- a/src/razor/src/dynamicFile/dynamicFileInfoHandler.ts +++ b/src/razor/src/dynamicFile/dynamicFileInfoHandler.ts @@ -4,19 +4,23 @@ *--------------------------------------------------------------------------------------------*/ import * as vscode from 'vscode'; -import { DocumentUri } from 'vscode-languageclient/node'; import { UriConverter } from '../../../lsptoolshost/uriConverter'; import { RazorDocumentManager } from '../document/razorDocumentManager'; import { RazorLogger } from '../razorLogger'; import { ProvideDynamicFileParams } from './provideDynamicFileParams'; import { ProvideDynamicFileResponse } from './provideDynamicFileResponse'; import { RemoveDynamicFileParams } from './removeDynamicFileParams'; +import { CSharpProjectedDocument } from '../csharp/csharpProjectedDocument'; +import { RazorDocumentChangeKind } from '../document/razorDocumentChangeKind'; +import { RazorDynamicFileChangedParams } from './dynamicFileUpdatedParams'; +import { TextDocumentIdentifier } from 'vscode-languageserver-protocol'; // Handles Razor generated doc communication between the Roslyn workspace and Razor. // didChange behavior for Razor generated docs is handled in the RazorDocumentManager. export class DynamicFileInfoHandler { public static readonly provideDynamicFileInfoCommand = 'razor.provideDynamicFileInfo'; public static readonly removeDynamicFileInfoCommand = 'razor.removeDynamicFileInfo'; + public static readonly dynamicFileUpdatedCommand = 'razor.dynamicFileUpdated'; constructor(private readonly documentManager: RazorDocumentManager, private readonly logger: RazorLogger) {} @@ -33,39 +37,57 @@ export class DynamicFileInfoHandler { await this.removeDynamicFileInfo(request); } ); + this.documentManager.onChange(async (e) => { + if (e.kind == RazorDocumentChangeKind.csharpChanged && !e.document.isOpen) { + const uriString = UriConverter.serialize(e.document.uri); + const identifier = TextDocumentIdentifier.create(uriString); + await vscode.commands.executeCommand( + DynamicFileInfoHandler.dynamicFileUpdatedCommand, + new RazorDynamicFileChangedParams(identifier) + ); + } + }); } // Given Razor document URIs, returns associated generated doc URIs private async provideDynamicFileInfo( request: ProvideDynamicFileParams ): Promise { - let virtualUri: DocumentUri | null = null; + this.documentManager.roslynActivated = true; + const vscodeUri = vscode.Uri.parse(request.razorDocument.uri, true); + + // Normally we start receiving dynamic info after Razor is initialized, but if the user had a .razor file open + // when they started VS Code, the order is the other way around. This no-ops if Razor is already initialized. + await this.documentManager.ensureRazorInitialized(); + + const razorDocument = await this.documentManager.getDocument(vscodeUri); try { - const vscodeUri = vscode.Uri.parse(request.razorDocument.uri, true); - const razorDocument = await this.documentManager.getDocument(vscodeUri); if (razorDocument === undefined) { this.logger.logWarning( `Could not find Razor document ${vscodeUri.fsPath}; adding null as a placeholder in URI array.` ); - } else { - // Retrieve generated doc URIs for each Razor URI we are given - const virtualCsharpUri = UriConverter.serialize(razorDocument.csharpDocument.uri); - virtualUri = virtualCsharpUri; + + return null; } - this.documentManager.roslynActivated = true; + const virtualCsharpUri = UriConverter.serialize(razorDocument.csharpDocument.uri); - // Normally we start receiving dynamic info after Razor is initialized, but if the user had a .razor file open - // when they started VS Code, the order is the other way around. This no-ops if Razor is already initialized. - await this.documentManager.ensureRazorInitialized(); + if (this.documentManager.isRazorDocumentOpenInCSharpWorkspace(vscodeUri)) { + // Open documents have didOpen/didChange to update the csharp buffer. Razor + // does not send edits and instead lets vscode handle them. + return new ProvideDynamicFileResponse({ uri: virtualCsharpUri }, null); + } else { + // Closed documents provide edits since the last time they were requested since + // there is no open buffer in vscode corresponding to the csharp content. + const csharpDocument = razorDocument.csharpDocument as CSharpProjectedDocument; + const edits = csharpDocument.getAndApplyEdits(); + + return new ProvideDynamicFileResponse({ uri: virtualCsharpUri }, edits ?? null); + } } catch (error) { this.logger.logWarning(`${DynamicFileInfoHandler.provideDynamicFileInfoCommand} failed with ${error}`); } - if (virtualUri) { - return new ProvideDynamicFileResponse({ uri: virtualUri }); - } - return null; } diff --git a/src/razor/src/dynamicFile/dynamicFileUpdatedParams.ts b/src/razor/src/dynamicFile/dynamicFileUpdatedParams.ts new file mode 100644 index 000000000..3a5f4c397 --- /dev/null +++ b/src/razor/src/dynamicFile/dynamicFileUpdatedParams.ts @@ -0,0 +1,10 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { TextDocumentIdentifier } from 'vscode-languageserver-protocol'; + +export class RazorDynamicFileChangedParams { + constructor(public readonly razorDocument: TextDocumentIdentifier) {} +} diff --git a/src/razor/src/dynamicFile/provideDynamicFileResponse.ts b/src/razor/src/dynamicFile/provideDynamicFileResponse.ts index 7be8d1f01..42aa48022 100644 --- a/src/razor/src/dynamicFile/provideDynamicFileResponse.ts +++ b/src/razor/src/dynamicFile/provideDynamicFileResponse.ts @@ -4,8 +4,12 @@ *--------------------------------------------------------------------------------------------*/ import { TextDocumentIdentifier } from 'vscode-languageclient/node'; +import { ServerTextChange } from '../rpc/serverTextChange'; // matches https://github.com/dotnet/roslyn/blob/9e91ca6590450e66e0041ee3135bbf044ac0687a/src/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/HostWorkspace/RazorDynamicFileInfoProvider.cs#L28 export class ProvideDynamicFileResponse { - constructor(public readonly csharpDocument: TextDocumentIdentifier | null) {} + constructor( + public readonly csharpDocument: TextDocumentIdentifier | null, + public readonly edits: ServerTextChange[] | null + ) {} } From 212e71030ad98e7ef70364a4779ead8f2bed0b1e Mon Sep 17 00:00:00 2001 From: Andrew Hall Date: Tue, 26 Nov 2024 18:21:13 -0800 Subject: [PATCH 2/2] Encapsulate each set of changes in an Update class --- .../src/csharp/csharpProjectedDocument.ts | 47 ++++++++++++------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/src/razor/src/csharp/csharpProjectedDocument.ts b/src/razor/src/csharp/csharpProjectedDocument.ts index 6e90a1de7..ec4cabe13 100644 --- a/src/razor/src/csharp/csharpProjectedDocument.ts +++ b/src/razor/src/csharp/csharpProjectedDocument.ts @@ -19,7 +19,7 @@ export class CSharpProjectedDocument implements IProjectedDocument { private resolveProvisionalEditAt: number | undefined; private ProvisionalDotPosition: Position | undefined; private hostDocumentVersion: number | null = null; - private edits: ServerTextChange[] | null = null; + private updates: Update[] | null = null; public constructor(public readonly uri: vscode.Uri) { this.path = getUriPath(uri); @@ -38,38 +38,45 @@ export class CSharpProjectedDocument implements IProjectedDocument { } public update(edits: ServerTextChange[], hostDocumentVersion: number) { + this.removeProvisionalDot(); + // Apply any stored edits if needed - if (this.edits) { - edits = this.edits.concat(edits); - this.edits = null; - } + if (this.updates) { + for (const update of this.updates) { + this.updateContent(update.changes); + } - this.removeProvisionalDot(); + this.updates = null; + } this.hostDocumentVersion = hostDocumentVersion; - this.updateContent(edits); } public storeEdits(edits: ServerTextChange[], hostDocumentVersion: number) { this.hostDocumentVersion = hostDocumentVersion; - if (this.edits) { - this.edits = this.edits.concat(edits); + if (this.updates) { + this.updates = this.updates.concat(new Update(edits)); } else { - this.edits = edits; + this.updates = [new Update(edits)]; } } public getAndApplyEdits() { - const edits = this.edits; + const updates = this.updates; + this.updates = null; + + if (updates) { + let changes: ServerTextChange[] = []; + for (const update of updates) { + this.updateContent(update.changes); + changes = changes.concat(update.changes); + } - // Make sure the internal representation of the content is updated - if (edits) { - this.updateContent(edits); + return changes; } - this.edits = null; - return edits; + return null; } public getContent() { @@ -158,8 +165,8 @@ export class CSharpProjectedDocument implements IProjectedDocument { } private getEditedContent(newText: string, start: number, end: number, content: string) { - const before = content.substr(0, start); - const after = content.substr(end); + const before = content.substring(0, start); + const after = content.substring(end); content = `${before}${newText}${after}`; return content; @@ -183,3 +190,7 @@ export class CSharpProjectedDocument implements IProjectedDocument { this.setContent(content); } } + +class Update { + constructor(public readonly changes: ServerTextChange[]) {} +}