Add API for custom LSP requests #2549
Conversation
Implement a function `ale#lsp_linter#SendRequest` that allows to send custom LSP requests to an enabled LSP linter. Resolves dense-analysis#2474
There was a problem hiding this comment.
You might need to use the existing StartLSP function to accomplish most of what I mentioned here. That will handle the executable, address, project root, connection ID calculation, setting up the callback, etc.
When setting up a callback for custom requests, make sure to use a Funcref like function('s:HandleCustomRequestResponse'), so only one function is stored in the array of callbacks, which are de-duplicated with Funcref equality checks.
autoload/ale/lsp_linter.vim
Outdated
| if l:linter.lsp is# 'socket' | ||
| let l:executable_or_address = ale#linter#GetAddress(a:buffer, l:linter) | ||
| else | ||
| let l:executable_or_address = ale#linter#GetExecutable(a:buffer, l:linter) |
There was a problem hiding this comment.
The results of these could be deferred objects, which are Dictionary values for computing the executable or address based on some other command, etc. We'll have to handle those here.
autoload/ale/lsp_linter.vim
Outdated
| endif | ||
|
|
||
| let l:root = ale#util#GetFunction(l:linter.project_root)(a:buffer) | ||
| let l:conn_id = l:executable_or_address . ':' . l:root |
There was a problem hiding this comment.
The connection ID should be computed somewhere in lsp.vim and returned here instead. That way this code will continue to work if the format changes.
autoload/ale/lsp_linter.vim
Outdated
| let l:executable_or_address = ale#linter#GetExecutable(a:buffer, l:linter) | ||
| endif | ||
|
|
||
| let l:root = ale#util#GetFunction(l:linter.project_root)(a:buffer) |
There was a problem hiding this comment.
The project root could just be a string, so handle that like we do elsewhere.
There was a problem hiding this comment.
I didn't realise that, thanks for pointing it out! Removed!
autoload/ale/lsp.vim
Outdated
| for l:Callback in l:conn.callback_list | ||
| call ale#util#GetFunction(l:Callback)(a:conn_id, l:response) | ||
| endfor | ||
| endif |
There was a problem hiding this comment.
In order to avoid making this part of the code slower, let's keep this code how it is, and set up a callback which handles the message with the custom callbacks dictionary in the lsp_linter.vim file instead. That way we won't have to check for custom callbacks on every response when you never send custom requests.
There was a problem hiding this comment.
Done, I added a callback.
|
Thanks for the review comments! I didn't realise I could re-use the logic of |
autoload/ale/lsp_linter.vim
Outdated
| throw 'Linter "' . a:linter_name . '" does not support LSP!' | ||
| endif | ||
|
|
||
| let l:message = [0, a:method, a:parameters] |
There was a problem hiding this comment.
I just realised that we'll need one more parameter for the value that is 0 here. That value is 1 when sending notifications, and JSON-RPC spec requires that notifications are sent without the id parameter used for other messages.
Maybe we could replace method, parameters, above with message, and document the format of the messages as [is_notification, method, parameters] in the help text. That will match the format returned by functions in autoload/ale/lsp/message.vim, some of which we might decide to expose later.
There was a problem hiding this comment.
We should make the callback optional for notifications, but required for messages that aren't notifications, as it shouldn't be stored in a Dictionary and won't be used. No responses are sent for notifications.
There was a problem hiding this comment.
Good point, I did not think of this use case. That also requires to make the handler argument optional, since no response is expected when a notification is sent.
Edit: I missed your last message. I was thinking exactly the same though.
Allow to send custom notification mesages, that expect no response from the server.
| call l:Callback(a:response) | ||
| \&& has_key(s:custom_handlers_map, a:response.id) | ||
| let l:Handler = remove(s:custom_handlers_map, a:response.id) | ||
| call l:Handler(a:response) |
There was a problem hiding this comment.
I thought using Handler here instead of Callback may help to make the code a bit more readable, since there is another layer of callbacks above it that are already called Callbacks.
|
Cheers! 🍻 |
Implement a function
ale#lsp_linter#SendRequestthat allows to sendcustom LSP requests to an enabled LSP linter.
Resolves #2474