Skip to content

refactor(footnotes): unify blockquote-marker stripping into one canonical helper #357

Description

@lodyai

Background

Blockquote-marker stripping — repeatedly peeling leading > characters and the
whitespace between them — is currently reimplemented in at least three places
with subtly different code:

  • src/footnotes/renumber.rs:71-74 (is_fence_line) —
    while let Some(rest) = trimmed.strip_prefix('>') { trimmed = rest.trim_start(); }
  • src/footnotes/lists.rs:62-65 (has_existing_footnote_block) — the same loop
    over a differently named binding.
  • src/footnotes/renumber.rs:42-54 (matches_definition_prefix) — the same idea
    expressed as a loop with explicit trim_start_matches(char::is_whitespace) and
    an emptiness check.

Problem

Three hand-rolled variants of one concept invite drift: a fix to nested-marker
handling (> > ) or whitespace rules in one site silently leaves the others
behind, and each copy adds cognitive load to the footnotes pipeline. This is a
tactical DRY defect; the broader semantic rework of blockquotes is tracked
separately in #347, but the duplication should be removed regardless.

Objective

Converge on a single canonical implementation and route every call site
through it.

  • Add fn strip_blockquote_markers(s: &str) -> &str at the footnotes module
    root (src/footnotes/mod.rs), returning the input with all leading
    blockquote markers and their surrounding whitespace removed.
  • Rewrite is_fence_line and has_existing_footnote_block to call it.
  • Rewrite matches_definition_prefix as
    strip_blockquote_markers(prefix).is_empty().

Required approach

This refactor must converge on one canonical helper and be verified, per the
project's DRY and testing conventions:

  1. One canonical function. Exactly one strip_blockquote_markers
    definition; remove all duplicate loops and route every caller through it.
  2. Document it. Add an entry to docs/developers-guide.md (Internal API
    reference) naming strip_blockquote_markers, stating its contract (idempotent;
    strips arbitrary-depth > markers and interleaved whitespace) and the rule
    that new blockquote-prefix code must reuse it rather than re-deriving the loop.
  3. Property tests. Add proptest coverage (existing dev-dependency)
    asserting invariants such as: idempotence
    (strip(strip(s)) == strip(s)); the result never starts with >; for an
    input built from n markers plus a known suffix, the suffix is returned
    unchanged.
  4. Model checking (if applicable). The input domain is unbounded UTF-8
    strings, so bounded model checking adds little over property tests; record
    this rationale in the PR rather than adding a Kani harness.
  5. Snapshot assertions. Add insta snapshot tests (existing dev-dependency)
    pinning end-to-end footnote renumbering/list-detection output for
    representative single- and nested-blockquote inputs.

All existing tests must pass; run the full make commit gate before merging.

References

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions