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

Completion resolve endpoint skeleton #11226

Closed

Conversation

alexgav
Copy link
Contributor

@alexgav alexgav commented Nov 18, 2024

Summary of the changes

  • Adds skeleton for the cohosting completion resolve request handler
  • Adds TextDocument identifier data to the completion item so that resolve request handler is now callable by Roslyn (Roslyn uses that data to figure out that request should go to Razor)

Fixes:
Partial fix for #11101

- Making the resolve completion use document for cohosting
- Serializing TextDocument property into Data member of completion items so that Roslyn will forward the request to us
- Basic sanity test (shows we are getting called now)
@alexgav alexgav requested a review from a team as a code owner November 18, 2024 23:33
@davidwengier
Copy link
Contributor

This feels like a commit that should be part of a larger PR, not a PR itself. I don't think we should be diverging the normal and cohosting endpoints like this, as that is how bugs creep in. Additionally, I can't see how there won't have to be changes to the completion resolve service in light of this change, and trying to review those changes separately in another PR will just be harder, with a complete lack of context.

@alexgav alexgav closed this Nov 20, 2024
ryzngard added a commit to dotnet/vscode-csharp that referenced this pull request Nov 20, 2024
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.

2 participants