-
Notifications
You must be signed in to change notification settings - Fork 786
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
Support that server can opt-out of requests on non open document #1912
Comments
Actually a server should be able to full fill requests without a didOpen notification. Otherwise you could not ask questions about documents that are not open. The LSP spec says:
DidOpen / didClose transfers the ownership of the document's content between server and client. It purpose is not to allow requests. I do agree that a better name could have been chosen :-). Hope that helps. |
Thanks @dbaeumer for your answer. The main problem is that all language server that I have seen (and I have implemented) consider that a feature like textDocument/codeLens requires the fill of internal document filled with didOpen. In Rust language server for instance, they throw an error that document is not opened when textDocument/codeLens is called. Is it a wrong implementation? The behavior between vscode and IntelliJ with the same language server are different. vscode seems doing in any case didOpen before codelens, so there is not the problem. In my case with IntelliJ the didOpen can occur after codeLens and I have an error with the same language server and same context file in vscode. So I have the impression that vscode ensures that didOpen is every time done before codelens. |
The reason for this is that codeLens requests in VS Code are only sent for documents that are present in the editor. Having them visible in the editor results in an didOpen notification since the ownership of the content of the document move to the client. However VS Code also offers API to trigger |
Ok thanks for the clarification. My initial question was about LSP client implementation (and not LSP language server implementation) and I think the question can be splitted in 2 questions. Language Server & didOpenAt first I have never seen some language server implementation which takes care of call of features (codelens, hover, completion, etc) without calling didOpen. Takes a sample with CSS language server with completion https://github.com/microsoft/vscode/blob/dc29cb5afd8dcdb7bfbad15f12fbc0443958ed74/extensions/css-language-features/server/src/cssServer.ts#L198 //cc @aeschli The following code assumes that a didOpen has occured (the https://github.com/microsoft/vscode/blob/dc29cb5afd8dcdb7bfbad15f12fbc0443958ed74/extensions/css-language-features/server/src/cssServer.ts#L43 fill the documents map when a didopen occurs). Is it a wrong implementation? If yes, if I understand correctly your answer it means that the code should be updated to check if documents has been opened (documents map is filled) and otherwise language server should open the content of the file himself? LSP client with editor & didOpenI think LSP client implementation editor should take care of calling didOpen before other features, otherwise the if language server opens himself the file it could be unsynchronized with the editor content. Hope I'm not wrong and I have understood correctly your explanation. |
Loading resources that are not open is something that could be added to the CSS language server, but I think it's also valid if a server decides not to handle certain URIs. Reading a resource is not trivial. For file URI's the tricky part s loading the file with the current encoding and for other URI schemes all depends on the scheme. |
I was also thinking that if the server should be able to fullfill requests for non-open files then how would it know which I would prefer if the spec said something to the effect of "the server might be able to fullfill requests for files that are not opened by the client". |
@dbaeumer why the issue has been closed? I think LSP spec should be improved to explain the correct behavior for:
I ask you because I have seen 2. and 3. behavior in different LS, so I think teh behavior should be more clarified. Thanks! |
Sorry, that someone slipped through the cracks. I do see @aeschli point that for language servers that don't operate on a file system it is hard to fullfil these requests. Instead of returning nothing it would make sense to allow them to provide an error that the client can handle appropriately This being said we shouldn't ask client to open file automatically since an open is an ownership transfer and usually triggers a lot of other events. |
Clangd is another example of a language server which does not currently support requests on files which haven't been opened with In clangd's case, I believe the reason is performance related: building the initial AST for a file is slow (often several seconds) but incremental updates are fast, and if a file is not open (client is not sending |
@HighCommander4 IMO doing a slow AST build is fine. All requests are async and a client should usually not be blocked by this. |
Perhaps it is a stupid question, but I have not seen some docmentation about that.
I'm implementing an LSP support for IntelliJ https://github.com/eclipse-lsp4j/lsp4j it starts working well but sometimes my textDocument/* (ex : textDocument/codeLens, textDocument/documentLink) is done before the didOpen.
This usecase comes from only when the language server starts, and takes some times.
It is the reason why I ask the following question : Is didOpen must be done before textDocument/* ?
Perhaps it is natural in vscode, but in my case, the didOpen is done in a Thread (to avoid blocking the IDE) and the call of textDocument/codeLens is done by the implementation of IJ CodeVision where I don't master the call.
I suppose that I need to implement something to consume textDocument/codeLens before the didOpen, but before doing that I would like to be sure that the it is the proper behavior.
If it is the proper behavior, is LSP specification didOpen could mention that didOpen must be done before all textDocument/*? As didOpen is a notification (there is no response) how the LSP client can be sure that didOpen is finished on language server side?
Many thanks for your answer.
The text was updated successfully, but these errors were encountered: