-
Notifications
You must be signed in to change notification settings - Fork 817
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
Progress support in LSP #786
Comments
this looks awesome! couple comments:
|
Per our discussion regarding when errors occur during streaming progress, the decision is to keep current set of results reported thus far, but report an error/warning notification to user indicating something went wrong and results may be incomplete. |
Our C/C++ extension Find All References sends back references with types like "Possible Reference (confirmation in progress)" and we later update it to "Not a Reference", "Confirmed Reference", "Possible Reference (confirmation cancelled)", "Cannot confirm reference, etc.-- the API described here doesn't appear to allow partial results that overwrite all the existing results instead of just results that are accumulated. Could you add a progress support that supports this, such as via an "overwriteResult" progress type? We're (currently) okay with overwriting all the results and not just a subset of results that are potentially changeable and overwritable with better results later, but it's possible there could be future scenarios where this is not good, if there are too many results to overwrite -- I guess the "partialResult" could be used for stuff that is not overwritable and the "overwriteResult" could form at different set. There is another LSP request for "typed references" to match VS. We currently implement our own "streaming" via causing a 2nd textDocument/references request that overwrites the previous results (when we want to update the results), but there are bugs, because this type of "hack" wasn't intentionally designed for, e.g. it breaks if the cursor is moved off an identifier (because it canceled out too early without sending us the LSP message), and also any popup menus get closed when the final results come in (caused by executing the references-view.find command). |
Ah, the details of a streaming textDocument/references were already discussed in https://github.com/microsoft/ms-lsp/issues/10 . |
Closing the issue. Corresponding support has been speced in dbaeumer/3.15 branch. |
There are two distinct requests in LSP that ask for progress:
The corresponding items are:
#70
#110
#182
There has been a long discussion in microsoft/vs-streamjsonrpc#139 on how to best do this in a homogeneous why in JSON-RPC and I propose that we implement the outcome of this issue in LSP as well.
The basic idea is as follows:
$/progress
notification and the provided token to report progress.LSP should standardize the token property names used to indicate the progress types:
workDone
: to indicate that the client accepts work done progress.partialResult
: to indicate that the client accepts partial result for a request.Work Done
The value of the work done progress notification would be the same as for the
window/progress/*
notifications which support progress initiated from the server.If the server supports progress it is supposed to send a first notification indicating the start.
followed by many progress report messages:
an an optional done(end is implicit when the request sends a response):
I would also adjust the proposed server initiated progress accordingly (e.g. having one one notification route with and an additional
kind
property). See https://github.com/Microsoft/vscode-languageserver-node/blob/master/protocol/src/protocol.progress.proposed.ts#L1Partial Result
The
value
property of the partial result depends on the kind of request and needs to be specified on a request to request basis.For example for the
workspace/symbol
request the value of a partial progress notification would beSymbolInformation[]
.If a client accepts partial result for a request and the server sends partial result the final response must not carry any additional result data. The value should either be
null
or an empty data structure (e.g. empty array). The final response can of course still contain error information.The text was updated successfully, but these errors were encountered: