-
Notifications
You must be signed in to change notification settings - Fork 196
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
Move C# and Html document generation (ie, compilation) to the Cohost side of the world #9766
Changes from 1 commit
18bc37c
4398c6a
34f01cd
78ad08f
8f51c5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,148 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
||
using System; | ||
using System.Composition; | ||
using System.Diagnostics; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Razor.Logging; | ||
using Microsoft.CodeAnalysis.Razor.ProjectSystem; | ||
using Microsoft.CodeAnalysis.Razor.Workspaces; | ||
using Microsoft.CodeAnalysis.Razor.Workspaces.Extensions; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.VisualStudio.LanguageServer.ContainedLanguage; | ||
using Microsoft.VisualStudio.Threading; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServerClient.Razor.Cohost; | ||
|
||
[Export(typeof(OpenDocumentGenerator)), Shared] | ||
[method: ImportingConstructor] | ||
internal sealed class OpenDocumentGenerator( | ||
LanguageServerFeatureOptions languageServerFeatureOptions, | ||
LSPDocumentManager documentManager, | ||
CSharpVirtualDocumentAddListener csharpVirtualDocumentAddListener, | ||
JoinableTaskContext joinableTaskContext, | ||
IRazorLoggerFactory loggerFactory) | ||
{ | ||
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions ?? throw new ArgumentNullException(nameof(languageServerFeatureOptions)); | ||
private readonly TrackingLSPDocumentManager _documentManager = documentManager as TrackingLSPDocumentManager ?? throw new ArgumentNullException(nameof(documentManager)); | ||
private readonly CSharpVirtualDocumentAddListener _csharpVirtualDocumentAddListener = csharpVirtualDocumentAddListener ?? throw new ArgumentNullException(nameof(csharpVirtualDocumentAddListener)); | ||
private readonly JoinableTaskContext _joinableTaskContext = joinableTaskContext; | ||
private readonly ILogger _logger = loggerFactory.CreateLogger<OpenDocumentGenerator>(); | ||
|
||
public async Task DocumentOpenedOrChangedAsync(IDocumentSnapshot document, int version, CancellationToken cancellationToken) | ||
{ | ||
// These flags exist to workaround things in VS Code, so bringing cohosting to VS Code without also fixing these flags, is very silly. | ||
Debug.Assert(_languageServerFeatureOptions.IncludeProjectKeyInGeneratedFilePath); | ||
Debug.Assert(!_languageServerFeatureOptions.UpdateBuffersForClosedDocuments); | ||
|
||
// Actually do the generation | ||
var generatedOutput = await document.GetGeneratedOutputAsync().ConfigureAwait(false); | ||
|
||
// Now we have to update the LSP buffer etc. | ||
// Fortunate this code will be removed in time | ||
var hostDocumentUri = new Uri(document.FilePath); | ||
|
||
_logger.LogDebug("[Cohost] Updating generated document buffers for {version} of {uri} in {projectKey}", version, hostDocumentUri, document.Project.Key); | ||
|
||
if (_documentManager.TryGetDocument(hostDocumentUri, out var documentSnapshot)) | ||
{ | ||
// Html | ||
var htmlVirtualDocumentSnapshot = TryGetHtmlSnapshot(documentSnapshot); | ||
|
||
// CSharp | ||
var csharpVirtualDocumentSnapshot = await TryGetCSharpSnapshotAsync(documentSnapshot, document.Project.Key, version, cancellationToken).ConfigureAwait(false); | ||
|
||
// Buffer work has to be on the UI thread | ||
await _joinableTaskContext.Factory.SwitchToMainThreadAsync(cancellationToken); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Debug.Assert(htmlVirtualDocumentSnapshot is not null && csharpVirtualDocumentSnapshot is not null || | ||
htmlVirtualDocumentSnapshot is null && csharpVirtualDocumentSnapshot is null, "Found a Html XOR a C# document. Expected both or neither."); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if (htmlVirtualDocumentSnapshot is not null) | ||
{ | ||
_documentManager.UpdateVirtualDocument<HtmlVirtualDocument>( | ||
hostDocumentUri, | ||
[new VisualStudioTextChange(0, htmlVirtualDocumentSnapshot.Snapshot.Length, generatedOutput.GetHtmlSourceText().ToString())], | ||
version, | ||
state: null); | ||
} | ||
|
||
if (csharpVirtualDocumentSnapshot is not null) | ||
{ | ||
_documentManager.UpdateVirtualDocument<CSharpVirtualDocument>( | ||
hostDocumentUri, | ||
csharpVirtualDocumentSnapshot.Uri, | ||
[new VisualStudioTextChange(0, csharpVirtualDocumentSnapshot.Snapshot.Length, generatedOutput.GetCSharpSourceText().ToString())], | ||
version, | ||
state: null); | ||
return; | ||
} | ||
} | ||
} | ||
|
||
private async Task<CSharpVirtualDocumentSnapshot?> TryGetCSharpSnapshotAsync(LSPDocumentSnapshot documentSnapshot, ProjectKey projectKey, int version, CancellationToken cancellationToken) | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
if (documentSnapshot.TryGetAllVirtualDocuments<CSharpVirtualDocumentSnapshot>(out var virtualDocuments)) | ||
{ | ||
if (virtualDocuments is [{ ProjectKey.Id: null }]) | ||
{ | ||
// If there is only a single virtual document, and its got a null id, then that means it's in our "misc files" project | ||
// but the server clearly knows about it in a real project. That means its probably new, as Visual Studio opens a buffer | ||
// for a document before we get the notifications about it being added to any projects. Lets try refreshing before | ||
// we worry. | ||
_logger.LogDebug("[Cohost] Refreshing virtual documents, and waiting for them, (for {hostDocumentUri})", documentSnapshot.Uri); | ||
|
||
var task = _csharpVirtualDocumentAddListener.WaitForDocumentAddAsync(cancellationToken); | ||
_documentManager.RefreshVirtualDocuments(); | ||
_ = await task.ConfigureAwait(true); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Since we're dealing with snapshots, we have to get the new ones after refreshing | ||
if (!_documentManager.TryGetDocument(documentSnapshot.Uri, out var newDocumentSnapshot) || | ||
!newDocumentSnapshot.TryGetAllVirtualDocuments<CSharpVirtualDocumentSnapshot>(out virtualDocuments)) | ||
{ | ||
// This should never happen. | ||
// The server clearly wants to tell us about a document in a project, but we don't know which project it's in. | ||
// Sadly there isn't anything we can do here to, we're just in a state where the server and client are out of | ||
// sync with their understanding of the document contents, and since changes come in as a list of changes, | ||
// the user experience is broken. All we can do is hope the user closes and re-opens the document. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
Debug.Fail($"Server wants to update {documentSnapshot.Uri} in {projectKey} but we don't know about the document being in any projects"); | ||
_logger.LogError("[Cohost] Server wants to update {hostDocumentUri} in {projectKeyId} by we only know about that document in misc files. Server and client are now out of sync.", documentSnapshot.Uri, projectKey); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might as well include a "closing and opening the document may fix this issue" if a user happens to look at the logs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we fault on states like this? Is there really nothing we can do, or is it we don't know what causes this yet? |
||
return null; | ||
} | ||
} | ||
|
||
foreach (var virtualDocument in virtualDocuments) | ||
{ | ||
if (virtualDocument.ProjectKey.Equals(projectKey)) | ||
{ | ||
_logger.LogDebug("[Cohost] Found C# virtual doc for {version}: {uri}", version, virtualDocument.Uri); | ||
|
||
return virtualDocument; | ||
} | ||
} | ||
|
||
if (virtualDocuments.Length > 1) | ||
{ | ||
// If the particular document supports multiple virtual documents, we don't want to try to update a single one | ||
// TODO: Remove this eventually, as it is a possibly valid state (see comment below) but this assert will help track | ||
// down bugs for now. | ||
Debug.Fail("Multiple virtual documents seem to be supported, but none were updated, which is not impossible, but surprising."); | ||
} | ||
|
||
_logger.LogDebug("[Cohost] Couldn't find any virtual docs for {version} of {uri}", version, documentSnapshot.Uri); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// Don't know about document, no-op. This can happen if the language server found a project.razor.bin from an old build | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// and is sending us updates. | ||
} | ||
|
||
return null; | ||
} | ||
|
||
private static HtmlVirtualDocumentSnapshot? TryGetHtmlSnapshot(LSPDocumentSnapshot documentSnapshot) | ||
{ | ||
_ = documentSnapshot.TryGetVirtualDocument<HtmlVirtualDocumentSnapshot>(out var htmlVirtualDocumentSnapshot); | ||
davidwengier marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return htmlVirtualDocumentSnapshot; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just for my education - curious why projectSnapshotManager needs it's own dispatcher and when should we run on that dispatcher's thread. We can chat in sync-up if it's a long answer :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a long, and hopefully eventually removed, answer :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The questions in this PR are all excellent and unfortunately the answer to most of them is "yes, this is not ideal, but hopefully we can remove it eventually" 😁