Skip to content
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

Allow Razor to get diagnostics for cohosting #75102

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Sep 13, 2024

Roslyn side of dotnet/razor#10696
Razor PR is dotnet/razor#10882

Review commit-at-a-time if you want to see the details of the changes, but most of it is just moving the code that converts from DiagnosticData to LSP Diagnostic into the ProtocolConversions class.

Doing this as a separate commit to prove I didn't break anything :P
Purely mechanical, except for making one method public
These two methods have diverged so I don't honestly know if this is valuable, but updating a comment seems easy enough.
@davidwengier davidwengier requested review from JoeRobich, dibarbet and a team September 13, 2024 01:51
@davidwengier davidwengier requested a review from a team as a code owner September 13, 2024 01:51
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Sep 13, 2024
@davidwengier davidwengier merged commit 876ea36 into dotnet:main Sep 16, 2024
25 checks passed
@davidwengier davidwengier deleted the RazorDiagnostics branch September 16, 2024 20:08
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Sep 16, 2024
davidwengier added a commit to dotnet/razor that referenced this pull request Sep 18, 2024
Fixes #10696
Fixes #10769
Fixes #10874
Needs Roslyn change: dotnet/roslyn#75102

The actual code change was quite straight foward. The test code was a
bigger pain. Previously in cohosting tests we had two MEF composition,
one for Razor and one for Roslyn. Since Roslyn diagnostics can only run,
or at least start, in devenv, we now need three MEF compositions -
Roslyn devenv, Roslyn OOP and razor OOP - and two workspaces - Roslyn
devenv and Roslyn OOP - and our previous MEF infrastructure really
didn't handle this well, as evidenced by the fact that this PR also
fixes #10874

Now our MEF infra is simpler: We still cache catalogs etc. statically
the save time, but we create an `ExportProvider` as needed and expect
consumers to dispose of it like any other `IDisposable`, though there is
also here a test fixture to share the editor composition in the
formatting tests so we don't pay a huge penalty in terms of test run
time.

Commit-at-a-time if you want to read the story from beginning to end,
but it should be reasonably okay to just review as a whole too.
@akhera99 akhera99 modified the milestones: Next, 17.12 P3 Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants