Add only LSP server projects without liveshare dependencies. #36249
Conversation
|
Val build is good |
src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/TestUtilities/LanguageServer/AbstractLanguageServerProtocolTests.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/ProtocolUnitTests/SignatureHelp/SignatureHelpTests.cs
Outdated
Show resolved
Hide resolved
heejaechang
left a comment
heejaechang
left a comment
There was a problem hiding this comment.
sorry I thought I already signed off. code looks like same as what I saw before except live share handler part is split out
|
@tmat did the changes to compare the JSON, looks better. Anything else you'd like me to look at? |
src/Features/LanguageServer/Protocol/Microsoft.CodeAnalysis.LanguageServer.Protocol.csproj
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/FoldingRanges/FoldingRangesHandler.cs
Outdated
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/Highlights/DocumentHighlightHandler.cs
Outdated
Show resolved
Hide resolved
| [ExportLspMethod(Methods.TextDocumentDocumentHighlightName)] | ||
| internal class DocumentHighlightsHandler : IRequestHandler<TextDocumentPositionParams, DocumentHighlight[]> | ||
| { | ||
| public async Task<DocumentHighlight[]> HandleRequestAsync(Solution solution, TextDocumentPositionParams request, |
There was a problem hiding this comment.
DocumentHighlight[] [](start = 26, length = 19)
Any reason not to use ImmutableArray?
There was a problem hiding this comment.
LSP package defines the types as array - https://devdiv.visualstudio.com/DevDiv/_git/VSExtensibility?path=%2Fsrc%2Fproduct%2FcommonLanguageServer%2FRemoteLanguage.Protocol%2FMethods.cs&version=GBdevelop&line=285&lineStyle=plain&lineEnd=286&lineStartColumn=1&lineEndColumn=1 I guess we would ask them why not immutable array.
Since liveshare is using the LSP package as well, it depends on them being the same type.
There was a problem hiding this comment.
That seems unfortunate. Any idea why the package is not using ImmutableArray? @heejaechang
There was a problem hiding this comment.
@tinaschrepfer might be able to offer some insight
src/Features/LanguageServer/Protocol/Handler/Formatting/FormatDocumentHandlerBase.cs
Show resolved
Hide resolved
src/Features/LanguageServer/Protocol/Handler/Symbols/DocumentSymbolsHandler.cs
Show resolved
Hide resolved
|
@tmat am I good to merge this one? |
|
@jinujoseph would you like me to attempt to squeeze this into 16.3p1, or wait until tomorrow and take it for 16.3p2? |
|
|
5 participants
Editor team wishes to start testing out LSP stuff with Roslyn. So push this to master so we can start work on that. Also will help make the overall insertion easier, won't have to handle both the new dependencies and liveshare switch in the same insertion.
The implementations are copied from liveshare. The additional work I did was adding tests and setting up the projects.
This only includes the pieces that don't have a UI dependency / other liveshare dependency. Those will be moved later on.
This will need cleanup - tracked in https://github.com/dotnet/roslyn/projects/45
TODO - Run val build