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

Add co-hosting support for hover #11150

Merged
merged 13 commits into from
Nov 6, 2024

Conversation

DustinCampbell
Copy link
Member

@DustinCampbell DustinCampbell commented Nov 5, 2024

Fixes #10839

Now that all of the infrastructure is in place, adding a co-hosting endpoint and remote service for Hover is mostly boilerplate. Similar to the signature help endpoint, the result type might be a Roslyn LSP Hover or a VS LSP Hover. The remote service always returns a Roslyn LSP Hover, but HTML will return a VS LSP Hover. So, we join the possibilities together with a SumType.

@DustinCampbell DustinCampbell requested a review from a team as a code owner November 5, 2024 01:11
Copy link
Contributor

@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.

Nifty raw content validation

Rework the logic in RemoteHoverService. It isn't necessary to try and map the host document index to C#, since `GetPositionInfo(..., preferCSharpOverHtml: true)`already does that for us.
@davidwengier
Copy link
Contributor

FYI I merged #11151 so you'll want to merge main, as IDynamicRegistrationProvider has changed. Sorry!

@DustinCampbell
Copy link
Member Author

FYI I merged #11151 so you'll want to merge main, as IDynamicRegistrationProvider has changed. Sorry!

No worries! Thanks for the heads up! I'll get it fixed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Port Hover endpoint to cohosting
3 participants