Port validate breakpoint endpoint to cohosting #11352
Conversation
davidwengier
changed the title
src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/Debugging/RemoteDebugInfoService.cs
Outdated
Show resolved
Hide resolved
...soft.VisualStudio.LanguageServices.Razor/LanguageClient/Debugging/RazorBreakpointResolver.cs
Outdated
Show resolved
Hide resolved
...soft.VisualStudio.LanguageServices.Razor/LanguageClient/Debugging/RazorBreakpointResolver.cs
Outdated
Show resolved
Hide resolved
| // We've seen this request before. Hopefully the TryGetTextVersion call above was successful so this whole path | ||
| // will have been sync, and the cache will have been useful. | ||
| return cachedRange; | ||
| } |
There was a problem hiding this comment.
Nit: Seems like mostly the same as first part of equivalent method of RazorBreakpointResolver? No huge savings, but I wonder if a generic helper method (with ChacheKey subtype being type parameter) or maybe even a simple base class would make sense. No strong feelings here, just seeing almost identical code as I'm reading it.
There was a problem hiding this comment.
I think if there were more of these sorts of things, I could see us having two different interfaces (ie ILspBreakpointResolver and ICohostBreakpointResolver) and each impl could share a base class etc. but I'm not sure it makes sense for just two. Perhaps in future if something like this comes up we could do something interesting with some custom MEF attributes so that the right impl gets magically imported.
Fixes #11337
Needs dotnet/roslyn#76629
Ports one endpoint to cohosting
Updates two
IVsLanguageInfoentry points to call cohosting in OOP rather than making LSP calls.These mostly just call Roslyn and don't do anything interesting with the results. I thought about making some changes here (eg, does
__oneed to be a proximity expression?) but given the very low test coverage of our existing code, I didn't want to rock the boat.