feat: textDocument/foldingRange#1256
Conversation
| if Dm.DocumentManager.is_parsing st then | ||
| Error {code=(Some Jsonrpc.Response.Error.Code.ServerCancelled); message="Parsing not finished"} |
There was a problem hiding this comment.
How will this work if we really are in the middle of parsing? The client won't ask us again for the folding ranges once we are done parsing, no?
There was a problem hiding this comment.
You're right. I pretty much copy-pasted the code from other requests that require parsing. Maybe we should implement some callback mechanism for those, so that the requests would resume after parsing is done...
There was a problem hiding this comment.
I don't know about folding ranges, but for the overview we do return the message "not ready yet" but vscode never asks again. It seems a bug in vscode (or better, it does not implement LSP as designed).
There was a problem hiding this comment.
Hmmm, IIUC the ServerCancelled error is only supported for semantic tokens and diagnostics.
There was a problem hiding this comment.
Should parsing be an asynchronous task then? It seems reasonable to me for the LSP to wait until it is done with parsing before responding to a request.
There was a problem hiding this comment.
That seems reasonable. Still, I would implement that in a new PR, and allow for returning ServerCancelled here for now.
|
|
||
| (** Extracts sub-sentence Gallina folds from vernacular AST nodes that contain | ||
| located terms. *) | ||
| let entries_of_vernac_ast (document: Document.document) (sentence: Document.sentence) (ast: Synterp.vernac_control_entry) : entry list = |
There was a problem hiding this comment.
Can you have a look at the outline? (it is roughly a bunch of ranges with a label)
There is quite some code that is common, in spirit at least.
vsrocq/language-server/dm/document.ml
Lines 191 to 252 in 90a46b4
I wonder if it makes sense to have the outline be a projection of this folding info...
There was a problem hiding this comment.
I designed the folding code with that in mind, I think that it should be fairly easy to reuse the new folding logic for the outline. I use a more "structured" type as an intermediate datatype that should also preserve the nesting information needed for the outline.
I think that we should be able to slightly extend this datatype and be able to project both the folding ranges and the outline from that. (One thing that would have to be filtered out is the indentation-based "fallback" ranges)
There was a problem hiding this comment.
Do you prefer merging the two features in one PR or is it OK to do a separate one for refactoring the outline?
There was a problem hiding this comment.
If it is true that this new code is fast enough and a superset of the other, I think the best way to proceed is to:
- add to the folding ranges data structure the "labels" used by the overview
- write the projections
- remove the old overview code and call the new one
Then we merge 1+2 in a PR. Finally we replace the overview code with 3.
If 3 turns out be not be ok, we can just revert one PR and still keep the folding.
But I think it is easier to develop 3 atop the other changes, and just (not) merge the last commit.
closes #799 #137