Entirely use lsprotocol types for serialization and deserialization. #1
Conversation
This file contains 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
The `lsprotocol.types` module is re-exported through the `pygls.lsp.types` module hopefully minimsing the number of broken imports. This also drops pygls' `LSP_METHODS_MAP` in favour of the `METHOD_TO_TYPES` map provided by `lsprotocol`. The `get_method_xxx_type` functions have been adjusted to use the new mapping. As far as I can tell `lsprotocol` doesn't provide generic JSON RPC message types (except for `ResponseErrorMessage`) so the old `JsonRPCNotification`, `JsonRPCResponseMessage` and `JsonRPCRequestMessage` types have been preserved and converted to `attrs`.
The machine readable version of the LSP spec (and therefore `lsprotocol`) provides a mapping from an LSP method's name to its `RegistrationOptions` type, which is an extension of the method's `Options` type used when computing a server's capabilities. This means the `RegistrationOptions` type includes additional fields that are not valid within the `ServerCapabilities` response. This commit introduces a new `get_method_options_type` function that returns the correct `Options` type for a given method, automatically deriving the type name from the result of the existing `get_method_registration_options_type` function when appropriate.
There seems to be a mismatch with how pygls and lsprotocol want to work. pygls says to the user "give me you params/result/error fields and I'll put them in the correct JSON RPC structure for you. This meant that pygls' `LSP_METHODS_MAP` returned types representing just the params/result fields of messages. However, lsprotocol's `METHOD_TO_TYPES` map is set up to return types representing the full JSON RPC message bodies which seems to complicate the serialization/deserialization code a bit. I think I've managed to work around the differences, but this could probably be done in a nicer way.
The timeouts can get in the way when trying to debug the code under test. This commit makes it possible to disable the timeout by running the testsuite with the `DISABLE_TIMEOUT` environment variable set e.g. $ DISABLE_TIMEOUT=1 pytest -x tests/
Nothing too interesting in this one, just updating imports, class names etc to align `pygls` to the definitions in `lsprotocol`
8 tasks
|
Thanks for this! Is this PR something you're looking to get merged, or is it meant more as a guide for the next round of refinements? (I don't mind either way, just figuring out what I need to do next to keep things moving) These Finally, I assume I'd need the next version of |
|
This was just for reference, not for merging. The |
|
published a new release of |
alcarney
force-pushed
the
lsprotocol
branch
3 times, most recently
from
58adfd5 to
b7373e7
Compare
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.
This is a draft, and just test
lsprotocolitself and see how it fairs whenpyglsfully relies on it.Switched the serialization/de-serialization of request, response and notification to use lsprotocol.
I updated the tests as well. skiped some of the protocol tests in
test_protocol.pysince they relied onJsonRPC*classes which i removed. and skiped 3 command tests, they were flaky.