-
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
Cohosting: Update Html virtual document from OOP #10175
Merged
Merged
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
e5cf17d
Create a service to receive document updates from Roslyn
davidwengier 00fee20
Create remote service for getting Html document
davidwengier e23cef3
Define remote service files and attributes etc.
davidwengier 4ec3e36
Let cohosting handle Html virtual document updates
davidwengier dab18f4
Update tests
davidwengier 0a8e430
Merge remote-tracking branch 'upstream/main' into CohostHtmlDocuments
davidwengier 3bfa78a
Move to new way of specifying services MY GOD ITS SO MUCH EASIER NOW
davidwengier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
13 changes: 13 additions & 0 deletions
13
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/IRemoteHtmlDocumentService.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Razor; | ||
|
||
namespace Microsoft.CodeAnalysis.Razor.Remote; | ||
|
||
internal interface IRemoteHtmlDocumentService | ||
{ | ||
ValueTask<string?> GetHtmlDocumentTextAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId razorDocumentId, CancellationToken cancellationToken); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
42 changes: 42 additions & 0 deletions
42
src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/HtmlDocuments/RemoteHtmlDocumentService.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Razor; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Api; | ||
using Microsoft.CodeAnalysis.Razor.Remote; | ||
using Microsoft.CodeAnalysis.Razor.Workspaces; | ||
using Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem; | ||
using Microsoft.ServiceHub.Framework; | ||
|
||
namespace Microsoft.CodeAnalysis.Remote.Razor; | ||
|
||
internal sealed class RemoteHtmlDocumentService( | ||
IServiceBroker serviceBroker, | ||
DocumentSnapshotFactory documentSnapshotFactory) | ||
: RazorServiceBase(serviceBroker), IRemoteHtmlDocumentService | ||
{ | ||
private readonly DocumentSnapshotFactory _documentSnapshotFactory = documentSnapshotFactory; | ||
|
||
public ValueTask<string?> GetHtmlDocumentTextAsync(RazorPinnedSolutionInfoWrapper solutionInfo, DocumentId razorDocumentId, CancellationToken cancellationToken) | ||
=> RazorBrokeredServiceImplementation.RunServiceAsync( | ||
solutionInfo, | ||
ServiceBrokerClient, | ||
solution => GetHtmlDocumentTextAsync(solution, razorDocumentId, cancellationToken), | ||
cancellationToken); | ||
|
||
private async ValueTask<string?> GetHtmlDocumentTextAsync(Solution solution, DocumentId razorDocumentId, CancellationToken _) | ||
{ | ||
var razorDocument = solution.GetAdditionalDocument(razorDocumentId); | ||
if (razorDocument is null) | ||
{ | ||
return null; | ||
} | ||
|
||
var documentSnapshot = _documentSnapshotFactory.GetOrCreate(razorDocument); | ||
var codeDocument = await documentSnapshot.GetGeneratedOutputAsync(); | ||
|
||
return codeDocument.GetHtmlSourceText().ToString(); | ||
} | ||
} |
24 changes: 24 additions & 0 deletions
24
...src/Microsoft.CodeAnalysis.Remote.Razor/HtmlDocuments/RemoteHtmlDocumentServiceFactory.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
// Copyright (c) .NET Foundation. All rights reserved. | ||
// Licensed under the MIT license. See License.txt in the project root for license information. | ||
|
||
using Microsoft.CodeAnalysis.Razor.Remote; | ||
using Microsoft.CodeAnalysis.Remote.Razor.ProjectSystem; | ||
using Microsoft.ServiceHub.Framework; | ||
using Microsoft.VisualStudio.Composition; | ||
|
||
namespace Microsoft.CodeAnalysis.Remote.Razor; | ||
|
||
internal sealed class RemoteHtmlDocumentServiceFactory : RazorServiceFactoryBase<IRemoteHtmlDocumentService> | ||
{ | ||
// WARNING: We must always have a parameterless constructor in order to be properly handled by ServiceHub. | ||
public RemoteHtmlDocumentServiceFactory() | ||
: base(RazorServices.Descriptors) | ||
{ | ||
} | ||
|
||
protected override IRemoteHtmlDocumentService CreateService(IServiceBroker serviceBroker, ExportProvider exportProvider) | ||
{ | ||
var documentSnapshotFactory = exportProvider.GetExportedValue<DocumentSnapshotFactory>(); | ||
return new RemoteHtmlDocumentService(serviceBroker, documentSnapshotFactory); | ||
} | ||
} |
85 changes: 85 additions & 0 deletions
85
...Microsoft.VisualStudio.LanguageServerClient.Razor/Cohost/CohostTextDocumentSyncHandler.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
// 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.AspNetCore.Razor; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Razor; | ||
using Microsoft.CodeAnalysis.ExternalAccess.Razor.Cohost; | ||
using Microsoft.CodeAnalysis.Razor.Logging; | ||
using Microsoft.CodeAnalysis.Razor.Remote; | ||
using Microsoft.Extensions.Logging; | ||
using Microsoft.VisualStudio.LanguageServer.ContainedLanguage; | ||
using Microsoft.VisualStudio.LanguageServer.Protocol; | ||
using Microsoft.VisualStudio.Threading; | ||
|
||
namespace Microsoft.VisualStudio.LanguageServerClient.Razor.Cohost; | ||
|
||
[Export(typeof(IRazorCohostTextDocumentSyncHandler)), Shared] | ||
[method: ImportingConstructor] | ||
internal class CohostTextDocumentSyncHandler( | ||
IRemoteClientProvider remoteClientProvider, | ||
LSPDocumentManager documentManager, | ||
JoinableTaskContext joinableTaskContext, | ||
IRazorLoggerFactory loggerFactory) : IRazorCohostTextDocumentSyncHandler | ||
{ | ||
private readonly IRemoteClientProvider _remoteClientProvider = remoteClientProvider; | ||
private readonly JoinableTaskContext _joinableTaskContext = joinableTaskContext; | ||
private readonly TrackingLSPDocumentManager _documentManager = documentManager as TrackingLSPDocumentManager ?? throw new InvalidOperationException("Expected TrackingLSPDocumentManager"); | ||
private readonly ILogger _logger = loggerFactory.CreateLogger<CohostTextDocumentSyncHandler>(); | ||
|
||
public async Task HandleAsync(int version, RazorCohostRequestContext context, CancellationToken cancellationToken) | ||
{ | ||
// For didClose, we don't need to do anything. We can't close the virtual document, because that requires the buffer | ||
// so we just no-op and let our VS components handle closure. | ||
if (context.Method == Methods.TextDocumentDidCloseName) | ||
{ | ||
return; | ||
} | ||
|
||
var textDocument = context.TextDocument.AssumeNotNull(); | ||
var textDocumentPath = context.TextDocument.FilePath.AssumeNotNull(); | ||
|
||
_logger.LogDebug("[Cohost] {method} of '{document}' with version {version}.", context.Method, textDocumentPath, version); | ||
|
||
var client = await _remoteClientProvider.TryGetClientAsync(cancellationToken); | ||
if (client is null) | ||
{ | ||
_logger.LogError("[Cohost] Couldn't get remote client for {method} of '{document}'. Html document contents will be stale", context.Method, textDocumentPath); | ||
return; | ||
} | ||
|
||
var htmlText = await client.TryInvokeAsync<IRemoteHtmlDocumentService, string?>(textDocument.Project.Solution, | ||
(service, solutionInfo, ct) => service.GetHtmlDocumentTextAsync(solutionInfo, textDocument.Id, ct), | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
if (!htmlText.HasValue || htmlText.Value is null) | ||
{ | ||
_logger.LogError("[Cohost] Couldn't get Html text for {method} of '{document}'. Html document contents will be stale", context.Method, textDocumentPath); | ||
return; | ||
} | ||
|
||
// Eventually, for VS Code, the following piece of logic needs to make an LSP call rather than directly update the buffer | ||
|
||
// Roslyn might have got changes from the LSP server, but we just get the actual source text, so we just construct one giant change | ||
// from that. Guaranteed no sync issues, though we are passing long strings around unfortunately. | ||
var uri = textDocument.CreateUri(); | ||
if (!_documentManager.TryGetDocument(uri, out var documentSnapshot) || | ||
!documentSnapshot.TryGetVirtualDocument<HtmlVirtualDocumentSnapshot>(out var htmlDocument)) | ||
{ | ||
Debug.Fail("Got an LSP text document update before getting informed of the VS buffer"); | ||
_logger.LogError("[Cohost] Couldn't get an Html document for {method} of '{document}'. Html document contents will be stale (or non-existent?)", context.Method, textDocumentPath); | ||
return; | ||
} | ||
|
||
await _joinableTaskContext.Factory.SwitchToMainThreadAsync(cancellationToken); | ||
|
||
VisualStudioTextChange[] changes = [new(0, htmlDocument.Snapshot.Length, htmlText.Value)]; | ||
_documentManager.UpdateVirtualDocument<HtmlVirtualDocument>(uri, changes, version, state: null); | ||
|
||
_logger.LogDebug("[Cohost] Exiting {method} of '{document}' with version {version}.", context.Method, textDocumentPath, version); | ||
} | ||
} |
11 changes: 11 additions & 0 deletions
11
...Studio.RazorExtension/Microsoft.VisualStudio.Razor.HtmlDocument64.servicehub.service.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"host": "netfx.anycpu", | ||
"hostId": "RoslynCodeAnalysisService", | ||
"hostGroupAllowed": true, | ||
"entryPoint": { | ||
"assemblyPath": "Microsoft.CodeAnalysis.Remote.Razor.dll", | ||
"fullClassName": "Microsoft.CodeAnalysis.Remote.Razor.RemoteHtmlDocumentServiceFactory", | ||
"appBasePath": "%VSAPPIDDIR%", | ||
"configPath": "%PkgDefApplicationConfigFile%" | ||
} | ||
} |
11 changes: 11 additions & 0 deletions
11
...tudio.RazorExtension/Microsoft.VisualStudio.Razor.HtmlDocument64S.servicehub.service.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"host": "netfx.anycpu", | ||
"hostId": "RoslynCodeAnalysisServiceS", | ||
"hostGroupAllowed": true, | ||
"entryPoint": { | ||
"assemblyPath": "Microsoft.CodeAnalysis.Remote.Razor.dll", | ||
"fullClassName": "Microsoft.CodeAnalysis.Remote.Razor.RemoteHtmlDocumentServiceFactory", | ||
"appBasePath": "%VSAPPIDDIR%", | ||
"configPath": "%PkgDefApplicationConfigFile%" | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
...on/ServiceHubCore/Microsoft.VisualStudio.Razor.HtmlDocumentCore64.servicehub.service.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"host": "dotnet", | ||
"hostId": "RoslynCodeAnalysisService", | ||
"hostGroupAllowed": true, | ||
"entryPoint": { | ||
"assemblyPath": "Microsoft.CodeAnalysis.Remote.Razor.dll", | ||
"fullClassName": "Microsoft.CodeAnalysis.Remote.Razor.RemoteHtmlDocumentServiceFactory" | ||
}, | ||
"friendServices": [ "Microsoft.VisualStudio.LanguageServices.DiagnosticAnalyzerCore64" ] | ||
} |
10 changes: 10 additions & 0 deletions
10
...n/ServiceHubCore/Microsoft.VisualStudio.Razor.HtmlDocumentCore64S.servicehub.service.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
{ | ||
"host": "dotnet", | ||
"hostId": "RoslynCodeAnalysisServiceS", | ||
"hostGroupAllowed": true, | ||
"entryPoint": { | ||
"assemblyPath": "Microsoft.CodeAnalysis.Remote.Razor.dll", | ||
"fullClassName": "Microsoft.CodeAnalysis.Remote.Razor.RemoteHtmlDocumentServiceFactory" | ||
}, | ||
"friendServices": [ "Microsoft.VisualStudio.LanguageServices.DiagnosticAnalyzerCore64S" ] | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
These changes look worse than they are. What I did was rearrange the entries so that each service is grouped together, and I noticed some paths used the
ServiceHubCoreSubPath
variable, and some hardcoded the path, so I fixed that too.I'm going to prioritize #10106 and get rid of all of this though, because I lost about half a day trying to get this new service to work, only to discover than Roslyn strips the word "Service" off the end of a service, and assumes that the json files do the same thing.