Add more break points for long lines by gold2718 · Pull Request #727 · NCAR/ccpp-framework

gold2718 · 2026-03-21T22:12:48Z

Some long lines are still broken incorrectly

Some long lines are still broken into invalid Fortran. This PR addresses this by adding more non-ID characters to the allowed break points (currently only spaces and commas).

User interface changes?: No

Fixes: TBA

Testing:
test removed:
unit tests:
system tests:
manual testing:

gold2718 · 2026-03-22T10:13:37Z

@climbfuji, The latest Codee failure seems to be a mismatch between the default settings of the FortranWriter class and what Codee expects. Am I seeing that correctly? If so, I suggest we either modify Codee's expectations or the FortranWriter class defaults (e.g., __INDENT, __CONTINUE_INDENT, etc.). Thoughts?

Note, linebreak_test.F90 now compiles with no warnings so this is just a formatting issue.

gold2718 · 2026-03-22T10:28:57Z

Another Codee comment. While I can match most of Codee's formatting expectations by setting __INDENT = 2, __CONTINUE_INDENT = 4, lowercasing CONTAINS, and minimizing the spacing before the continue character, Codee expects spaces around operators. Unfortunately, this defeats the purpose of the test which is how to break lines when spaces are not present.

Ironically, if capgen had been generating Codee-approved Fortran, @jimmielin would never have seen the bug he reported in #724 🤷.

climbfuji · 2026-03-23T04:05:05Z

Another Codee comment. While I can match most of Codee's formatting expectations by setting __INDENT = 2, __CONTINUE_INDENT = 4, lowercasing CONTAINS, and minimizing the spacing before the continue character, Codee expects spaces around operators. Unfortunately, this defeats the purpose of the test which is how to break lines when spaces are not present.

Ironically, if capgen had been generating Codee-approved Fortran, @jimmielin would never have seen the bug he reported in #724 🤷.

We had a similar issue with another test (#722; comment #722 (comment)). If you want to preserve formatting for certain parts of the code, you can wrap those lines in

! codee format off
...
! codee format on

See c221aaf

climbfuji

try this please

climbfuji · 2026-03-23T16:44:59Z

-        name079, name080, name081, name082, name083, name084, name085, name086, name087,          &
-        name088, name089, name090, name091, name092, name093, name094, name095, name096,          &
-        name097, name098, name099 /)
+   character(len=7) :: data(100) = (/ 'name000', 'name001', 'name002', 'name003', 'name004',      &


Suggested change

character(len=7) :: data(100) = (/ 'name000', 'name001', 'name002', 'name003', 'name004', &

! codee format off

character(len=7) :: data(100) = (/ 'name000', 'name001', 'name002', 'name003', 'name004', &

climbfuji · 2026-03-23T16:45:19Z

+           'Cannot read columns_on_task from file'//                                              &
+           ', columns_on_task has no horizontal dimension; columns_on_task is a protected variable'
+
+   end subroutine foo


Suggested change

end subroutine foo

end subroutine foo

! codee format on

Add missing spaces in host cap (for Codee).

climbfuji · 2026-04-03T13:54:25Z

Update 2026/04/03. This PR is included in #732. When #732 gets merged, this PR will be automatically closed as merged.

gold2718 · 2026-04-05T03:32:26Z

I managed to fix the Fortran writer to pass these tests. The two sections that are Codee commented out are purposefully bad Fortran lines to make sure the writer can still break them legally. While #732 avoids the previous problems with this PR, I think the new version helps move the Fortran writer closer to being Codee compliant.

climbfuji · 2026-04-06T11:40:51Z

I managed to fix the Fortran writer to pass these tests. The two sections that are Codee commented out are purposefully bad Fortran lines to make sure the writer can still break them legally. While #732 avoids the previous problems with this PR, I think the new version helps move the Fortran writer closer to being Codee compliant.

I agree. I propose to merge #727 and then update #732 to make the list of excluded files in the GitHub workflow an empty list for now. It's nice to have the capability to exclude individual files entirely.

mkavulich

LGTM. @climbfuji I assume we can go ahead and merge this PR then?

gold2718 · 2026-04-06T15:30:01Z

I agree. I propose to merge #727 and then update #732 to make the list of excluded files in the GitHub workflow an empty list for now. It's nice to have the capability to exclude individual files entirely.

This sounds like a good idea although aside from testing line breaking, I can't think of any reasons we would want to generate non-compliant code.

climbfuji · 2026-04-06T15:43:04Z

I agree. I propose to merge #727 and then update #732 to make the list of excluded files in the GitHub workflow an empty list for now. It's nice to have the capability to exclude individual files entirely.

This sounds like a good idea although aside from testing line breaking, I can't think of any reasons we would want to generate non-compliant code.

One reason that we are hitting with the NEPTUNE code base is not related to non-compliant code, but to bugs that we identified in the Codee formatter itself. We report those back and the team needs a bit of time to fix them and release a new version. In the meantime, we exclude those files from formatting.

climbfuji · 2026-04-06T15:43:22Z

LGTM. @climbfuji I assume we can go ahead and merge this PR then?

Yes please

…ing in GitHub actions (#732) ## Description This PR adds the capability to exclude individual Fortran files from Codee formatting in GitHub actions. This was originally motivated by issues with making the Fortran writer Codee compliant (PR #727). These issues were resolved, but there are scenarios were excluding individual files from Codee formatting checks may be needed (see comments in #727). User interface changes?: No ## Issues N/A ## Testing Testing: **all pass** (only updates to existing tests, no new tests, no tests removed) ## Additional information See comments in #727 --------- Co-authored-by: Steve Goldhaber <gold2718@gmail.com> Co-authored-by: Haipeng Lin <hplin@ucar.edu>

Add more break points for long lines

d46b203

gold2718 requested review from a team as code owners March 21, 2026 22:12

gold2718 mentioned this pull request Mar 21, 2026

When allocating dyn_const_array pre-calculate size to avoid line length limits #724

Closed

Steve Goldhaber added 2 commits March 22, 2026 10:41

Try to make Codee happy

8919657

Try harder to make codee happy

f960f1f

peverwhee approved these changes Mar 23, 2026

View reviewed changes

climbfuji requested changes Mar 23, 2026

View reviewed changes

Make Fortran output more Codee compliant

6c42138

Add missing spaces in host cap (for Codee).

jimmielin mentioned this pull request Mar 24, 2026

sima0_13_006: CCPPize prescribed_volcanic_aerosol (prescribed_volcaero); update tracer_data_test snapshot ESCOMP/CAM-SIMA#477

Merged

climbfuji mentioned this pull request Mar 26, 2026

Update codee formatter to require a space after .not. #728

Closed

Merge branch 'develop' into fix_breaks

0096bb3

climbfuji mentioned this pull request Apr 3, 2026

Add capability to exclude individual Fortran files from Codee formatting in GitHub actions #732

Merged

Fortran unit tests passing Codee with two omissions

b9a9a9f

gold2718 requested a review from climbfuji April 5, 2026 20:42

climbfuji approved these changes Apr 6, 2026

View reviewed changes

mkavulich approved these changes Apr 6, 2026

View reviewed changes

mkavulich merged commit 7db88e5 into NCAR:develop Apr 6, 2026
15 checks passed

gold2718 deleted the fix_breaks branch April 6, 2026 21:51

	character(len=7) :: data(100) = (/ 'name000', 'name001', 'name002', 'name003', 'name004', &
	! codee format off
	character(len=7) :: data(100) = (/ 'name000', 'name001', 'name002', 'name003', 'name004', &

Uh oh!

Conversation

gold2718 commented Mar 21, 2026

Uh oh!

gold2718 commented Mar 22, 2026

Uh oh!

gold2718 commented Mar 22, 2026

Uh oh!

climbfuji commented Mar 23, 2026

Uh oh!

climbfuji left a comment

Choose a reason for hiding this comment

Uh oh!

climbfuji Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

climbfuji Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

climbfuji commented Apr 3, 2026

Uh oh!

gold2718 commented Apr 5, 2026

Uh oh!

climbfuji commented Apr 6, 2026

Uh oh!

mkavulich left a comment

Choose a reason for hiding this comment

Uh oh!

gold2718 commented Apr 6, 2026

Uh oh!

climbfuji commented Apr 6, 2026

Uh oh!

climbfuji commented Apr 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants