-
Notifications
You must be signed in to change notification settings - Fork 65
Initial configuration of Codee Fortran formatter with examples (src/*.F90)
#707
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
base: develop
Are you sure you want to change the base?
Conversation
dustinswales
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@climbfuji Thanks for incorporating the code formatter here!
I love the idea, we just need to decide on a few details and incorporate this into our workflows.
As a github action, running the formatter could be invoked either diagnostically (report format violations) or prognostically (fix format violations).
run_codee_tmp.sh
Outdated
| #!/usr/bin/env bash | ||
|
|
||
| files=( | ||
| "src/ccpp_constituent_prop_mod.F90:free" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@climbfuji Is the "free" suffix here to use "free formatted" version of Codee?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I had some logic in that script previously that would use this, it's no longer there because not needed.
Since the script won't be merged, I didn't bother taking this out.
.codee-format
Outdated
| UnaryPlusMinus: NoTrailing | ||
| Comma: OnlyTrailing | ||
| Concat: Both | ||
| DoubleColon: Preserve |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to Preserve as per our discussion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to "unresolve" this conversation about whitespaces before/after colons. The argument was that these lines in CAM/SIMA are aligned for better readability. But this aligning doesn't just affect the white spaces before/after colons, but also before and after commas, parentheses, etc. See screenshot
Unless I change many of these SpacesAroundOperators to Preserve - which then defeats the purpose of using a formatter -, I'll get "unaligned" lines.
Going back to the previous comments and questions, "who is the audience"? The audience for the source code in ccpp-framework is ccpp-framework developers, not cam/sima scientists. I think it is ok to expect ccpp-framework developers to deal with non-aligned variable definitions in a handful of source files (and in auto-generated files, but these were never in question).
And, again, CAM/SIMA can do whatever it wants in their source code, independent on how we format ccpp-framework Fortran files.
src/*.F90)src/*.F90)
Initial configuration of Codee Fortran formatter with examples (
src/*.F90).This PR adds an initial
.codee-formatconfiguration file for the free Codee Fortran formatter and a temporary scriptrun_codee_tmp.shto get started with the tool (note that the day-to-day usage will be much simpler than in the script). The PR also demonstrates the effect of the formatting rules for the four Fortran source files insrc/.The goal of this PR is to provide the necessary information to decide on the formatting rules we want to use. Once we have agreed on the format, we can update the
src/*.F90files as needed and merge this PR. The integration with GitHub actions and the simplified usage on the command line will come after that. In a 3rd PR, we will work on a tighter integration of the codee format config with the capgen Fortran write to make sure that the auto-generated code also complies with the formatting rules.The majority of the diffs are because NEPTUNE (and UFS) use two whitespaces as indentation, whereas the files in
src/currently use 3 whitespaces. We can change the codee config, but every space we save makes the lines shorter. To see the remaining differences, please select "Hide whitespaces" when looking at the files changed.User interface changes?: No
Working toward #703
Testing: No changes to the tests - GitHub actions CI tests passed
test removed:
unit tests:
system tests:
manual testing: