-
Notifications
You must be signed in to change notification settings - Fork 199
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
Replace IProjectCollectionResolver with ISolutionQueryOperations #10943
Replace IProjectCollectionResolver with ISolutionQueryOperations #10943
Conversation
IProjectCollectionResolver was intended to be passed into services that need to enumerate project snapshots to allow them to be used in co-hosting. Instead of encapsulating the state it operates on, a "context" IDocumentSnapshot was passed into IProjectCollectionResolver.EnumerateProjects(...). In co-hosting, the "context" IDocumentSnapshot would be used to extract the Roslyn Solution, while the language server would ignore the parameter entirely. This change introduces IProjectQueryService to replace IProjectCollectionResolver. In the language server, it wraps IProjectSnapshotManager. In co-hosting, a new instance of the service is created for any service call and wraps the Roslyn Solution snapshot.
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.
Clicking approve, because I have no issue with the code as written, but I think there are some interesting questions it raises.
src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/GoToDefinition/RemoteGoToDefinitionService.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/ProjectQueryServiceFactory.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/RazorBrokeredServiceBase.cs
Outdated
Show resolved
Hide resolved
This is a significant change for co-hosting that redesigns how remote snapshots work. - Introduce RemoteSolutionSnapshot to wrap a solution and a collection of RemoteProjectSnapshots. - Rework RemoteProjectSnapshot to simply wrap a document and a collection of RemoteDocumentSnapshots. - Introduce RemoteSnapshotManager that can retrieve a snapshot for a Solution, Project, or TextDocument. - Enforce that RemoteDocumentSnapshots can *only* be created from razor or cshtml documents. - Reduce to a single CWT that maps Solution -> RemoteSolutionSnapshot. - Add RemoteDocumentContext.ProjectQueryService convenience property.
This has gone through enough churn, that I've clicked refresh on your review. Thanks very much for your feedback! Please take another look when you have time! |
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.
Love it
IProjectCollectionResolver
was intended to be passed into services that need to enumerate project snapshots to allow them to be used in co-hosting. Instead of encapsulating the state, it operates on a "context"IDocumentSnapshot
passed intoIProjectCollectionResolver.EnumerateProjects(...)
. In co-hosting, this "context"IDocumentSnapshot
would be used to extract the Roslyn Solution for project enumeration, while the language server would ignore the parameter entirely.This change introduces
ISolutionQueryOperations
and replacesIProjectCollectionResolver
. A key difference fromIProjectCollectionResolver
is thatISolutionQueryOperations
is not available via DI or the MEF composition. Instead of importing a singleton interface in the language server, anISolutionQueryOperations
can be retrieved by callingIProjectSnapshotManager.GetSolutionQueryOperations()
. In co-hosting, there is a newRemoteSolutionSnapshot
class that wraps a Roslyn Solution and implementsISolutionQueryOperations
. This ensures that co-hosting endpoints that don't have immediate access to the target document (such ascompletionitem/resolve
) can still walk projects.