Overhaul part 2/2 (Refactored Language Client, Pipelines and unit tests!) #248
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… handler that uses the new pipelines for lower allocations of incomming data. Added validation tests for handler methods to ensure that all required methods exist when new handlers are added
david-driscoll
force-pushed
the
feature/client-refactor
branch
from
1d5d6c1 to
cdfbbb9
Compare
Codecov Report
@@ Coverage Diff @@
## master #248 +/- ##
===========================================
+ Coverage 33.37% 62.12% +28.75%
===========================================
Files 391 383 -8
Lines 11626 11483 -143
Branches 1459 1361 -98
===========================================
+ Hits 3880 7134 +3254
+ Misses 7420 3782 -3638
- Partials 326 567 +241
Continue to review full report at Codecov.
|
TylerLeonhardt
requested review from
NTaylorMullen,
tintoy,
bjorkstromm and
TylerLeonhardt
…in LspClientReciever which was causing the powershell integration tests to not work correctly.
…pture unhandled exceptions during input processing
tintoy
approved these changes
May 28, 2020
…pogation of cancelled results back to the caller. Also fixed an issue where request cancellation was not working. Also request cancellation would not throw custom exceptions for anyone that was task like.
…e that the JToken is not sent through the serailzier (this ensure that the casing stays correct)
…/parallel) Fixed a bug where things would lock due to response tasks being set in the same thread.
TylerLeonhardt
approved these changes
Jun 1, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
So yeah... this is huge, sorry for that.
This includes the pipeline changes (and revealed a few problems with the
InputHandler)This rewrites the
LanguageClientto be theLanguageServerjust in reverse. Same design but mirrored methods. (ie. The server willPublishDiagosticsand the client will have anIPublishDiagnosticsHandlerto recieve them).As a core part of this change the goal is to ensure consistency between handlers. This has introduced a few things.
JsonRpc
MethodAttributehas a new (optional) enum added to it for theDirectionthis handler, which is just used a hint for a JsonRpcServer. Direction is required for both LSP and DAP protocols however, if it's missing there is a unit test which will fail.LSP/DAP
LSP
IPartialResultRequestorIPartialResultsRequesthave an overload for dealing with partial resultsIProgressManagerIProgressManager( https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#partialResults )MonitorUntilTaskLikeobject that is both anIObservable<T>and{ GetAwaiter(): TaskAwaiter<TResponse> }IServerWorkDoneManagerandIClientWorkDoneManager( https://microsoft.github.io/language-server-protocol/specifications/specification-current/#workDoneProgress )IClientProxyfor sending requests, which will enrich the request with a work done token.IServerWorkDoneManagerthat is safe to call onIWorkDoneProgressParamseven if workdone is not set, it will just be an Observer that goes no whereLSP/DAP Unit Testing
JsonRpcServerSettleto ensure state is consistent on both sidesITestOutputHandler)With the new client we now have the ability to test the library in both directions to help improve code coverage as well as help with developing tests for corner cases (such as servers that don't support dynamic registration for example).