Keep link reference definitions verbatim during wrap (#292) - #294
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
This PR fixes issue Changes Core implementation
Tests
Documentation
Validation and review
Design/notes
WalkthroughRecognise CommonMark link reference definitions and optional standalone titles, classify them as passthrough BlockKind::LinkReferenceDefinition, integrate a LinkTitleWindow FSM into wrap_text to emit required title lines verbatim, add unit/property/integration tests, update docs and changelog, and wire classification into table-flush logic. ChangesLink reference definition passthrough support
Possibly related issues
Suggested labelsbug, enhancement Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 15❌ Failed checks (15 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideAdds explicit detection and passthrough handling for CommonMark link reference definitions in the wrapping pipeline so they are preserved verbatim, along with targeted unit and integration tests. Flow diagram for passthrough handling of link reference definitionsflowchart TD
A[Input line] --> B[classify_block]
B -->|matches LINK_REF_RE| C[BlockKind::LinkReferenceDefinition]
B -->|other match| D[Other BlockKind]
B -->|no match| E[None]
C --> F[is_passthrough_block]
D --> F
E --> F
F -->|BlockKind::LinkReferenceDefinition| G[Pass through verbatim]
F -->|Heading or MarkdownlintDirective| G
F -->|Other| H[Subject to wrapping]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6d42debca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/wrap/block.rs`:
- Line 44: Add a short comment above the LINK_REF_RE declaration in
src/wrap/block.rs documenting its limitation: explain that the regex
r"^(\s*)(\[[^\]]+\]:\s*)(.*)$" does not handle balanced nested brackets or
escaped brackets in link labels (e.g., "[label [nested]]" or "[\[escaped\]]"),
and note that this is acceptable for issue `#292` and current tests; keep the
regex unchanged but include the explanatory comment for future maintainers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 162e6e8b-404c-48c6-949e-b7c047acfdf2
📒 Files selected for processing (5)
src/wrap.rssrc/wrap/block.rssrc/wrap/tests.rstests/wrap/link_reference_definitions.rstests/wrap/mod.rs
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/wrap/block.rs`:
- Around line 139-148: The function link_ref_needs_title currently infers an
inline title by checking the last character of remainder, which misclassifies
destinations that legitimately end with ')' (e.g., path_(v1)). Change it to
explicitly detect an inline title from the LINK_REF_RE captures instead of the
trailing-character heuristic: use the appropriate capture group(s) produced by
LINK_REF_RE (not the remainder variable) to determine if a title token is
present (quotes, parentheses or single quotes per CommonMark) and return false
only when no title capture exists; remove the matches!(remainder.chars().last(),
...) check and base the boolean on the explicit title capture.
In `@src/wrap/tests.rs`:
- Around line 403-453: Move the link-reference related tests (the functions
wrap_text_preserves_inline_link_reference_title,
wrap_text_preserves_link_reference_title_on_next_line,
wrap_text_reflows_paragraph_after_link_reference_title,
wrap_text_treats_title_after_blank_line_as_prose) out of tests.rs into a new
test submodule file named link_reference_definitions.rs; copy the test functions
and any required imports/attributes (e.g., #[test], #[rstest]) into that new
file, remove the original copies from tests.rs, and then add a module
declaration in the tests.rs test module (e.g., mod link_reference_definitions;)
so the new file is compiled as part of the test suite while keeping files under
400 lines. Ensure function names and attributes remain unchanged so wrap_text is
still referenced correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3911e19-9773-476a-a98f-8bf31defe34b
📒 Files selected for processing (3)
src/wrap.rssrc/wrap/block.rssrc/wrap/tests.rs
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/wrap/tests.rs (1)
404-404:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReduce this file below the 400-line ceiling.
Line 404 still leaves
src/wrap/tests.rsat 406 lines, which breaches the repository limit. Move at least one additional test block into an existing or new submodule.As per coding guidelines, "Files must not exceed 400 lines in length."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/wrap/tests.rs` at line 404, The file exceeds the 400-line limit; move at least one test block out of src/wrap/tests.rs into a submodule to reduce its length. Either relocate a full #[test] function or a related test block into the existing submodule mod link_reference_definitions or create a new #[cfg(test)] submodule (e.g., mod extra_tests) and paste the test(s) there, updating any use/imports or visibility (pub(crate)) so the moved tests compile; run cargo test to verify everything still builds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/wrap/tests.rs`:
- Line 404: The file exceeds the 400-line limit; move at least one test block
out of src/wrap/tests.rs into a submodule to reduce its length. Either relocate
a full #[test] function or a related test block into the existing submodule mod
link_reference_definitions or create a new #[cfg(test)] submodule (e.g., mod
extra_tests) and paste the test(s) there, updating any use/imports or visibility
(pub(crate)) so the moved tests compile; run cargo test to verify everything
still builds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b4c9066b-df25-4b2b-96e9-eafb34fecb5a
📒 Files selected for processing (3)
src/wrap/block.rssrc/wrap/tests.rssrc/wrap/tests/link_reference_definitions.rs
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. ❌ Failed checks (3 warnings)
|
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
f93a367 to
1663fcc
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Recognise CommonMark link reference definitions as a distinct block type and route them through passthrough handling so the reflow pass leaves each definition on its own line. Add regression tests for single and consecutive definitions, mixed paragraphs, and optional titles. Co-authored-by: Cursor <cursoragent@cursor.com>
Note that the link reference regex does not handle nested or escaped brackets in labels, which is acceptable for the current scope. Co-authored-by: Cursor <cursoragent@cursor.com>
Recognise standalone title lines that follow a bare link reference definition per CommonMark spec §4.7 and emit them verbatim alongside the definition URL line. Co-authored-by: Cursor <cursoragent@cursor.com>
Extend LINK_REF_RE with an optional title capture group so link_ref_needs_title no longer misclassifies destinations ending in ')'. Move link reference wrap tests into a dedicated submodule. Co-authored-by: Cursor <cursoragent@cursor.com>
Extract classify_block_detects_markdown_prefixes so tests.rs stays under the 400-line file limit. Co-authored-by: Cursor <cursoragent@cursor.com>
Record the fix in CHANGELOG and users-guide, document block classification helpers in developers-guide, and add proptest coverage for LINK_REF_RE and LINK_TITLE_RE. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep the proptest 1.6 entry from main and regenerate Cargo.lock. Co-authored-by: Cursor <cursoragent@cursor.com>
Alphabetise module declarations in tests.rs to satisfy rustfmt. Co-authored-by: Cursor <cursoragent@cursor.com>
1663fcc to
96b4073
Compare
Cover awaiting_link_title edge cases, add insta snapshot tests for link reference preservation, and cache classify_block per line in wrap_text. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix MD012/no-multiple-blanks before the block classification section. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== src/wrap/block.rs: classify_block signature and internal instantiation ==="
cat src/wrap/block.rs 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== docs/developers-guide.md: lines 155-195 ==="
sed -n '155,195p' docs/developers-guide.md 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== docs/developers-guide.md: LinkReferenceMatcher/LinkTitleWindow in API reference table ==="
rg -n "LinkReferenceMatcher|LinkTitleWindow|API reference|Internal API" docs/developers-guide.md 2>/dev/null || echo "NOT FOUND or no matches"
echo ""
echo "=== src/wrap/tests/link_ref_regex.rs: full content ==="
cat src/wrap/tests/link_ref_regex.rs 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== src/wrap.rs: classify_block calls to see if matcher is injected ==="
grep -n "classify_block\|LinkReferenceMatcher\|link_matcher\|block_kind" src/wrap.rs 2>/dev/null || echo "NOT FOUND"
echo ""
echo "=== src/wrap/block.rs: check for internal LinkReferenceMatcher::production() call ==="
rg -n "LinkReferenceMatcher\|production()" src/wrap/block.rs 2>/dev/null || echo "no matches"Length of output: 22149
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
This branch keeps CommonMark link reference definitions verbatim during the
--wrapreflow pass. The wrap pipeline now recognises[label]: urllines asa distinct block type and routes them through passthrough handling so they are
not collapsed into prose paragraphs or split across lines.
Closes #292.
Review walkthrough
LINK_REF_REregex,BlockKind::LinkReferenceDefinition, and classification order after footnote definitions.Validation
make check-fmt: passedmake lint: passedmake test: passedcoderabbit review --agent: 0 findingsSummary by Sourcery
Preserve CommonMark link reference definitions verbatim during the wrapping pass so they are not reflowed as prose.
New Features:
Tests: