Snapshot TextDocuments when delayOpenNotifications=true to avoid serving updated info to middleware/server#1755
Snapshot TextDocuments when delayOpenNotifications=true to avoid serving updated info to middleware/server#1755DanTup wants to merge 2 commits into
Conversation
…ing updated info to middleware/server When delayOpenNotifications is enabled and an open notification is delayed, we cannot use the live VS Code TextDocument to pass to middleware or to the server because it may have updated version/content info. Instead, capture a snapshot of the document at the time the original notification would've been sent, and then when the notification will be sent, provide that snapshot to the middleware and build the server parameters from it. Fixes microsoft#1695
DanTup
force-pushed
the
delay-open-notifications
branch
from
3b58e46 to
d12ba08
Compare
| } No newline at end of file | ||
| } | ||
|
|
||
| class TextDocumentSnapshot implements TextDocument { |
There was a problem hiding this comment.
@dbaeumer the implementation here was written by GPT-5.4 with some tidying up by me. I don't like that so much is repeated here from VS Code (and I haven't tried to verify the behaviour matches perfectly, because I'm hoping you might have a better idea :-))
I did add 'vscode-languageserver-textdocument' but it only provided a small number of the methods we needed.
I also noticed there is a very similar class here, but it uses some additional types from VS Code so I couldn't just lift it:
I wonder if it'd be better adding a good implementation of this to vscode-languageserver-textdocument and then using it both in Copilot and here?
|
|
||
| class TextDocumentSnapshot implements TextDocument { | ||
|
|
||
| private static readonly DefaultWordRegExp: RegExp = /(-?\d*\.\d\w*)|([^\s`~!@#%^&*()=+[\]{}\\|;:'",.<>/?-]+)/g; |
There was a problem hiding this comment.
Question: did you copy this from somewhere?
There was a problem hiding this comment.
No, as noted above - most of the implementation here was written by GPT 5.4. Although TypeScript has a very similar class here:
It's possible that this was in its training (it uses DEFAULT_WORD_REGEXP imported from the VS Code source).
I don't like having all this code here, and since TS has a version too, but would really be nice if they could be shared in some way. This feels a bit fragile (and I have not fully reviewed the generated code here yet, because I was hoping you had a better idea 😄)
There was a problem hiding this comment.
I think this might also be incorrect depending on the language?
| } | ||
|
|
||
| public save(): Thenable<boolean> { | ||
| return this.liveVsCodeDocument.save(); |
There was a problem hiding this comment.
I would only forward this to the live document if both documents have the same version. If not I would return false, indicating that the save didn't happen
|
@DanTup nice work. |
2 participants
When delayOpenNotifications is enabled and an open notification is delayed, we cannot use the live VS Code TextDocument to pass to middleware or to the server because it may have updated version/content info.
Instead, capture a snapshot of the document at the time the original notification would've been sent, and then when the notification will be sent, provide that snapshot to the middleware and build the server parameters from it.
Fixes #1695