diff --git a/docs/configuration.md b/docs/configuration.md index 625061b..9b11aaf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -125,7 +125,9 @@ private; public :: s_foo private; public :: s_foo ! preserved ### `rewrap-comments` (toggle) -Re-wrap long `!` and `!!` comment blocks at `line-length`. Doxygen `!>` / `!!` blocks are joined and re-wrapped as units. Short consecutive comment lines are merged. +Re-wrap long `!` and `!!` comment blocks at `line-length`. Doxygen `!>` / `!!` blocks are joined and re-wrapped as units. Short consecutive comment lines are merged. When a comment of running prose overflows, the overflow is pushed into the following prose line of the same block rather than left on a line of its own. + +Only running text absorbs overflow. A comment line that carries structure ends the block and is never rewritten: any marker (`!!`, `!>`, `!<`, `!*`, `!@`, `!$`, `!&`, or a vendor directive such as `!DEC$`), a blank comment line, a separator banner (`! ----`, `! === Setup ===`), a bullet or numbered item (`! - x`, `! 1. x`), a `TODO:`-style tag, a line indented past the marker for alignment, and `! ffmt off`. A comment written without the space after `!` is left alone as well. **Default:** `true` diff --git a/src/formatter.rs b/src/formatter.rs index aaedab9..cf3c748 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -4,7 +4,7 @@ use crate::classifier::{ }; use crate::config::{Config, EndOfLine, KeywordCase}; use crate::keyword_norm::normalize_keywords; -use crate::reader::read_logical_lines; +use crate::reader::{read_logical_lines, LogicalLine}; use crate::scope::ScopeTracker; use crate::whitespace::normalize_whitespace; @@ -349,12 +349,41 @@ pub fn format_with_config(source: &str, config: &Config, range: Option<(usize, u output_lines.extend(wrapped); } else { let indented = apply_indent(&content, depth, config.indent_width); - let wrapped = if config.rewrap_comments.is_enabled() { - wrap_comment(&indented, config.line_length, depth, config.indent_width) + if !config.rewrap_comments.is_enabled() { + output_lines.push(indented); } else { - vec![indented.clone()] - }; - output_lines.extend(wrapped); + // A prose comment that overflows must not leave its tail + // as a standalone one-word line: push the overflow into + // the following comment lines of the same block instead. + // That rewrites those lines, so it is disabled in range + // mode, which must not touch lines outside the range. + // The raw line is tested as well as the normalized one, so + // that a marker normalization cannot turn into prose. + let reflow = indented.len() > config.line_length + && config.line_length < 1000 + && range.is_none() + && ll.raw_lines.len() == 1 + && prose_comment_text(trimmed.trim_start()).is_some() + && prose_comment_text(&content).is_some(); + if reflow { + reflow_comment_block( + &content, + &logical_lines, + &mut idx, + &mut tracker, + depth, + config, + &mut output_lines, + ); + } else { + output_lines.extend(wrap_comment( + &indented, + config.line_length, + depth, + config.indent_width, + )); + } + } } } LineKind::Blank => unreachable!(), @@ -746,20 +775,25 @@ fn emit_hoisted_comments( output_lines: &mut Vec, ) { for comment in comments { - let c = if config.unicode_to_ascii { - crate::unicode::replace_unicode(comment) - } else { - comment.clone() - }; - let c = if config.space_after_comment.is_enabled() { - normalize_comment_space(&c) - } else { - c - }; + let c = normalize_comment_content(comment, config); output_lines.push(apply_indent(&c, depth, config.indent_width)); } } +/// Apply the per-comment content cleanups (unicode folding, marker spacing). +fn normalize_comment_content(content: &str, config: &Config) -> String { + let c = if config.unicode_to_ascii { + crate::unicode::replace_unicode(content) + } else { + content.to_string() + }; + if config.space_after_comment.is_enabled() { + normalize_comment_space(&c) + } else { + c + } +} + fn is_ffmt_marker(line: &str) -> Option { let t = line.trim().to_ascii_lowercase(); if t == "! ffmt off" || t == "! ffmt: off" { @@ -1027,6 +1061,148 @@ fn extract_comment_text<'a>(line: &'a str, marker: &str) -> &'a str { after_marker.strip_prefix(' ').unwrap_or(after_marker) } +/// Characters that make a horizontal rule when repeated. +const RULE_CHARS: &[char] = &['-', '=', '*', '#', '_', '~', '+']; + +/// Text of a plain `!` comment that is free prose, or `None`. +/// +/// Reflowing moves words between lines, so it may only touch lines that are +/// running text. Everything that carries structure is refused, and refusing it +/// also ends the block: a marker of any kind (`!!`, `!>`, `!<`, `!*`, `!@`, +/// `!$`, `!&`, or a vendor directive such as `!DEC$`), a blank comment line, a +/// separator banner, a bullet or numbered list item, a `TODO:`-style tag, and +/// `! ffmt off`. Anything not spelled exactly `!` + one space is a marker of +/// some kind, which also leaves an unspaced `!text` alone when +/// `space-after-comment` is off. +fn prose_comment_text(content: &str) -> Option<&str> { + let text = content.strip_prefix("! ")?; + // A second space is deliberate alignment (tables, ASCII art), not prose. + if text.starts_with(' ') { + return None; + } + if is_ffmt_marker(content).is_some() { + return None; + } + // Blank comment lines and pure banners (`! ----`) have nothing to reflow. + if !text.chars().any(|c| c.is_alphanumeric()) { + return None; + } + let first = text.chars().next()?; + if "-*+>|#@=~:.".contains(first) { + return None; + } + // A run of rule characters marks a titled banner (`! === Setup ===`). + let mut prev = '\0'; + let mut run = 1; + for c in text.chars() { + if c == prev && RULE_CHARS.contains(&c) { + run += 1; + if run >= 4 { + return None; + } + } else { + run = 1; + prev = c; + } + } + let word = text.split_whitespace().next()?; + // `TODO:`, `NOTE:`, `FIXME:` start a new remark rather than continue one. + if let Some(tag) = word.strip_suffix(':') { + if !tag.is_empty() && tag.chars().all(|c| c.is_ascii_uppercase() || c == '_') { + return None; + } + } + // `1.` / `2)` open a numbered list item. + if let Some((num, sep)) = word.split_at_checked(word.len().saturating_sub(1)) { + if (sep == "." || sep == ")") && !num.is_empty() && num.chars().all(|c| c.is_ascii_digit()) + { + return None; + } + } + Some(text) +} + +/// Move as many leading words of `words` as fit into one `prefix`-ed comment +/// line, appended to `output_lines`. At least one word always moves, so a word +/// longer than `avail` gets its own line instead of looping forever. +fn emit_packed_comment_line( + words: &mut Vec, + avail: usize, + prefix: &str, + output_lines: &mut Vec, +) { + let mut line = String::new(); + let mut used = 0; + for word in words.iter() { + if line.is_empty() { + line.push_str(word); + } else if line.len() + 1 + word.len() <= avail { + line.push(' '); + line.push_str(word); + } else { + break; + } + used += 1; + } + words.drain(..used); + output_lines.push(format!("{}{}", prefix, line)); +} + +/// Emit an over-long prose comment, pushing its overflow into the comment +/// lines that follow it rather than leaving a one-word line behind. +/// +/// `content` is the already-normalized text of the comment at `idx`. Every +/// following prose comment line of the same block is consumed (advancing `idx` +/// and `tracker`), absorbs the overflow from above, and passes its own +/// overflow down; whatever is left past the end of the block starts new lines. +fn reflow_comment_block( + content: &str, + logical_lines: &[LogicalLine], + idx: &mut usize, + tracker: &mut ScopeTracker, + depth: usize, + config: &Config, + output_lines: &mut Vec, +) { + let prefix = format!("{}! ", " ".repeat(depth * config.indent_width)); + let avail = if config.line_length > prefix.len() { + config.line_length - prefix.len() + } else { + 40 + }; + + let mut carry: Vec = prose_comment_text(content) + .unwrap_or("") + .split_whitespace() + .map(String::from) + .collect(); + emit_packed_comment_line(&mut carry, avail, &prefix, output_lines); + + while !carry.is_empty() && *idx + 1 < logical_lines.len() { + let next_ll = &logical_lines[*idx + 1]; + let next_kind = classify(&next_ll.joined); + if next_kind != LineKind::Comment || next_ll.raw_lines.len() != 1 { + break; + } + let raw_next = next_ll.joined.trim(); + if prose_comment_text(raw_next).is_none() { + break; + } + let next_content = normalize_comment_content(raw_next, config); + let Some(next_text) = prose_comment_text(&next_content) else { + break; + }; + carry.extend(next_text.split_whitespace().map(String::from)); + *idx += 1; + let _ = tracker.process(next_kind); + emit_packed_comment_line(&mut carry, avail, &prefix, output_lines); + } + + while !carry.is_empty() { + emit_packed_comment_line(&mut carry, avail, &prefix, output_lines); + } +} + /// Wrap a long comment line at word boundaries. /// Preserves the comment marker style (!, !>, !<, etc.) fn wrap_comment(line: &str, max_length: usize, _depth: usize, _indent_width: usize) -> Vec { diff --git a/tests/bugfixes.rs b/tests/bugfixes.rs index 62fc3d8..be3de29 100644 --- a/tests/bugfixes.rs +++ b/tests/bugfixes.rs @@ -786,3 +786,212 @@ end program p\n"; "single-line constructs compounded the indent:\n{out}" ); } + +// --- Comment re-wrap must not orphan the overflow onto its own line (#9) --- + +/// 124 characters; overflows the 132-column limit once indented 12 columns. +const LONG_COMMENT: &str = "! 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"; + +#[test] +fn test_comment_overflow_reflows_into_next_comment_line() { + // Wrapping the body in `#:if` adds one indent level, which pushes the + // comment 4 columns over the limit. The trailing `the` must join the + // following comment line instead of becoming a line of its own. + let src = format!( + "module m\ncontains\n subroutine s(x)\n real, intent(inout) :: x\n \ +#:if SOME_CONDITION\n {LONG_COMMENT}\n ! normal velocity, and x is the normal direction\n \ +x = 1.0\n #:endif\n end subroutine s\nend module m\n" + ); + let out = ffmt::format_string(&src); + assert!( + !out.contains("! the\n"), + "overflow word orphaned onto its own line:\n{out}" + ); + assert!( + out.contains("so vel(1) is\n"), + "first comment line not wrapped at the expected word:\n{out}" + ); + assert!( + out.contains(" ! the normal velocity, and x is the normal direction\n"), + "overflow did not reflow into the next comment line:\n{out}" + ); + assert_eq!( + out.matches('!').count(), + 2, + "comment block gained or lost a line:\n{out}" + ); +} + +#[test] +fn test_comment_overflow_reflow_is_idempotent() { + let src = format!( + "module m\ncontains\n subroutine s(x)\n real, intent(inout) :: x\n \ +#:if SOME_CONDITION\n {LONG_COMMENT}\n ! normal velocity, and x is the normal direction\n \ +x = 1.0\n #:endif\n end subroutine s\nend module m\n" + ); + let once = ffmt::format_string(&src); + let twice = ffmt::format_string(&once); + assert_eq!(once, twice, "reflowed comment is not idempotent"); +} + +#[test] +fn test_comment_overflow_cascades_through_the_block() { + // Every line of the block is over the limit, so each one absorbs the + // overflow from above and passes its own tail down. Only past the last + // line may the leftovers start a new one. + let src = format!( + "program p\n if (x > 0) then\n if (y > 0) then\n {LONG_COMMENT}\n \ +{LONG_COMMENT}\n {LONG_COMMENT}\n x = 1\n end if\n end if\nend program p\n" + ); + let out = ffmt::format_string(&src); + for line in out.lines() { + assert!( + line.len() <= 132, + "line exceeds the limit after reflow:\n{out}" + ); + } + assert!( + out.lines() + .filter(|l| l.trim_start().starts_with('!')) + .all(|l| l.split_whitespace().count() > 2), + "a comment line was left with an orphaned fragment:\n{out}" + ); + let twice = ffmt::format_string(&out); + assert_eq!(out, twice, "cascaded reflow is not idempotent"); +} + +#[test] +fn test_comment_overflow_does_not_reflow_into_a_separator_line() { + // A banner/separator line is not prose, so the overflow must not be + // pushed into it. + let src = format!( + "program p\n if (x > 0) then\n if (y > 0) then\n {LONG_COMMENT}\n \ +! ----------------------------------------\n x = 1\n end if\n end if\nend program p\n" + ); + let out = ffmt::format_string(&src); + assert!( + out.contains("! ----------------------------------------"), + "separator line was rewritten:\n{out}" + ); + assert!( + out.contains("! the\n"), + "overflow should stay on its own line above a separator:\n{out}" + ); +} + +#[test] +fn test_comment_overflow_does_not_consume_ffmt_marker() { + let src = format!( + "program p\n if (x > 0) then\n if (y > 0) then\n {LONG_COMMENT}\n \ +! ffmt off\n x = 1\n ! ffmt on\n end if\n end if\nend program p\n" + ); + let out = ffmt::format_string(&src); + assert!( + out.contains("! ffmt off"), + "ffmt marker was consumed by the comment reflow:\n{out}" + ); + assert!( + out.contains("x = 1"), + "formatting-disabled region was formatted anyway:\n{out}" + ); +} + +#[test] +fn test_comment_overflow_not_reflowed_in_range_mode() { + // Range mode must not rewrite the comment line below the range. + let src = format!( + "program p\n if (x > 0) then\n if (y > 0) then\n {LONG_COMMENT}\n \ +! normal velocity, and x is the normal direction\n x = 1\n end if\n end if\nend program p\n" + ); + let out = ffmt::format_range(&src, 4, 4); + assert!( + out.contains(" ! normal velocity, and x is the normal direction"), + "comment outside the range was modified:\n{out}" + ); +} + +/// Build a program whose over-long comment is followed by `next`. +fn block_after_long_comment(next: &str) -> String { + format!( + "program p\n if (x > 0) then\n if (y > 0) then\n {LONG_COMMENT}\n \ +{next}\n x = 1\n end if\n end if\nend program p\n" + ) +} + +#[test] +fn test_overflow_does_not_absorb_structured_comment_lines() { + // Only running text may absorb overflow. Everything that carries structure + // ends the block, leaving the overflow on its own line as before. + for next in [ + "! TODO: rewrite this loop", + "! NOTE: see the paper", + "! - first item", + "! 1. first step", + "! @param x the thing", + "! ===== Initialization =====", + "! > quoted text", + "! # heading", + ] { + let out = ffmt::format_string(&block_after_long_comment(next)); + assert!( + out.contains("! the\n"), + "overflow was pushed into a structured comment line {next:?}:\n{out}" + ); + assert!( + out.contains(&format!(" {next}\n")), + "structured comment line {next:?} was rewritten:\n{out}" + ); + } +} + +#[test] +fn test_overflow_does_not_absorb_marker_comments() { + // `!&` is a protected Fypp continuation marker and `!DEC$` / `!GCC$` are + // vendor directives. Neither is prose, and master left both alone. + for next in [ + "!& keep me", + "!DEC$ ATTRIBUTES INLINE :: foo", + "!GCC$ unroll 4", + ] { + let out = ffmt::format_string(&block_after_long_comment(next)); + assert!( + out.contains("! the\n"), + "overflow was pushed into marker line {next:?}:\n{out}" + ); + assert!( + !out.contains("! the &") && !out.contains("! the DEC$") && !out.contains("! the GCC$"), + "marker line {next:?} was merged into the prose:\n{out}" + ); + } +} + +#[test] +fn test_overflow_does_not_absorb_unspaced_comment() { + // With space-after-comment off, `!text` keeps its shape and must not be + // pulled into a reflow that would insert the space the user turned off. + let config = ffmt::Config { + space_after_comment: Toggle::Disable, + ..ffmt::Config::default() + }; + let out = ffmt::format_string_with_config(&block_after_long_comment("!unspaced note"), &config); + assert!( + out.contains("!unspaced note"), + "unspaced comment was reflowed despite space-after-comment=false:\n{out}" + ); +} + +#[test] +fn test_overflow_still_absorbs_ordinary_prose() { + // The tightened guard must not block the case the fix exists for. + let out = ffmt::format_string(&block_after_long_comment( + "! normal velocity, and x is the normal direction", + )); + assert!( + !out.contains("! the\n"), + "ordinary prose no longer absorbs the overflow:\n{out}" + ); + assert!( + out.contains("! the normal velocity, and x is the normal direction"), + "overflow did not reflow into the following prose line:\n{out}" + ); +}