-
Notifications
You must be signed in to change notification settings - Fork 196
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
Conversation
There was a problem hiding this 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
.
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? |
Taking a quick look, I think that what you probably need to do is the following:
The extension methods being moved down here are for retrieving an |
Didn't notice that MiscFileProject was moved. That is definitely the wrong thing to do.
…ct per PR feedback
src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Tooltip/ProjectAvailabilityTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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!
Summary of the changes
Fixes: #11157
Partial fix for #11101 (completing this shouldn't resolve the item)