Conversation
Reviewer's GuideThis PR enhances wrap_text to support wrapping of footnote definitions with proper indentation by introducing a FOOTNOTE_RE regex and corresponding handling logic, and adds an integration test to ensure multi-line footnotes are wrapped correctly. Class diagram for updated wrap_text footnote handlingclassDiagram
class wrap_text {
+wrap_text(lines: &[String], width: usize) Vec<String>
}
class FOOTNOTE_RE {
+Regex: LazyLock<Regex>
}
wrap_text ..> FOOTNOTE_RE : uses
Flow diagram for footnote wrapping logic in wrap_textflowchart TD
A[Start wrap_text] --> B{Line matches FOOTNOTE_RE?}
B -- No --> C[Other wrapping logic]
B -- Yes --> D[Flush current paragraph]
D --> E[Clear buffer and indent]
E --> F[Extract prefix and rest]
F --> G[Wrap rest with wrap_preserving_code]
G --> H{First line?}
H -- Yes --> I[Output prefix + line]
H -- No --> J[Output spaces + line]
I & J --> K[Continue]
C --> K
K --> L[Next line or End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughExtend the text wrapping logic to recognise and handle markdown footnote definitions. Add a static regular expression to detect footnote lines, update the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant wrap_text
participant FOOTNOTE_RE
Caller->>wrap_text: Pass lines, width
loop For each line
wrap_text->>FOOTNOTE_RE: Check if line matches footnote pattern
alt Footnote match
wrap_text->>wrap_text: Flush buffer, clear indentation
wrap_text->>wrap_text: Wrap content after prefix with indentation
wrap_text->>Caller: Output wrapped footnote lines
else Not a footnote
wrap_text->>wrap_text: Continue standard wrapping logic (including bullet lists)
end
end
wrap_text->>Caller: Return wrapped lines
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
- Consider refactoring the footnote wrapping logic to reuse your existing list-item wrapping function instead of duplicating almost identical code.
- Ensure your FOOTNOTE_RE handles optional leading indentation and more complex identifiers (e.g. underscores, hyphens) by extending the regex or normalizing the prefix capture.
- Add a test for footnote definitions that include inline code or other Markdown spans to verify wrap_preserving_code is applied correctly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider refactoring the footnote wrapping logic to reuse your existing list-item wrapping function instead of duplicating almost identical code.
- Ensure your FOOTNOTE_RE handles optional leading indentation and more complex identifiers (e.g. underscores, hyphens) by extending the regex or normalizing the prefix capture.
- Add a test for footnote definitions that include inline code or other Markdown spans to verify wrap_preserving_code is applied correctly.
## Individual Comments
### Comment 1
<location> `src/lib.rs:506` </location>
<code_context>
continue;
}
+ if let Some(cap) = FOOTNOTE_RE.captures(line) {
+ flush_paragraph(&mut out, &buf, &indent, width);
+ buf.clear();
+ indent.clear();
+ let prefix = cap.get(1).unwrap().as_str();
+ let rest = cap.get(2).unwrap().as_str().trim();
+ let spaces = " ".repeat(prefix.len());
+ for (i, l) in wrap_preserving_code(rest, width - prefix.len())
+ .iter()
+ .enumerate()
+ {
+ if i == 0 {
+ out.push(format!("{prefix}{l}"));
+ } else {
+ out.push(format!("{spaces}{l}"));
+ }
+ }
+ continue;
+ }
+
</code_context>
<issue_to_address>
The logic for handling footnote lines may not preserve leading whitespace for indented footnotes.
Using only the prefix length to generate spaces may misalign wrapped lines if the original indentation used mixed whitespace. Preserving the exact original indentation could improve alignment.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
if let Some(cap) = FOOTNOTE_RE.captures(line) {
flush_paragraph(&mut out, &buf, &indent, width);
buf.clear();
indent.clear();
let prefix = cap.get(1).unwrap().as_str();
let rest = cap.get(2).unwrap().as_str().trim();
let spaces = " ".repeat(prefix.len());
for (i, l) in wrap_preserving_code(rest, width - prefix.len())
.iter()
.enumerate()
{
if i == 0 {
out.push(format!("{prefix}{l}"));
} else {
out.push(format!("{spaces}{l}"));
}
}
continue;
}
=======
if let Some(cap) = FOOTNOTE_RE.captures(line) {
flush_paragraph(&mut out, &buf, &indent, width);
buf.clear();
indent.clear();
let prefix = cap.get(1).unwrap().as_str();
let rest = cap.get(2).unwrap().as_str().trim();
// Extract the exact leading whitespace from the prefix
let orig_indent = prefix
.chars()
.take_while(|c| c.is_whitespace())
.collect::<String>();
// For wrapped lines, use the same indentation as the prefix
let wrapped_indent = format!(
"{}{}",
orig_indent,
" ".repeat(prefix.trim_start().len())
);
for (i, l) in wrap_preserving_code(rest, width - prefix.len())
.iter()
.enumerate()
{
if i == 0 {
out.push(format!("{prefix}{l}"));
} else {
out.push(format!("{wrapped_indent}{l}"));
}
}
continue;
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/lib.rs:510` </location>
<code_context>
+ flush_paragraph(&mut out, &buf, &indent, width);
+ buf.clear();
+ indent.clear();
+ let prefix = cap.get(1).unwrap().as_str();
+ let rest = cap.get(2).unwrap().as_str().trim();
+ let spaces = " ".repeat(prefix.len());
+ for (i, l) in wrap_preserving_code(rest, width - prefix.len())
</code_context>
<issue_to_address>
Trimming the footnote content may remove intentional leading/trailing spaces.
Using .trim() here may unintentionally remove meaningful whitespace from footnote content. Please verify if this is required for your use case.
</issue_to_address>
### Comment 3
<location> `src/lib.rs:513` </location>
<code_context>
+ let prefix = cap.get(1).unwrap().as_str();
+ let rest = cap.get(2).unwrap().as_str().trim();
+ let spaces = " ".repeat(prefix.len());
+ for (i, l) in wrap_preserving_code(rest, width - prefix.len())
+ .iter()
+ .enumerate()
+ {
+ if i == 0 {
+ out.push(format!("{prefix}{l}"));
+ } else {
+ out.push(format!("{spaces}{l}"));
+ }
+ }
+ continue;
</code_context>
<issue_to_address>
Subtracting prefix length from width may cause issues if prefix contains non-printable characters.
Since .len() returns the byte length, not the display width, this can misalign output for prefixes with tabs or multi-byte characters. Use a display width calculation to ensure correct wrapping.
Suggested implementation:
```rust
use unicode_width::UnicodeWidthStr;
let prefix = cap.get(1).unwrap().as_str();
let rest = cap.get(2).unwrap().as_str().trim();
let prefix_width = UnicodeWidthStr::width(prefix);
let spaces = " ".repeat(prefix_width);
```
```rust
for (i, l) in wrap_preserving_code(rest, width - prefix_width)
```
Make sure the `unicode-width` crate is included in your `Cargo.toml` dependencies:
```toml
[dependencies]
unicode-width = "0.1"
```
If the `use unicode_width::UnicodeWidthStr;` import is already present at the top of the file, you can omit it from the inner scope.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib.rs(2 hunks)tests/integration.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🧬 Code Graph Analysis (1)
tests/integration.rs (2)
src/lib.rs (1)
process_stream(746-746)tests/common/mod.rs (1)
assert_wrapped_list_item(17-53)
🔇 Additional comments (2)
src/lib.rs (1)
229-230: LGTM: Regex pattern correctly matches footnote definitions.The pattern properly captures footnote markers like
[^note]: contentwith appropriate grouping for prefix and content extraction.tests/integration.rs (1)
719-728: LGTM: Test properly validates footnote wrapping functionality.The test correctly verifies that long footnote definitions wrap with proper indentation, following the same pattern as list item tests.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/lib.rs (1)
516-525: Ensure footnote content trimming is intentional.The footnote content is trimmed with
.trim()on line 524, which could remove meaningful whitespace from footnote definitions. Past review comments raised this concern.Verify that trimming footnote content doesn't affect legitimate use cases where leading or trailing spaces are significant.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib.rs(3 hunks)tests/integration.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (4)
src/lib.rs (2)
406-424: Addresses past review comments on Unicode width handling.The helper function correctly uses
unicode_width::UnicodeWidthStrto calculate display widths, addressing the previous concern about byte length vs display width calculations. The implementation properly handles mixed whitespace in prefixes.
512-514: Addresses code duplication through helper function.The refactoring to use
append_wrapped_with_prefixeliminates the duplication previously flagged between bullet and footnote handling logic.tests/integration.rs (2)
732-743: Test covers inline code preservation in footnotes.The test properly verifies that inline code spans are preserved during footnote wrapping, which is crucial for maintaining markdown structure.
719-730: Drop incorrect indentation level assumptionThe
expectedargument denotes the number of wrapped lines (2), not two spaces of indentation. The prefix[^note]:is 9 characters long, so wrapped lines correctly indent by 9 spaces. No changes required.Likely an incorrect or invalid review comment.
|
@sourcery-ai resolve |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/lib.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (2)
src/lib.rs (2)
229-230: LGTM! Regex pattern correctly updated.The footnote regex pattern now properly allows any character except the closing bracket in footnote labels, addressing the previous concern about being too restrictive compared to the Markdown specification.
406-424: Excellent helper function implementation.The helper function properly addresses previous concerns about Unicode width handling and code duplication. The implementation correctly:
- Uses
UnicodeWidthStr::width()for accurate display width calculations- Extracts leading whitespace from the prefix for proper indentation
- Creates consistent indentation for wrapped lines
- Provides reusable logic for both bullet lists and footnotes
1 participant
Summary
Testing
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_6876cdda278c83228287b9522ef84593
Summary by Sourcery
Add support for wrapping footnote definitions with proper indentation in text wrapping functionality
Enhancements:
Tests: