Skip to content

fix: comment re-wrap no longer orphans the overflow word (#9) - #11

Merged
sbryngelson merged 2 commits into
masterfrom
fix/comment-rewrap-orphan
Sep 2, 2026
Merged

fix: comment re-wrap no longer orphans the overflow word (#9)#11
sbryngelson merged 2 commits into
masterfrom
fix/comment-rewrap-orphan

Conversation

@sbryngelson

@sbryngelson sbryngelson commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Closes #9.

Root cause

Plain ! comments are wrapped one source line at a time (src/formatter.rs, the LineKind::Comment arm). wrap_comment splits the over-long line at a word boundary and both halves are emitted immediately, so the tail becomes a standalone comment line. Nothing looks at the comment line that follows, which is where that tail belongs. Doxygen !> / !! blocks already avoid this because they are joined and re-wrapped as a unit; plain prose had no equivalent.

That is why a purely structural edit rewrites prose: wrapping a block in #:if adds one indent level, which pushes a comment sitting near the limit over it.

Fix

An over-long prose comment now pushes its overflow into the following prose lines of the same block. Each line absorbs what arrives from above and passes its own overflow down; only what is left past the end of the block starts a new line.

Words are only ever pushed down, never pulled up. A comment that already fits is emitted unchanged, so the change is inert for files that were not being mangled and the resulting diff stays minimal. The reproducer goes from a 3-line mangled block to the 2 lines the issue asks for.

What counts as prose

Reflowing moves words between lines, so it may only touch running text. A line is eligible only if it is spelled exactly ! plus one space, which in one rule rejects every marker form: !!, !>, !<, !*, !@, !$, the protected Fypp continuation !&, vendor directives such as !DEC$ and !GCC$, and an unspaced !text under space-after-comment = false. On top of that, a line is refused when it is

  • blank, or a separator banner (! ------, ! === Setup ===)
  • a bullet or numbered item (! - x, ! 1. x), or a Doxygen command (! @param x)
  • a TODO: / NOTE: / FIXME:-style tag
  • indented past the marker for alignment
  • ! ffmt off / ! ffmt on

Refusing a line also ends the block, so the overflow stays on its own line exactly as before. Both the raw and the normalized form of each line are tested, so normalizing a marker cannot turn it into prose.

This keeps the invariant join_short_comments already states in its own comment: lines that are independent comments are not merged.

Reflowing is disabled in range mode, since consuming the next line would rewrite a line outside the requested range.

Before / after

Reproducer from the issue, ffmt repro.fpp:

! before
            ! Indices for U and F: (rho, rho*vel(1), rho*vel(2), rho*vel(3), By, Bz, E) Note: vel and B are permutated, so vel(1) is
            ! the
            ! normal velocity, and x is the normal direction

! after
            ! Indices for U and F: (rho, rho*vel(1), rho*vel(2), rho*vel(3), By, Bz, E) Note: vel and B are permutated, so vel(1) is
            ! the normal velocity, and x is the normal direction

Tests

Ten reflow tests in tests/bugfixes.rs. Five fail on master or on the first commit of this branch and pass at its head; the rest encode behavior that must not change (separator lines, ! ffmt off, range mode, ordinary prose, idempotency) and pass throughout.

cargo test (all 15 binaries), cargo fmt --check, and cargo clippy --all-targets -- -D warnings are clean.

Corpus check

Both binaries were run over all 99 MFC Fortran and Fypp sources at line-length 132, 100, 80 and 72:

  • byte-identical to master at MFC's real 132, so the change is inert until a comment actually overflows
  • zero content divergence at every width: every non-comment line identical, every comment word identical in order and marker
  • identical over-limit line counts, so no new long lines
  • idempotent on a second pass
  • 49 / 52 / 51 files improved at 100 / 80 / 72, unchanged by the prose guard

Not addressed

The follow-up in the issue, an option to keep Fypp conditional bodies at their existing indent, is a separate concern and is not touched here.

Plain `!` comments were wrapped one source line at a time, so the tail of
an over-long line became a standalone comment line instead of joining the
prose that followed it. Indenting an existing block one level deeper (for
example by wrapping it in `#:if`) was enough to mangle a comment.

An over-long prose comment now pushes its overflow into the following
comment lines of the same block: each line absorbs what came from above
and passes its own overflow down, and only what is left past the end of
the block starts a new line. Words are never pulled upward, so comments
that already fit are untouched and the diff stays minimal.

A block ends at anything that is not free prose - Doxygen markers,
separator banners, blank comment lines, directives, and `! ffmt off` -
and reflowing is disabled in range mode, which must not rewrite lines
outside the requested range.
Code review found the reflow guard was far too permissive. It accepted
any following comment line containing one alphanumeric character, so
overflow was pushed into lines that carry structure, and two of those
cases were regressions against the previous behavior.

Confirmed against the released binary, all now fixed:

- `!&`, a protected Fypp continuation marker, was merged into the prose
  (`! the & keep me`). Master left it alone.
- Vendor directives such as `!DEC$ ATTRIBUTES INLINE :: foo` and
  `!GCC$ unroll 4` were absorbed. `classify` routes only `!$` and `!DIR`
  to `LineKind::Directive`, so these arrive as ordinary comments.
- `! TODO: rewrite this loop` became `! the TODO: rewrite this loop`,
  moving the tag off the start of its line where scanners look for it.
- Bullets (`! - first item`), numbered items and `! @PARAM x` were
  absorbed, and titled banners such as `! ===== Initialization =====`
  were swallowed even though the all-punctuation form was refused.
- With `space-after-comment` disabled, an unspaced `!text` line was
  rewritten as `! text`, inserting the space the user turned off.

`prose_comment_text` now requires the line to be spelled exactly `!`
plus one space, which rejects every marker form including `!&` and the
vendor directives, and refuses bullets, numbered items, `TODO:`-style
tags, banners with a run of rule characters, and lines indented past the
marker for alignment. The raw line is tested alongside the normalized
one so that normalizing a marker cannot turn it into prose.

This restores the invariant `join_short_comments` already states: lines
that are independent comments are not merged.

The overflow check is now a length comparison rather than a wrap that
gets discarded on the reflow path.
@sbryngelson

Copy link
Copy Markdown
Owner Author

Review round

A code review of this branch found the block-boundary guard was too permissive, and I pushed 69e2cb0 to fix it. Six issues, all reproduced against the released binary before fixing and covered by tests now.

Two were regressions against master:

  • !& is a protected Fypp continuation marker (normalize_comment_space exempts it, and docs/configuration.md says so). It was being merged into the prose as ! the & keep me.
  • Vendor directives such as !DEC$ ATTRIBUTES INLINE :: foo and !GCC$ unroll 4 were absorbed. classify routes only !$ and !DIR to LineKind::Directive, so these arrive as ordinary comments.

Four were new mangling of structured comments:

  • ! TODO: rewrite this loop became ! the TODO: rewrite this loop, moving the tag off the start of its line.
  • Bullets (! - first item), numbered items (! 1. step) and ! @param x were absorbed.
  • Titled banners such as ! ===== Initialization ===== were swallowed, even though the all-punctuation form ! ------ was already refused.
  • With space-after-comment disabled, an unspaced !text line was rewritten as ! text, inserting the space the user turned off.

prose_comment_text now requires the line to be spelled exactly ! plus one space, which rejects every marker form in one rule, and additionally refuses bullets, numbered items, TODO:-style tags, banners containing a run of rule characters, and lines indented past the marker for alignment. The raw line is checked alongside the normalized one, so normalizing a marker cannot turn it into prose. That restores the invariant join_short_comments already states in its own comment: independent comment lines are not merged.

The review also noted the overflow trigger computed a full wrap_comment and then threw it away on the reflow path; that is now a length comparison.

One finding I did not change: avail falls back to 40 columns when line-length is smaller than the indent plus marker. It mirrors wrap_comment, and diverging would produce two different widths for comments in the same file, so the fallback stays consistent with the existing behavior.

Re-verification

  • 10 reflow tests now, 65 in bugfixes.rs. The three new guard tests fail on a69fb6c and pass here; the original reproducer test still passes.
  • Full suite (15 binaries), cargo fmt --check, cargo clippy --all-targets -- -D warnings clean.
  • MFC corpus differential re-run at line-length 132, 100, 80 and 72 across all 99 files: byte-identical to master at 132, zero content divergence at every width, identical over-limit line counts, idempotent on a second pass, and the same 49/52/51 files improved as before the tightening. The guard costs nothing on real prose.

@sbryngelson
sbryngelson merged commit b0d4d23 into master Sep 2, 2026
9 checks passed
@sbryngelson
sbryngelson deleted the fix/comment-rewrap-orphan branch September 2, 2026 20:42
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.

Comment re-wrap orphans the overflow word onto its own line instead of reflowing into the next comment line

1 participant