Gracefully degrade if the cohost editor isn't going to work #11981
Conversation
fixup: create base class
This required a new virtual method, and it means we have to override the abstract method even though it won't be called. I don't love it, and if you have an alternate suggestion, I'm all ears!
Only LSP services have to be MEFv2
There was a problem hiding this comment.
Note: I'm specifically requesting changes in PathUtilities.
The test failures appear to be real. Also, because the test failures are Windows-only, it highlights a growing problem that our co-hosting tests require NetFx and don't on Mac or Linux. That's a test hole that needs to be addressed. Is there an issue tracking it?
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Cohost/IIncompatibleProjectService.cs
Outdated
Show resolved
Hide resolved
src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/PathUtilitiesTests.cs
Outdated
Show resolved
Hide resolved
src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/PathUtilities.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectCapabilityResolver.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectCapabilityResolver.cs
Outdated
Show resolved
Hide resolved
|
|
||
| if (vsHierarchy.GetProjectFilePath(_jtf) is { } projectFilePath) | ||
| { | ||
| _cachedCapabilities = _cachedCapabilities.SetItem((projectFilePath, capability), isMatch); |
There was a problem hiding this comment.
How does this cache get invalidated if the project is reloaded -- for example, if the user does a git pull and capabilities change?
There was a problem hiding this comment.
It doesn't. As below, this is not intended for critical uses (and i've updated the comments to reflect that).
src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectCapabilityResolver.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/IncompatibleProjectNotifier.cs
Outdated
Show resolved
Hide resolved
I don't disagree, this is something that has been annoying me for a bit too, and is on my list. I will formalize it into an issue though, for sure. Having said that, for the record, the test failure has nothing to do with being Windows only, it's just me not re-running all tests after a change. |
|
Updated with a full implementation of |
src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/ProjectCapabilityResolver.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/IProjectCapabilityResolver.cs
Outdated
Show resolved
Hide resolved
…ility check and notifier
Mostly fixes #11942
We still currently get "Couldn't get code document" errors when the source generator is hooked up, but didn't run, but I'm leaving that because it should be fixed by #11972, and if it isn't then we either want to know, or can add handling of it in then.
Commit-at-a-time is probably easiest, mostly because the 2nd commit changes every cohost endpoint to inherit from a new base class, so it's pretty noisy.