Always use a unique virtual document file path #688
Conversation
It is shared with the LSP Client
| // create output channel for extension logs and lsp client logs | ||
| const outputChannel = vscode.window.createOutputChannel("Quarto", { log: true }); | ||
|
|
||
| outputChannel.info("Activating Quarto extension."); |
There was a problem hiding this comment.
I found it extremely useful to have an outputChannel to log to while debugging. I plumbed this through to points of interest so I could log.
I have since removed the log calls, but I still think this is quite useful for future us so I've left the actual output channel in to make this easier later on.
It is shared with the LSP client, essentially we provide our own output channel now rather than having the LSP client generate one automatically for us (we do the same thing in positron)
There was a problem hiding this comment.
I have streamlined this function quite a bit. I found that pretty much everything was shared code, and we don't actually need an "early exit" path for the local case - it's just the normal path tweaked to create the file in a known directory rather than a temp directory.
The diffs are a little large but I hope it is a big improvement overall
(PS, it is much easier to review this file in Split View in the github ui)
There was a problem hiding this comment.
Really the main actual change is that the local branch that used to exist used a static .vdoc.{ext} as the file path, which was being overwritten in the middle of an lsp request. It now uses .vdoc.{uuid}.{ext} to avoid this.
| * language server request is running (#683). | ||
| */ | ||
| function generateVirtualDocFilepath(directory: string, extension: string): string { | ||
| return path.join(directory, ".vdoc." + uuid.v4() + "." + extension); |
There was a problem hiding this comment.
Worth noting that for non-local LSP requests this was called .intellisense.{uuid}.{ext} but I do not think that name mattered. We now always use .vdoc.{uuid}.{extension}.
cscheid
left a comment
cscheid
left a comment
There was a problem hiding this comment.
Very good fix, LGTM.
The only comment I have is that this fix makes me wonder about other data races, and a code review might be in order...
To get the fix in quarto-dev/quarto#688 that addresses the virtual document problems observed in quarto-dev/quarto#683 (reported here in Positron several times as well)
2 participants
Closes #683
See full analysis in #683 (comment)
I have managed to reliably (or at least, quickly) reproduce the problem by repeatedly mashing
Cmd + Swhile also moving my mouse around over the code in this qmd (i.e. simultaneously invoking many hover requests). I have not been able to reproduce the issue this way with this PR.