Closed digrapsas closed 5 months ago
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.:exclamation: 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 :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@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.
@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.
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.