-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
WIP: Expose Roslyn OOP as a limited LSP server supporting code search via extended LSP #37359
WIP: Expose Roslyn OOP as a limited LSP server supporting code search via extended LSP #37359
Conversation
…o dev/olegtk/CodeSearchProto
…o dev/olegtk/CodeSearchProto
@@ -10,45 +10,59 @@ | |||
using Microsoft.CodeAnalysis.Remote; | |||
using Microsoft.ServiceHub.Client; | |||
using Microsoft.CodeAnalysis.Editor; | |||
using Microsoft.VisualStudio.Shell.Interop; | |||
using Microsoft.VisualStudio.Shell.Events; | |||
|
|||
namespace Microsoft.VisualStudio.LanguageServices.CSharp.LanguageServerClient | |||
{ | |||
[ContentType(ContentTypeNames.CSharpContentType)] |
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.
Is there any reason not to support VB?
@@ -23,6 +23,9 @@ | |||
<ProjectReference Include="..\Core\Microsoft.CodeAnalysis.Remote.Workspaces.csproj" /> | |||
</ItemGroup> | |||
<ItemGroup> | |||
<Reference Include="Microsoft.VisualStudio.LanguageServer.Protocol.Extensions"> | |||
<HintPath>C:\Program Files (x86)\Microsoft Visual Studio\2019\master\Common7\IDE\CommonExtensions\Microsoft\LanguageServer\Microsoft.VisualStudio.LanguageServer.Protocol.Extensions.dll</HintPath> |
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.
Remove before check in
/// <summary> | ||
/// Gets the name of the language client (displayed to the user). | ||
/// </summary> | ||
public string Name { get; } = "C# Language Server Client"; |
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.
loc'ed?
@@ -0,0 +1,34 @@ | |||
using System; |
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.
copyrights on all these files.
return new VSBeginSymbolParams(); | ||
} | ||
|
||
private async Task SearchAsync(string query, int searchId, CancellationToken cancellationToken) |
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.
a lot of this code feels very duplicative.
return; | ||
} | ||
|
||
VSSymbolInformation[] convertedResults = await Convert(result, cancellationToken).ConfigureAwait(false); |
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.
var
@@ -269,7 +269,7 @@ | |||
<NuGetPackageToIncludeInVsix Include="System.Composition.Hosting" /> | |||
<NuGetPackageToIncludeInVsix Include="ICSharpCode.Decompiler" /> | |||
<NuGetPackageToIncludeInVsix Include="Microsoft.VisualStudio.LanguageServer.Protocol" /> | |||
<NugetPackageToIncludeInVsix Include="Microsoft.VisualStudio.LanguageServer.Protocol.Extensions" /> | |||
<!--<NugetPackageToIncludeInVsix Include="Microsoft.VisualStudio.LanguageServer.Protocol.Extensions" /> --> |
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.
Replace with new nuget
|
||
public async Task<string> GetHostGroupIdAsync(CancellationToken cancellationToken) | ||
{ | ||
var client = await _primaryWorkspace.Workspace.TryGetRemoteHostClientAsync(cancellationToken).ConfigureAwait(false); |
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.
@heejaechang I'm seeing a problem here: if the user attempts to search during solution load of a large solution, the remote host client isn't setup yet. What's the preferred pattern here? Can we request to start the client or await its startup?
Polish for @olegtk's PR #37190... I don't have permissions to push directly to the existing PR.
This prototype exposes RoslynCodeAnalysis service hub service as a limited LSP server supporting only extended (streaming) version of LSP's workspace symbol search. This allows Roslyn to support VS search on par with GoTo.
More context: VS Search (former Quick Launch) public extensibility is LSP only, it supports both standard workspace/symbol and extended workspace/beginSymbol and workspace/publishSymbol (not yet documented). Standard LSP's workspace/symbol doesn't support streaming of results yet (expected in next version via microsoft/vs-streamjsonrpc#139), that's why until it does we need to extend LSP with workspace/beginSymbol.
This implementation is reusing AbstractNavigateToSearchServic, just like GoTo, but unlike GoTo it uses PrimaryWorkspace as LSP doesn't support workspace synchronization.
Initialization of this Roslyn LSP server is left to VS LSP client for now, it will initialize it when user starts interacting with VS Search (based on "Capability" metadata).