Skip to content
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

jsonrpc: Skip serializing params if params are None #5471

Merged

Conversation

the-mikedavis
Copy link
Member

@the-mikedavis the-mikedavis commented Jan 9, 2023

The JSONRPC spec says:

If present, parameters for the rpc call MUST be provided as a Structured value

https://www.jsonrpc.org/specification#parameter_structures

(Where a "Structured value" is elsewhere defined as either a map or array.)

This change skips the serialization of the params field for JSONRPC method calls and notifications if the params field is the None variant. This fixes compatibility with LSP servers which adhere closely to that part of the spec: ocamllsp in the wild.

Fixes #5400

The JSONRPC spec says:

> If present, parameters for the rpc call MUST be provided as a
> Structured value

https://www.jsonrpc.org/specification#parameter_structures

(Where a "Structured value" is elsewhere defined as either a map or
array.)

This change skips the serialization of the `params` field for JSONRPC
method calls and notifications if the `params` field is the `None`
variant. This fixes compatibility with LSP servers which adhere closely
to that part of the spec: `ocamllsp` in the wild.
@the-mikedavis the-mikedavis added C-bug Category: This is a bug A-language-server Area: Language server client S-waiting-on-review Status: Awaiting review from a maintainer. labels Jan 9, 2023
@the-mikedavis
Copy link
Member Author

Looks like this was noticed on the upstream we vendored from as well: paritytech/jsonrpc#660

@archseer archseer merged commit 2229843 into helix-editor:master Jan 10, 2023
@the-mikedavis the-mikedavis deleted the md-jsonrpc-skip-params-if-is-none branch January 10, 2023 12:33
kirawi pushed a commit to kirawi/helix that referenced this pull request Jan 25, 2023
The JSONRPC spec says:

> If present, parameters for the rpc call MUST be provided as a
> Structured value

https://www.jsonrpc.org/specification#parameter_structures

(Where a "Structured value" is elsewhere defined as either a map or
array.)

This change skips the serialization of the `params` field for JSONRPC
method calls and notifications if the `params` field is the `None`
variant. This fixes compatibility with LSP servers which adhere closely
to that part of the spec: `ocamllsp` in the wild.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client C-bug Category: This is a bug S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quitting Helix when ocamllsp was active causes delay and exits with an error.
2 participants