Support Linked Editing Ranges in Cohosting. #10349
Conversation
Factored out common code into a static helper and created the remote part of the code as well as cohosting endpoing.
|
|
||
| namespace Microsoft.CodeAnalysis.Razor.LinkedEditingRange; | ||
|
|
||
| internal static class LinkedEditingRangeHelper |
There was a problem hiding this comment.
Decided to make this a static helper - one of the factored out methods was already static and I believe only one call needed a logger. Other than logger it doesn't really keep any state (and even logger seems like it's nice to have from whichever endpoing is using the helper code). Plus it's all temporary until we switch to cohosting full time. The three-pronged service seemed like an overkill here.
There was a problem hiding this comment.
Plus it's all temporary
We'll see 😁
There was a problem hiding this comment.
Looks great!
I didn't look too closely at the helper method, because I assume its all copy-and-paste, but there was a bit of cleanup that could be done there, if I'm getting nit-picky.
As far as testing goes, we don't have any testing for cohosting yet, and testing remote services has proved to be very tricky. Because the logic is shared though, we do get some testing coverage from the existing linked editing range tests. I wouldn't block on testing right now, but I'd be very keen to hear your thoughts about it if you wanted to have a go.
|
|
||
| namespace Microsoft.CodeAnalysis.Razor.LinkedEditingRange; | ||
|
|
||
| internal static class LinkedEditingRangeHelper |
There was a problem hiding this comment.
Plus it's all temporary
We'll see 😁
...osoft.CodeAnalysis.Remote.Razor/LinkedEditingRange/RemoteLinkedEditingRangeServiceFactory.cs
Outdated
Show resolved
Hide resolved
...isualStudio.LanguageServices.Razor/LanguageClient/Cohost/CohostLinkedEditingRangeEndpoint.cs
Outdated
Show resolved
Hide resolved
| var snapshot = _documentSnapshotFactory.GetOrCreate(razorDocument); | ||
| var codeDocument = await snapshot.GetGeneratedOutputAsync().ConfigureAwait(false); | ||
| var sourceText = await razorDocument.GetTextAsync(cancellationToken).ConfigureAwait(false); | ||
| var razorDocument = solution.GetAdditionalDocument(razorDocumentId); |
There was a problem hiding this comment.
I see what you're going for with the base class, and I like the initiative, but this line irks me, because we've already done this work in the base class. I think we need to think about the design of this a bit more in a follow up at some point.
There was a problem hiding this comment.
Yeah, I was thinking of maybe having GetCodeAndRazorDocuments and then a wrapper that discards the one we don't need. Does that sound reasonable?
* upstream/main: (374 commits) Don't use CancellationTokenSource for disposal Support Linked Editing Ranges in Cohosting. (#10349) Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2445915 Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2445915 Update 17.11 config after VS snap Update dependencies from https://github.com/dotnet/source-build-reference-packages build 20240502.1 Add test Don't think we need this any more Remove Base64 helpers now that they aren't used Whitespace is not significant, but it is important Add ProjectKey APIs for representing unknown projects Fix up serialization tests RazorProjectInfo.SerializationFilePath -> ProjectKey Clean up places were an invalid ProjectKey is used Move ProjectKey to MS.ANC.Razor.ProjectEngineHost Remove ProjectKey static construction methods Protect against exceptions when deleting files Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2443298 Localized file check-in by OneLocBuild Task: Build definition ID 262: Build ID 2443298 Correctly stitch `@` characters to following identifiers when rewriting tag helpers (#10331) ...
Factored out common code into a static helper and created the remote part of the code as well as cohosting endpoing.
### Summary of the changes
Publishing as draft initially as I'd like to do a bit more cleanup (some code can be factored out in some remote services into an extra base class) and more testing, but it's mostly there. Also need to test cohosting scenario :)