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.

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

Don't open closed documents #7826

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions src/lsptoolshost/roslynLanguageServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<RazorDynamicFileChangedParams>('razor/dynamicFileInfoChanged', notification)
);
}

// eslint-disable-next-line @typescript-eslint/promise-function-async
Expand Down
46 changes: 39 additions & 7 deletions src/razor/src/csharp/csharpProjectedDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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() {
Expand Down Expand Up @@ -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()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised this works, given elsewhere the edits are concated together. Wouldn't we need to insert subsequent edits at position 0?

Though that could be at odds with the Roslyn algorithm that processes the same edits?

// 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);
}
}
1 change: 1 addition & 0 deletions src/razor/src/document/IRazorDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export interface IRazorDocument {
readonly uri: vscode.Uri;
readonly csharpDocument: IProjectedDocument;
readonly htmlDocument: IProjectedDocument;
readonly isOpen: boolean;
}
32 changes: 32 additions & 0 deletions src/razor/src/document/razorDocument.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
12 changes: 3 additions & 9 deletions src/razor/src/document/razorDocumentFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
57 changes: 16 additions & 41 deletions src/razor/src/document/razorDocumentManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<IRazorDocument> {
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<IRazorDocument | null> {
if (!vscode.window.activeTextEditor) {
return null;
}
Expand Down Expand Up @@ -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);

Expand All @@ -159,7 +150,7 @@ export class RazorDocumentManager implements IRazorDocumentManager {
document = this.addDocument(uri);
}

return document;
return document!;
}

private async openDocument(uri: vscode.Uri) {
Expand All @@ -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();
}
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
}
54 changes: 38 additions & 16 deletions src/razor/src/dynamicFile/dynamicFileInfoHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand All @@ -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<ProvideDynamicFileResponse | null> {
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;
}

Expand Down
10 changes: 10 additions & 0 deletions src/razor/src/dynamicFile/dynamicFileUpdatedParams.ts
Original file line number Diff line number Diff line change
@@ -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) {}
}
6 changes: 5 additions & 1 deletion src/razor/src/dynamicFile/provideDynamicFileResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {}
}
Loading