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

Make unsupported LSP responses better #3645

Open
mattrunyon opened this issue Mar 31, 2023 · 0 comments
Open

Make unsupported LSP responses better #3645

mattrunyon opened this issue Mar 31, 2023 · 0 comments

Comments

@mattrunyon
Copy link
Contributor

mattrunyon commented Mar 31, 2023

Currently, if there is an unsupported LSP type (like get_signatures from Groovy), we return a successful response with a default object for the request type. Instead, we should either return a NOT_SUPPORTED response or not set a response type to indicate not supported.

This will require some minor improvements to the web UI code that creates these requests. Currently, the web UI looks at the JS API to determine if the server should support a certain response type (i.e. does the getSignatures function exist on the API object). Since the API is agnostic of the worker language, the request may not actually be supported by that worker language. In order to not throw web client errors, we currently return the default value so the web client thinks there was just no autocomplete help for that request.

The better option for the web UI would be to know that the request failed because it was not supported and remove the provider for that request type since all future requests will also result in a not supported response.

Original comment from #3607

It looks like the pattern here is to return success if the server knows about the type, but doesn't implement it. That said, I wonder if it's better to not set any response on the GET_SIGNATURE_HELP / GET_HOVER / GET_DIAGNOSTIC cases.

The way the implementation works now, for example, on a GET_DIAGNOSTIC request, the client needs to know that an empty kind field means it's not implemented?

message GetPullDiagnosticResponse {
    string kind = 1;
    optional string result_id = 2;
    repeated Diagnostic items = 3;
}

Note: as the server is previously implemented, and implemented now, a newer client won't be able to talk to an older server (REQUEST_NOT_SET will throw an exception, which seems acceptable as long as the client knows how to properly handle it).

Originally posted by @devinrsmith in #3607 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants