Skip to content

consume new ea #11723

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Apr 10, 2025
Merged

consume new ea #11723

merged 12 commits into from
Apr 10, 2025

Conversation

ryzngard
Copy link
Contributor

@ryzngard ryzngard commented Apr 8, 2025

@ryzngard ryzngard requested review from a team as code owners April 8, 2025 22:51
DustinCampbell
DustinCampbell approved these changes Apr 9, 2025
View reviewed changes
Copy link
Member

@DustinCampbell DustinCampbell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Do we expect an image load regression with this change? If so, have we reached out for a permanent exception?

@ryzngard
Copy link
Contributor Author

ryzngard commented Apr 9, 2025

Looks good to me. Do we expect an image load regression with this change? If so, have we reached out for a permanent exception?

This will be a regression but we only need a temporary exception. This helps get rid of the EditorFeatures.WPF layer in Roslyn so we're trading 1 load for another

davidwengier
davidwengier approved these changes Apr 10, 2025
View reviewed changes
Copy link
Member

@davidwengier davidwengier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Guessing this is waiting on a Roslyn insertion merge, right? Me too! Saves me updating my PRs at least :D

@ryzngard
Copy link
Contributor Author

Guessing this is waiting on a Roslyn insertion merge, right? Me too! Saves me updating my PRs at least :D

This will be a dual insertion, just had to wait for the EA packages to be published.

@ryzngard ryzngard enabled auto-merge (squash) April 10, 2025 18:18
DustinCampbell and others added 2 commits April 10, 2025 13:34
@DustinCampbell
Fix CSharpTestLspServer
001b584 
The RoslynLanguageServer in the latest Microsoft.CodeAnalysis.LanguageServer.Protocol.dll doesn't exit gracefully when the JsonRpc it's listening to is forcibly disconnected. Unfortunately, that's exactly how our CSharpTestLspServer shuts down the RoslynLanguageServer that it creates. Ultimately, the Razor EA should be updated to provide a IAsyncDisposable implementation that can be called to gracefully shut down the language server. However, in the meantime, this change uses reflection to get at the underlying RoslynLanguageServer and call ShutdownAsync and ExitAsync on it before disposing the server-side JsonRpc.
Merge pull request #3 from DustinCampbell/fix-csharptestlanguageserver
c1c975c 
Fix CSharpTestLspServer disposal to gracefully shutdown RoslynLanguageServer
@ryzngard ryzngard merged commit ac21bbe into dotnet:main Apr 10, 2025
11 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Apr 10, 2025
@ryzngard ryzngard deleted the consume_new_ea branch April 10, 2025 23:16
dibarbet added a commit to dibarbet/razor that referenced this pull request Apr 17, 2025
@dibarbet
Revert "consume new ea (dotnet#11723)"
339a32e 
This reverts commit ac21bbe.
@ryzngard ryzngard mentioned this pull request Apr 18, 2025
Closed
This was referenced Apr 23, 2025
Closed
Closed
Closed
@ryzngard ryzngard mentioned this pull request Apr 28, 2025
Merged
@dotnet-bot dotnet-bot mentioned this pull request May 31, 2025
Closed
@RikkiGibson RikkiGibson removed this from the Next milestone Aug 20, 2025
@RikkiGibson RikkiGibson added this to the 18.0 P1 milestone Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DustinCampbell DustinCampbell DustinCampbell approved these changes

@davidwengier davidwengier davidwengier approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

18.0 P1

Development

Successfully merging this pull request may close these issues.

4 participants

@ryzngard @DustinCampbell @davidwengier @RikkiGibson