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

Merged
merged 13 commits into from
Dec 6, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
}
},
"defaults": {
"roslyn": "4.13.0-3.24577.4",
"roslyn": "4.13.0-3.24605.12",
"omniSharp": "1.39.11",
"razor": "9.0.0-preview.24569.4",
"razor": "9.0.0-preview.24605.1",
"razorOmnisharp": "7.0.0-preview.23363.1",
"xamlTools": "17.13.35527.19"
},
Expand Down
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
114 changes: 102 additions & 12 deletions src/razor/src/csharp/csharpProjectedDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ export class CSharpProjectedDocument implements IProjectedDocument {
private resolveProvisionalEditAt: number | undefined;
private ProvisionalDotPosition: Position | undefined;
private hostDocumentVersion: number | null = null;
private updates: CSharpDocumentUpdate[] | null = null;
private _checksum: string = '';
private _checksumAlgorithm: number = 1; // Default to Sha1
private _encodingCodePage: number | null = null;

public constructor(public readonly uri: vscode.Uri) {
this.path = getUriPath(uri);
Expand All @@ -36,22 +40,78 @@ export class CSharpProjectedDocument implements IProjectedDocument {
this.setContent('');
}

public update(edits: ServerTextChange[], hostDocumentVersion: number) {
this.removeProvisionalDot();
public get checksum(): string {
return this._checksum;
}

this.hostDocumentVersion = hostDocumentVersion;
public get checksumAlgorithm(): number {
return this._checksumAlgorithm;
}

if (edits.length === 0) {
return;
public get encodingCodePage(): number | null {
return this._encodingCodePage;
}

public update(
hostDocumentIsOpen: boolean,
edits: ServerTextChange[],
hostDocumentVersion: number,
checksum: string,
checksumAlgorithm: number,
encodingCodePage: number | null
) {
if (hostDocumentIsOpen) {
this.removeProvisionalDot();

// Apply any stored edits if needed
if (this.updates) {
for (const update of this.updates) {
this.updateContent(update.changes);
}

this.updates = null;
}

this.updateContent(edits);
this._checksum = checksum;
this._checksumAlgorithm = checksumAlgorithm;
this._encodingCodePage = encodingCodePage;
} else {
const update = new CSharpDocumentUpdate(edits, checksum, checksumAlgorithm, encodingCodePage);

if (this.updates) {
this.updates = this.updates.concat(update);
} else {
this.updates = [update];
}
}

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.hostDocumentVersion = hostDocumentVersion;
}

public applyEdits(): ApplyEditsResponse {
const updates = this.updates;
this.updates = null;

const originalChecksum = this._checksum;
const originalChecksumAlgorithm = this._checksumAlgorithm;
const originalEncodingCodePage = this._encodingCodePage;

if (updates) {
for (const update of updates) {
this.updateContent(update.changes);
this._checksum = update.checksum;
this._checksumAlgorithm = update.checksumAlgorithm;
this._encodingCodePage = update.encodingCodePage;
}
}

this.setContent(content);
return {
edits: updates,
originalChecksum: originalChecksum,
originalChecksumAlgorithm: originalChecksumAlgorithm,
originalEncodingCodePage: originalEncodingCodePage,
};
}

public getContent() {
Expand Down Expand Up @@ -140,8 +200,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;
Expand All @@ -150,4 +210,34 @@ 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good point. I don't think I've actually hit a scenario where they got concat together in testing so probably missed this. Idk why reverse was used initially. Let me look...

Copy link
Contributor

Choose a reason for hiding this comment

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

Reverse makes sense for a single batch of ordered changes, it means you don't need to track forward when applying edits earlier in the file, and adjust positions later in the file. But the batches would need to be applied in order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't realize they were guaranteed to be ordered. Or that it was guaranteed to not overlap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I can just update to applying stored edits first instead of doing a concat

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be better now. I updated to keep track of Updates as individual units sent from Razor. How they get applied remains consistent and the edits being returned on ordered to be sent back to Roslyn

// 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);
}
}

export class CSharpDocumentUpdate {
constructor(
public readonly changes: ServerTextChange[],
public readonly checksum: string,
public readonly checksumAlgorithm: number,
public readonly encodingCodePage: number | null
) {}
}

export interface ApplyEditsResponse {
edits: CSharpDocumentUpdate[] | null;
originalChecksum: string;
originalChecksumAlgorithm: number;
originalEncodingCodePage: number | null;
}
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
54 changes: 13 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,14 @@ export class RazorDocumentManager implements IRazorDocumentManager {
csharpProjectedDocument.clear();
}

csharpProjectedDocument.update(updateBufferRequest.changes, updateBufferRequest.hostDocumentVersion);
csharpProjectedDocument.update(
document.isOpen,
updateBufferRequest.changes,
updateBufferRequest.hostDocumentVersion,
updateBufferRequest.checksum,
updateBufferRequest.checksumAlgorithm,
updateBufferRequest.encodingCodePage
);

this.notifyDocumentChange(document, RazorDocumentChangeKind.csharpChanged);
} else {
Expand Down Expand Up @@ -342,22 +332,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);
}
}
Loading