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

Move completion resolve code to common layer #11156

Merged
merged 4 commits into from
Nov 11, 2024

Conversation

alexgav
Copy link
Contributor

@alexgav alexgav commented Nov 5, 2024

Summary of the changes

  • Move completion resolve code that can be used from cohosting to the Workspaces layer
  • This is the first step of cohosting completion resolve implementation

Fixes: #11157

Partial fix for #11101 (completing this shouldn't resolve the item)

@alexgav alexgav requested a review from a team as a code owner November 5, 2024 18:30
davidwengier
davidwengier previously approved these changes Nov 5, 2024
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.

Please don't move IProjectSnapshotManagerExtensions and MiscFilesHostProject to Workspaces. These can only be used in the Razor language server and moving them down would be a layering violation. completionitem/resolve can't take a dependency on an IProjectSnapshotManager, since that isn't available in co-hosting. Instead, code shared between the language server and co-hosting layers that need to access project information need to take an ISolutionQueryOperations. None of the code moved here will work in co-hosting without removing dependencies on IProjectSnapshotManager.

@alexgav
Copy link
Contributor Author

alexgav commented Nov 5, 2024

IProjectSnapshotManager

Just so I understand - why do we have IProjectSnapshotManager in the Workspaces layer if it can only be used in the language server layer? Or is it only the misc project and extension methods that cannot be used outside of the language server layer?

@DustinCampbell
Copy link
Member

IProjectSnapshotManager is used in the language server and Visual Studio layers. That's why it's in Workspaces. However, it isn't available in co-hosting. So, you'll need to break dependencies on IProjectSnapshotManager in any code you want to move from the language server to Workspaces and share with co-hosting. I had to do exactly the same thing for hover.

Taking a quick look, I think that what you probably need to do is the following:

  1. Remove IProjectSnapshotManager from RazorCompletionItemResolver.
  2. Update CompletionItemResolver.ResolverAsync(...) to take an ISolutionQueryOperations that RazorCompletionItemResovler's implementation can pass into ClassifiedTagHelperTooltipFactory and MarkupTagHelperTooltipFactory.

The extension methods being moved down here are for retrieving an ISolutionQueryOperations in the language server only. So, they should really stay in the language server. In co-hosting, RemoteSolutionSnapshot implements ISolutionQueryOperations, so it's easy to get ahold of. Here's an example of Go to Definition passing an ISolutionQueryOperations in co-hosting.

@davidwengier davidwengier dismissed their stale review November 5, 2024 22:06

Didn't notice that MiscFileProject was moved. That is definitely the wrong thing to do.

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.

This looks much better. Thank you!

@alexgav alexgav merged commit aeebb01 into main Nov 11, 2024
12 checks passed
@alexgav alexgav deleted the dev/alexgav/MoveCompletionResolveToWokspaces branch November 11, 2024 22:37
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Nov 11, 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.

Move completion resolve code to common layer
3 participants