[Clangd] Implementing More Folding Ranges and the Pseudo Parser (original) (raw)

I am looking into extending the current folding range functionality. As I understand it, currently the folding logic uses the remains of a pseudo parser that has been abandoned. There exists another implementation of getFoldingRanges that uses the AST, but is currently unused. For new ranges, such as case statements, private/public sections of class definitions I think it would be much easier and cleaner to use this AST implementation. However I am also interested in being able to fold preprocessor directives, and have a PR open for that. Switching entirely to the AST implementation would make it impossible to fold these regions, along with block comments, if I understand correctly. I am wondering what the best course of action would be here, my initial thought is that both implementations should be called, and the resulting list each provides should be merged to create a complete set of ranges.

I think both of these would be possible to implement based on the AST as well:

Of course, that doesn’t necessarily mean that getting this information from the AST would be an improvement, as there is a latency to the AST becoming available that’s not there for the current implementation (as I described in more detail on Discord).

One thing to figure out here is how this would work over LSP.

Unlike diagnostics, which are a “notification” (server sends them to client of its own initiative), folding ranges are a “request” (client requests them from the server). If we respond to the request before the AST is built (to send folding ranges computed without the AST), we don’t really have a straightforward mechanism of pushing additional folding ranges to the client once the AST is built.

What might work is to use the LSP’s “partial result” feature to respond to the client’s request with the pre-AST folding ranges as a partial result, and then respond with the AST-based ranges as the rest of the result. This needs some experimentation to validate that it works in practice. Note also that this means the AST-based ranges need to be purely additive, i.e. they can’t “override” the pre-AST ranges, just add to them.

+1 to all subtle bits mentioned by @HighCommander4, i think the original requriements were also along these lines;