Don't open closed documents #7826
Conversation
ryzngard
changed the title
| } | ||
|
|
||
| let content = this.content; | ||
| for (const edit of edits.reverse()) { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I didn't realize they were guaranteed to be ordered. Or that it was guaranteed to not overlap.
There was a problem hiding this comment.
I think I can just update to applying stored edits first instead of doing a concat
There was a problem hiding this comment.
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
| 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); |
There was a problem hiding this comment.
Should this reset any stored edits, so that when a document is closed, things have a clean slate?
ie, when a document is opened, LSP will send a didOpen with the full contents, then any future edits, and then when its closed, are we at risk of sending some edits that were made before the open?
| const csharpDocument = razorDocument.csharpDocument as CSharpProjectedDocument; | ||
| const edits = csharpDocument.getAndApplyEdits(); | ||
|
|
||
| return new ProvideDynamicFileResponse({ uri: virtualCsharpUri }, edits ?? null); |
There was a problem hiding this comment.
Given this can send null, and open docs always do, and thinking about the above comment, I wonder if these two branches can be combined. ie, on open, clear the edits, then provide the dynamic file response with edits ?? null.
This updates the dynamic document system to keep track of edits for closed documents and report those to Roslyn. This helps us avoid creating buffers and opening documents to send content across, which should help avoid any issues with keeping buffers open as well as no longer block the Roslyn LSP on background document changes.
VS Code side of dotnet/roslyn#76050