Account for server request ID strings #906
Conversation
Request IDs can be strings or integers. We, the text editor, always use integers for request IDs. But some servers may send out e.g. GUIDs as request IDs. I decided to type-erase the request ID in various places, because all we're doing in most functions is passing through the received request ID from the server all the way to our Response object, finally serializing it back to JSON again.
|
Thanks, I tried this branch and now it seems like LSP responds with the correct request ID. However, the server immediately crashes after that. Here is what I get in the output panel (the server is implemented in Julia): I'm not that familiar with the protocol specification, do you believe this line is a valid response from LSP? |
|
From a look into the language server's code I found that it expects an array of one integer as first value and then 20 boolean values for that workspace/configuration request. Unfortunately the server's documentation is very sparse and doesn't include any infos about that. However, it will use fallback values if the array's entries are null values. It seems like LSP responds with an array, which contains repeatedly the whole "settings" object from the LSP.sublime-settings for the server. Is that intentional? Since I had no "settings" entry in my configuration, the respond consisted of empty dicts, which causes the error. When I set and the server does not crash anymore. But right the next log entry then is and the server is still unresponsive after that. I think the workspace/configuration implementation in LSP should not respond with Maybe related: #699 |
|
I think the code here: Line 282 in e39baf1 should probably be: items.append(self.config.settings or None) |
|
You guys are correct, we should set
But I consider this a separate issue, let's make another pull request for that. |
|
I don't think that Edit: this works for me: if requested_item['section'] in self.config.settings:
items.append(self.config.settings[requested_item['section']])
else:
items.append(None)Should I make a pull request for it? After that, I still get an This PR here looks good from my side and is working for me so far. I just have some suggestions for the ServerLog.sublime-syntax. As GitHub allows me only to comment on the changed lines, I paste the whole file here for simplicity: %YAML 1.2
---
# [Subl]: https://www.sublimetext.com/docs/3/syntax.html
# [LSP]: https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md
hidden: true
scope: output.lsp.log
variables:
method: '[[:alpha:]][[:alnum:]/]*'
servername: '[[:alnum:]_-]+'
uuid: '\h{8}\-\h{4}\-\h{4}\-\h{4}\-\h{12}'
contexts:
main:
- match: ^({{servername}})(:)
captures:
1: variable.function.lsp
2: punctuation.separator.lsp
push:
- meta_scope: meta.block.lsp
- match: $
pop: true
- match: '^::'
scope: punctuation.accessor.lsp
push:
- - meta_scope: meta.group.lsp
- match: $
pop: true
# responses
- - match: '{{uuid}}'
scope: constant.numeric.uuid.lsp
set:
- match: ':'
scope: punctuation.separator.lsp
set: maybe-payload
- match: ''
pop: true
- match: \d+
scope: constant.numeric.integer.decimal.lsp
set:
- match: ':'
scope: punctuation.separator.lsp
set: maybe-payload
- match: ''
pop: true
# notifications or requests
- match: (?=\w)
set:
# requests
- - match: \(
scope: punctuation.section.parens.begin.lsp
set:
- match: \)
scope: punctuation.section.parens.end.lsp
set:
- match: ':'
scope: punctuation.separator.lsp
set: maybe-payload
- match: ''
pop: true
- match: '{{uuid}}'
scope: constant.numeric.uuid.lsp
- match: \d+
scope: constant.numeric.integer.decimal.lsp
# notifications
- match: ':'
scope: punctuation.separator.lsp
set: maybe-payload
- match: ''
pop: true
- - match: '{{method}}'
scope: keyword.control.lsp
pop: true
# language server name
- - match: \S+
scope: variable.function.lsp
pop: true
# arrows
- - match: -->
scope: storage.modifier.lsp
pop: true
- match: <--
scope: storage.modifier.lsp
pop: true
- match: ==>
scope: storage.modifier.lsp
pop: true
- match: unhandled
scope: invalid.deprecated.lsp
pop: true
maybe-payload:
- match: \s*(?=\S)
set:
- match: $
pop: true
- include: scope:source.python#constants # e.g. shutdown request
- include: scope:source.python#lists
- include: scope:source.python#dictionaries-and-sets
- match: ''
pop: true
... |
I needed to disambiguate between responses and notifications, because request IDs can be any string! Solved this by using separate arrow symbols for notifications/responses/requests. This way, we can disambiguate based on the arrow symbol.
|
Thanks for the heads up about the syntax highlighting. I decided to disambiguate based on the arrow symbol. It generally looks like this now
|
Go for it! :)
We should ideally respond with a MethodNotFound error code, but I don't know how various servers in the wild would react to that response. |
Request IDs can be strings or integers.
We, the text editor, always use integers for request IDs.
But some servers may send out e.g. GUIDs as request IDs.
I decided to type-erase the request ID in various places,
because all we're doing in most functions is passing through
the received request ID from the server all the way to our
Response object, finally serializing it back to JSON again.
close #905
@jwortmann, mind giving this branch a try?