-
-
Notifications
You must be signed in to change notification settings - Fork 369
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
Feat: Folding Ranges #3058
Feat: Folding Ranges #3058
Conversation
@kokobd I have started to implement Folding Ranges. Is there any reference/documentation for Code Range that I can use to understand it better? |
cc: @michaelpj |
@sloorush "code range" is just a concept I come up with to unify selection range and folding range. The motivation is to split source code into "ranges". Please just read the code. And by design, you should be able to traverse the Moreover, feel free to add more fields to |
…rm folding ranges
It would be nice to post a screencast here and add some tests to |
Yeah, this definitely needs some tests. |
abbc76c
to
ca807f0
Compare
ca807f0
to
e6a2b5c
Compare
I'll try to write those as well, They were mostly very random, I will check if there is a pattern. |
Even writing down random examples is useful, it gives people something to start with if they want to investigate further. |
@michaelpj, All requested changes have been resolved, Please take a look. cc: @kokobd |
It will be better to collapse the export list rather than the whole file when clicking on the folding marker at line 6. See below: @sloorush I think it's okay to handle this in a subsequent PR if you are eager to merge this one. |
plugins/hls-code-range-plugin/test/testdata/selection-range/Import.hs
Outdated
Show resolved
Hide resolved
We will have to tinker with code ranges and why is this code range so big, Should we make a new issue to tackle this? |
I recommend adding a test case in |
Yeah, the best thing if you ever find a weird case is to record it with a test. Helps to remember and make it obvious why it's weird, and have somewhere to start to fix it. |
Not really sure why this is happening but it happens when there is an export, I'll check how exports work in the AST and write if I find something on this The folding ranges generated look like this:
The first one is causing the issue, This happens when there is an export(even a single line one) like: folding ranges:
|
So for this file: The first folding range seen on VSCode collapses everything after this line The first code range looks like this:
Which has the root node range ((2,16):(7,21)) which is from the start of the export parenthesis to the end of the file(even when there are functions that are not exported, it is folded till the end of the file) Full Output of CodeRanges of this function:
I believe this is an issue dealing with CodeRange conversion from the AST. |
@sloorush So you can just skip the root node when constructing folding ranges? I believe no one wants to fold the whole file. I wonder why it starts from line 2 instead of line 0, but that's another problem. |
For a file where there are no exports, The first folding range of the first function just folds the entire file.
I'll do this |
All set! Should be good to go now finally! |
a32924c
to
ccd9fa5
Compare
Added "merge me" label. It will be squashed and merged automatically when CI passes. |
This PR adds the feature of Folding Ranges to the Range Plugin.
References: