textDocument/documentSymbol: support unopened files #548
Conversation
|
This probably works for The specification does say
But I feel this can clutter server implementations and a client can actually circumvent easily by sending |
|
Right, this may not work so easily for other queries (although I am not sure why it should be impossible or prohibitively difficult to implement them). But I so far do not have a strong usecase for other queries (textDocument/references, could admittedly be useful when invoked through the Navigator, though). Hence this proposed patch only changes textDocument/documentSymbol, and nothing else, I believe. And right again that the clients could workaround by doing didOpen/didClose. But, besides cluttering client implementations (just for the sake of a very limited set of servers), this also brings a question of how many files should be kept virtually open, when to virtually close them, etc. And this all feels like a huge waste of resources - presumably when didOpen is issued, the server will start to compute diagnostics as if the file was really opened, but the only thing that is really requested is the document's structure, which (per my understanding) is available, and does not really need to be computed! Overall, I think the protocol puts some constraints on the clients, and some on the servers. And some of them are difficult to achieve (and this is true for both clients and servers). But I am not convinced that ignoring some statements of the spec is a good way to make the clients and server cooperate for the benefit of the users. All I am trying to do is to fix the problem where I think they are (and there were multiple problems on the client side exposed by ccls, which could be workarounded in ccls - but that would be a wrong place to do that!) |
There was a problem hiding this comment.
For this particular one, it probably does not matter much that this is implemented on the server side. The index-based (as opposed to AST-based which computes the AST first) approach makes such requests easier to implement.
Can you briefly describe the "Navigator" feature (it'd be nice if you have a screenshot)? When is it triggered? Why are other things like textDocument/documentLink not needed?
src/message_handler.cc
Outdated
| @@ -252,9 252,10 @@ QueryFile *MessageHandler::findFile(const std::string &path, int *out_file_id) { | |||
|
|
|||
| std::pair<QueryFile *, WorkingFile *> | |||
| MessageHandler::findOrFail(const std::string &path, ReplyOnce &reply, | |||
| int *out_file_id) { | |||
| int *out_file_id, | |||
| const bool acceptNotOpened) { | |||
src/message_handler.hh
Outdated
| @@ -236,7 236,8 @@ struct MessageHandler { | |||
| QueryFile *findFile(const std::string &path, int *out_file_id = nullptr); | |||
| std::pair<QueryFile *, WorkingFile *> findOrFail(const std::string &path, | |||
| ReplyOnce &reply, | |||
| int *out_file_id = nullptr); | |||
| int *out_file_id = nullptr, | |||
| const bool acceptNotOpened = false); | |||
|
Sure, here is a screenshot: The Navigator is in the bottom left corner - the content changes based on what is selected in the "Projects" view above (or in several other views, or in editor). No editor needs to be opened at all - it is just the file's structure. textDocument/documentLink (and most others) is not really needed, I think, as there is not a good way to show that to the user anyway. |
MaskRay
force-pushed
the
documentSymbol-for-unopened
branch
from
3ef09fc
to
98f25b5
Compare
MaskRay
changed the title
|
Thank you! |
I was trying to use ccls as a C/C server for the Apache NetBeans IDE, and it turned out the textDocument/documentSymbol only works for opened files. That is unfortunate, as NetBeans is using this request to fill the "Navigator", which generally works for any selected file, even those that are just selected in one of the files view. Moreover, the LSP specification says that: "Note that a server’s ability to fulfill requests is independent of whether a text document is open or closed.", so it would seem reasonable to me if the server would answer the query even for non-opened files. In the case of textDocument/documentSymbol, it seems this may be reasonably doable, see this patch.
What do you think?