foldingRange

[digrapsas]

Hi @gnikit, this is a first version of the foldingRange provider. It provides folding ranges from the beginning to the end of all existing scope blocks and I've added intermediate blocks for 1/ "else" in "if... else... endif" and 2/ "case" in "select case... case... end select" blocks.

The approach is quite simple and straightforward, ie I don't loop through the file_ast.scope_list to get the ranges, but I construct them as they come. For that reason, the "else" and "case" lines mentioned above, are not added in the corresponding "if" and "select case" scopes in file_ast.scope_list.

You might feel there is not enough testing, but 1/ the existing files are already a very good first test and 2/ the current version has been tested in an industrial size codebase (~10.000 *.f90 files, totally not respecting the "one thing per method" principle), where it loads without exceptions, and folding ranges look as expected in whatever file I have checked.

Any feedback is very welcome.

Regards

PS: It looks like I have some issues with the CI tests of the project, I'll try to have a look some time soon.

[codecov]

Codecov Report

Attention: Patch coverage is 93.87755% with 3 lines in your changes missing coverage. Please review.

Project coverage is 87.73%. Comparing base (522307d) to head (9f56239).
Report is 88 commits behind head on master.

❗ Current head 9f56239 differs from pull request most recent head 1ea1ebc

Please upload reports for the commit 1ea1ebc to get more accurate results.

Files	Patch %	Lines
fortls/langserver.py	90.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #376      +/-   ##
==========================================
+ Coverage   87.66%   87.73%   +0.06%     
==========================================
  Files          35       35              
  Lines        4793     4842      +49     
==========================================
+ Hits         4202     4248      +46     
- Misses        591      594       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gnikit]

Thanks for the PR @digrapsas, as I mentioned in our previous discussion adding a new LSP is not trivial and requires a fair bit of back and forth to get this to a mergeable state. I will try and review it within the next months, but I can't make any promises.

You said

The approach is quite simple and straightforward, ie I don't loop through the file_ast.scope_list to get the ranges, but I construct them as they come. For that reason, the "else" and "case" lines mentioned above, are not added in the corresponding "if" and "select case" scopes in file_ast.scope_list.

it's not very clear to me what you mean by this? Shouldn't you be using the existing start/end line and column numbers for scopes.

The CIs are failing because you haven't updated the schema, run

 python3 -m fortls.parsers.internal.intrinsics

and commit the updated .json file.

The more details and reasoning you can provide behind your decisions, the easier it will be for me to review and assess whether a design choice was necessary or it can be improved. I understand that this is a fair bit of work on your behalf, but as I mentioned previously there is a reason why I was listing this task as a 3 month GSoC project.

Thanks again for the contribution

[digrapsas]

@gnikit you're welcome, and no problem about going back and forth in order to get on the same page and merge the changes. I know this is a big project of which I've only seen a small part and I understand that probably many things I totally ignore have to be taken into consideration. I'm willing to respect your time schedule and propositions in order to merge the code, no rush.

Concerning the approach I've used to get the folding ranges, I don't exactly use the start/end lines of the scopes, which I suppose would look like: loop through file_ast.scope_list and get start/end lines to create the folding range couples. The approach I used is based on the idea that the latter "end" found is folding the latter "scope start keyword" parsed. Often though multiple scopes are nested, so a list of the remaining open scopes is needed. In total 3 lists are added as new members in FortranAST class (lines_to_fold, folding_start, folding_end) and the algorithm is the following:
1/ whenever a scope start keyword is found, add the line number in "lines_to_fold".
2/ a. If a scope end line is parsed: i) append the line number in "folding_end" list and
ii) append the last element of the "lines_to_fold" in "folding_start", pop this element from
"lines_to_fold"
b. If a scope start and end line is parsed (like "else" or "select case"), do steps i) and ii), and additionally
iii) append the current line number in "lines_to_fold"

In this way we finally get two lists (folding_start and folding_end) containing the folding start and end lines of each scope on the same position (ie folding_start[i] matches folding_end[i]), and an empty lines_to_fold list. This assumes that the file is correct syntactically, but when this is not the case (eg the size of the two lists doesn't match), None is returned and folding ranges are not provided in the given file.

I have to admit that the main reason I used this approach was because in first place I didn't realize that most of the scopes are already coded and parsed... So the next time I'll spend some time on the project I'll switch to an approach using the start/end lines of the scopes, to which I'll add a member "selines" to be able to treat the case 2/ b.

Glad to read your thoughts whenever you have some time.

PS: Thanks for the CI tests clarification.

[digrapsas]

@gnikit I changed the strategy used to get the folding ranges, ie now the ast_file.scope_list.sline and eline are used. I also added a new list member in Scope objects, mlines, in order to put there intermediate folding lines (like "else" in if statement, "select case", etc.). Then, mlines is used later in langserver.serve_folding_ranges in order to assign correct folding ranges.

I also added some code in order to fold multiline comment sections. I use the folding_start and folding_end lists that I mentioned in my previous comment because I didn't see any dedicated scope.

Concerning the CI tests, when I use the command you posted, I get the error posted below and the intrisics' file content gets erased. Probably because my python3 binary and part of my libraries are installed in different paths, but I haven't found out how figure this out yet...

Let me know if you have any questions or remarks.

[gnikit]

I haven't had a chance to look at this in detail but just make sure you don't duplicate information.
A lot of start/end line/column info is already stored, so you can infer them on-the-fly.

[digrapsas]

No rush here. 2 of the variables needed cleaning, but I have kept the rest for comments folding: because I don't see any structure storing the relative information.