Add Folding Range functionality to air #146
Conversation
|
I'm sorry that the current form is not based on biome-rowan, but I struggled with the fact that the rowan ast does not treat comments as equivalent elements in the tree (rather as children of the next element?), and I didn't figure out how to extract the cursor location (line row and column), which are important for folding ranges. With tree-sitter, I can mimick a lot of existing AIR code and get it working quickly. Maybe we can talk about converting this tree-sitter-based protytype to biome_rowan later? |
|
@lionel- @DavisVaughan Let me know your thoughts when you guys are less busy. |
| None => String::new(), | ||
| }; | ||
|
|
||
| let start = node.start_position(); |
There was a problem hiding this comment.
I couldn't find an equivalent of this using biome_rowan
| ); | ||
| folding_ranges.push(folding_range); | ||
| } | ||
| "comment" => { |
There was a problem hiding this comment.
biome_rowan treats comments as children of the next command. This is super annoying. Things are much neater with treesitter
| let node_type = node.kind(); | ||
|
|
||
| match node_type { | ||
| "parameters" | "arguments" | "braced_expression" => { |
There was a problem hiding this comment.
Let me know if I miss some of other node types with brackets that I should take care of
|
@lionel- any updates? |
|
We are focusing a ton of time just trying to get the initial version of the formatter out the door, we will come back and review this in detail after that, and we really appreciate your patience, sorry it is taking so long, it's just a very big context switch for us at the moment! |
|
@DavisVaughan @lionel- seems like air is now shipped in Positron. Is it now time to come back to this PR? |
|
Hi @kv9898. On hindsight we prefer to wait a bit before implementing other LSP features in Air. It's still a bit early and we're missing infrastructure and practices as you have mentioned. Would you mind moving this over to Ark? Then you can use the TS implementation without any issues. Sorry for the delay in treating your PR. |
|
@lionel- Good to hear back from you again. Personally I don't mind moving the PR back to ark - actually this PR was originally for ark anyway and I was told to move it to here. I'm just slightly worried that the PR would also be stuck in the ark repo for a really long time, in which case the migration might not be worthwhile. If the new PR will be reviewed promptly then I will be more than happy to do the migration in the next few days or so. |
Yea we apologize for that. We now think we can promptly get your PR reviewed and merged if it is on the Ark side, since it is "just one more LSP feature for Ark", rather than "the first non-formatting LSP feature for Air" which has much higher overhead for us to consider. |
Great! Give me one day or two and i'll get this done. |
|
Migration done: posit-dev/ark#815 |
3 participants
This PR replicates my old PR to ark: posit-dev/ark#615.
Old description:
It seems that the call for folding ranges has been quite a while: posit-dev/positron#18, posit-dev/positron#2924, posit-dev/positron#3822.
I was initially only thinking about adding foldable comments, but it seems that doing this will disable the existing folding support for things like regions and brackets. Therefore, I rewrote these functionalities also.
The PR already supports the folding-range-handling of brackets, regions, code cells and nested comment sections:
(The screenshot is new, though)
Note that, compared to the old PR, this PR no longer relies on a naive search for brackets. Instead it uses the new
tree_sitterof air to walk down the AST for node handling.