-
Notifications
You must be signed in to change notification settings - Fork 214
Remove Razor EA For Roslyn #11510
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
Remove Razor EA For Roslyn #11510
Conversation
Do you want reviews? I'll admit, the first thing I saw in here was code that used to be in ExternalAccess.RoslynWorkspace, which used to be VS Code exclusive, moved to MS.VS.RazorExtension which used to be VS exclusive, and I got very confused by what the intent was. Then I re-read it! :) |
I don't think it's ready for reviews yet |
Feel free to ping in Teams when you're ready. |
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.
Mostly LGTM. I skipped the one file with the merge conflict markers though 😛
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Razor.Microbenchmarks.Generator", "src\Compiler\perf\Microsoft.AspNetCore.Razor.Microbenchmarks.Generator\Microsoft.AspNetCore.Razor.Microbenchmarks.Generator.csproj", "{7400A168-2552-49C7-93E3-D4DAA90C216F}" | ||
EndProject | ||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace", "src\Razor\src\Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace\Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.csproj", "{2223B8FD-D98A-47BE-94A9-6A3A6B8557B8}" | ||
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.VisualStudioCode.RazorExtension", "src\Razor\src\Microsoft.VisualStudioCode.RazorExtension\Microsoft.VisualStudioCode.RazorExtension.csproj", "{2223B8FD-D98A-47BE-94A9-6A3A6B8557B8}" |
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.
I don't know of a better name, and I don't think it matters, but I would like to state for the record that Microsoft.VisualStudio.RazorExtension
is a visual studio extension and Microsoft.VisualStudioCode.RazorExtension
is not a visual studio code extension 😛
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.
I don't love the name either. It's a bit too generic, and MS.VS.RazorExtension is not something to emulate. 😄
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.
Yea... I didn't love writing it but wasn't able to come up with an alternative.
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.
Same. 😢
.../src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/RazorWorkspaceListenerBase.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Endpoints/RazorFileChangedResponse.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorDynamicFileUpdate.cs
Outdated
Show resolved
Hide resolved
...or/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorLspDynamicFileInfoProvider.cs
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.VisualStudioCode.RazorExtension/Services/RazorWorkspaceService.cs
Outdated
Show resolved
Hide resolved
…nto remove_razor_ea
using System.Collections.Immutable; | ||
|
||
namespace Microsoft.AspNetCore.Razor.ProjectEngineHost; | ||
namespace Microsoft.CodeAnalysis.Razor.Workspaces.ProjectEngineHost; |
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 can be fixed later, but we generally don't add "Workspaces" into namespaces in MS.CA.Razor.Workspaces anymore. This should probably have been Microsoft.CodeAnalysis.Razor.ProjectEngineHost, but please feel free to fix that later or tell somebody else to do it. 😄
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.
Shouldn't this (and other files moved to this folder) switch to the Microsoft.CodeAnalysis.Razor.ProjectSystem namespace?
Depends on the following PRs: dotnet/razor#11510 dotnet/roslyn#77715
Razor side of dotnet/roslyn#77715
Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace
, we no longer ship an EA for RoslynMicrosoft.VisualStudioCode.RazorExtension
and package that will be put in.razorExtension
in VS CodeMicrosoft.AspNetCore.Razor.ProjectEngineHost
as that layer of separation is no longer neededCopilot description below
This pull request includes several changes to the
Razor
project, focusing on project restructuring and dependency updates. The most important changes include adding a new package version, renaming and removing projects, and updating theRazorWorkspaceListenerBase
class.Project restructuring:
Razor.sln
: RenamedMicrosoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace
toMicrosoft.VisualStudioCode.RazorExtension
and removedMicrosoft.AspNetCore.Razor.ProjectEngineHost
and its test project. [1] [2] [3]docs/ProjectsAndLayering.md
: Updated documentation to reflect the new project name.Code updates:
RazorWorkspaceListenerBase.cs
: Replaced_disposed
with_disposeTokenSource.IsCancellationRequested
for better disposal handling and added_tagHelperResolver
and_projectChecksums
fields. [1] [2] [3] [4]Project removals:
Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace
project and its related files. [1] [2] [3]