Skip to content
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

Feature/refactor #58

Merged
merged 44 commits into from
Mar 25, 2022
Merged

Feature/refactor #58

merged 44 commits into from
Mar 25, 2022

Conversation

gnikit
Copy link
Member

@gnikit gnikit commented Feb 25, 2022

This PR aims to further simplify the servers design, make maintainace easier and improve performance where possible.

Changes

  • Move dataclasses to separate submodule
  • Break definition testing into blocks of similar content
  • Improve typing hints
  • Imporve docstrings
  • Improve test coverage
  • Changes how renaming type-bound procedures with implicit names is done. Previously it would create a pointer, now it correctly identifies all objects that are part of the procedure and the implementation. This is the correct fix to Rename fails to rename implementation reference hansec/fortran-language-server#104

TODO:

  • Make process_file a method of file_obj, named parse which resoves file objects contents and produces an AST file
  • In process_file rewrite every if ...: continue block to be a separate method
  • Consider changing the constructors in objects to receive their corresponding dataclass object
  • Create json_templates.py with functions for generating json boilerplate code
  • Remove Python 2 references
  • Minimise calls to self.post_messages.append in favour of self.post_message()
  • Create enum for windowMessage notifications

@codecov
Copy link

codecov bot commented Feb 25, 2022

Codecov Report

Merging #58 (6a4fb9f) into master (4e0bb5d) will increase coverage by 2.52%.
The diff coverage is 93.79%.

@@            Coverage Diff             @@
##           master      #58      +/-   ##
==========================================
+ Coverage   81.72%   84.24%   +2.52%     
==========================================
  Files           9       11       +2     
  Lines        4372     4361      -11     
==========================================
+ Hits         3573     3674     +101     
+ Misses        799      687     -112     
Impacted Files Coverage Δ
fortls/langserver.py 81.09% <86.30%> (+8.04%) ⬆️
fortls/parse_fortran.py 87.73% <94.65%> (+1.31%) ⬆️
fortls/helper_functions.py 87.86% <96.00%> (+1.03%) ⬆️
fortls/constants.py 100.00% <100.00%> (ø)
fortls/ftypes.py 100.00% <100.00%> (ø)
fortls/json_templates.py 100.00% <100.00%> (ø)
fortls/objects.py 81.46% <100.00%> (+0.29%) ⬆️
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

- Moves dataclasses to ftypes submodule
- Uses OOP to remove explicit dependance on array indices
- Imporves typing hints
- Improves docstrings
- Simplifies a few code blocks

TODO:
1. make process_file a method of `fortran_file`
2. further improve typing hints
3. further improve code quality and design
This method truly belongs inside the class.

TODO: investigate if pp variables need to be passed as args
The benefit of having the as args in the function level is that we can
update them from the server options i.e. config file
and pass them
fortls/ftypes.py Show resolved Hide resolved
fortls/langserver.py Outdated Show resolved Hide resolved
fortls/langserver.py Show resolved Hide resolved
fortls/langserver.py Show resolved Hide resolved
fortls/langserver.py Show resolved Hide resolved
fortls/parse_fortran.py Show resolved Hide resolved
fortls/parse_fortran.py Outdated Show resolved Hide resolved
fortls/parse_fortran.py Outdated Show resolved Hide resolved
fortls/parse_fortran.py Outdated Show resolved Hide resolved
fortls/parse_fortran.py Outdated Show resolved Hide resolved
@gnikit gnikit force-pushed the feature/refactor branch from 31d96dd to 00192b1 Compare March 11, 2022 00:21
This should make it easier to extend the parsing documentation
functionality of FORD and Doxygen in the future
@gnikit gnikit force-pushed the feature/refactor branch from 6dbb434 to d2ff31a Compare March 15, 2022 18:21
@gnikit gnikit merged commit b80a396 into master Mar 25, 2022
@gnikit gnikit deleted the feature/refactor branch March 25, 2022 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant