Skip to content

Add ocaml-language-server for OCaml and ReasonML#1110

Merged
w0rp merged 2 commits into
dense-analysis:masterfrom
jungomi:ocaml-language-server
Nov 11, 2017
Merged

Add ocaml-language-server for OCaml and ReasonML#1110
w0rp merged 2 commits into
dense-analysis:masterfrom
jungomi:ocaml-language-server

Conversation

@jungomi

@jungomi jungomi commented Nov 11, 2017

Copy link
Copy Markdown
Contributor

ocaml-language-server provides editor support for OCaml and Reason using the Language Server Protocol.

I had to remove the id field on the request object, because technically a notification must not have an id, which is different from id: null (present, but null).

From JSON-RPC 2.0 Specification - Notification

4.1 Notification
A Notification is a Request object without an "id" member. [...]

And JSON-RPC 2.0 Specification - Request object:

id
An identifier established by the Client that MUST contain a String, Number, or NULL value if included. If it is not included it is assumed to be a notification. The value SHOULD normally not be Null [1] and Numbers SHOULD NOT contain fractional parts [2]

[...]

[1] The use of Null as a value for the id member in a Request object is discouraged, because this specification uses a value of Null for Responses with an unknown id. Also, because JSON-RPC 1.0 uses an id value of Null for Notifications this could cause confusion in handling.

ocaml-language-server is built with vscode-languageserver, which is very strict about this and does not accept notifications with an id, even if it's null.

After removing 'id': v:null, Vim reorders the fields to {"method": "someMethod", "jsonrpc": "2.0"} for whatever reason, so I swapped them in the source to avoid confusion. The order only affects the tests.

For the record:

echo {'id': v:null, 'jsonrpc': '2.0', 'method': 'someMethod'}
" => {'id': v:null, 'jsonrpc': '2.0', 'method': 'someMethod'}

echo {'jsonrpc': '2.0', 'method': 'someMethod'}
" => {'method': 'someMethod', 'jsonrpc': '2.0'}

echo {'jsonrpc': '2.0', 'method': 'someMethod', 'id': v:null}
" => {'method': 'someMethod', 'jsonrpc': '2.0', 'id': v:null}

@w0rp w0rp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I'll merge this for the JSONRPC changes.

Could you create a follow up pull request with tests for the new linter functions? That's the best way to protect against regressions.

@w0rp w0rp merged commit b789b9e into dense-analysis:master Nov 11, 2017
@w0rp

w0rp commented Nov 11, 2017

Copy link
Copy Markdown
Member

Cheers! 🍻

@jungomi jungomi deleted the ocaml-language-server branch November 12, 2017 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants