Conversation
Reviewer's GuideRefactors wrap_preserving_code to preserve whitespace across wrapped lines by introducing a carry-based mechanism, replacing the old non-whitespace span approach, and updating tests to validate the new behavior. Class diagram for updated wrap_preserving_code and LineBufferclassDiagram
class LineBuffer {
+push_token(token: &str)
+push_span(tokens: &[String], start: usize, end: usize)
+flush_into(lines: &mut Vec<String>)
-last_split: Option<usize>
-text: String
}
class wrap_preserving_code {
+wrap_preserving_code(text: &str, width: usize) Vec<String>
-carried_whitespace: String
}
class push_span_with_carry {
+push_span_with_carry(buffer: &mut LineBuffer, tokens: &[String], start: usize, end: usize, carried_whitespace: &mut String)
}
LineBuffer <.. wrap_preserving_code: uses
wrap_preserving_code ..> push_span_with_carry: calls
Class diagram for removal of push_non_whitespace_span from LineBufferclassDiagram
class LineBuffer {
-push_non_whitespace_span(tokens: &[String], start: usize, end: usize) [REMOVED]
}
Flow diagram for whitespace carry logic in wrap_preserving_codeflowchart TD
A[Start wrap_preserving_code]
B{Is token span whitespace?}
C[Accumulate whitespace in carried_whitespace]
D{Can punctuation attach to previous line?}
E[Clear carried_whitespace]
F{Does buffer fit span?}
G[push_span_with_carry]
H[Flush buffer into lines]
I[Flush carried_whitespace if not empty]
J[End]
A --> B
B -- Yes --> C
B -- No --> D
C --> B
D -- Yes --> E
D -- No --> F
E --> B
F -- Yes --> G
F -- No --> H
G --> B
H --> B
B --> I
I --> J
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbitRelease Notes
WalkthroughThe changes implement whitespace carrying across token spans during inline code wrapping. A new helper function merges accumulated leading whitespace with spans, the wrapping logic defers whitespace emission to preserve formatting, and a now-redundant helper function is removed from the line buffer module, with corresponding test coverage added. Changes
Sequence DiagramsequenceDiagram
participant wrap as wrap_preserving_code
participant carry as carried_whitespace
participant push as push_span_with_carry
participant out as output buffer
wrap->>carry: initialise (empty)
loop for each token group
alt group is whitespace
wrap->>carry: accumulate whitespace
wrap->>wrap: advance without emitting
else group is non-whitespace
wrap->>push: merge carry + span
push->>out: emit merged span
push->>carry: clear accumulator
end
end
opt remaining carried_whitespace
wrap->>out: flush as final token
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring attention:
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (2)src/wrap/inline.rs (1)
src/wrap/tests.rs (2)
🔍 Remote MCP DeepwikiSummary of Additional Context for PR #253 ReviewIssue #67 & #80 ContextIssue #67 addresses a bug where trailing spaces—which signify hard breaks in Markdown—were being trimmed during the wrapping process, converting hard breaks into soft ones. The fix ensures that Purpose of
|
This change improves the inline wrapping logic by preserving carried leading whitespace between token spans. It introduces a helper function push_span_with_carry to properly attach whitespace to subsequent tokens during wrapping, ensuring original spacing is maintained. Additionally, tests were added to verify that wrapping preserves whitespace correctly. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
leynos
force-pushed
the
terragon/fix-wrap-preserving-code-whitespace-z08fsj
branch
from
fdcf8e8 to
84cf39a
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and found some issues that need to be addressed.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/wrap/inline.rs:153` </location>
<code_context>
false
}
+fn push_span_with_carry(
+ buffer: &mut LineBuffer,
+ tokens: &[String],
</code_context>
<issue_to_address>
**issue (review_instructions):** Add behavioural and unit tests for the new push_span_with_carry function.
The new function push_span_with_carry introduces new logic for handling token spans and carried whitespace, but there are no corresponding behavioural or unit tests added to verify its correctness. Please add comprehensive tests to cover its expected behaviour and edge cases.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 2
<location> `src/wrap/inline.rs:185` </location>
<code_context>
let mut lines = Vec::new();
let mut buffer = LineBuffer::new();
+ let mut carried_whitespace = String::new();
let mut i = 0;
</code_context>
<issue_to_address>
**issue (review_instructions):** Add tests to demonstrate the new carried_whitespace logic in wrap_preserving_code.
The logic for carrying whitespace between lines in wrap_preserving_code is new and non-trivial. There are no new tests added to demonstrate or verify this behaviour. Please add both behavioural and unit tests to ensure this logic works as intended and to prevent regressions.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*`
**Instructions:**
For any new feature or change to an existing feature, both behavioural *and* unit tests are required.
</details>
</issue_to_address>
### Comment 3
<location> `src/wrap/inline.rs:153` </location>
<code_context>
false
}
+fn push_span_with_carry(
+ buffer: &mut LineBuffer,
+ tokens: &[String],
</code_context>
<issue_to_address>
**issue (review_instructions):** The function `push_span_with_carry` is missing a doc comment, and any attributes should be placed after it.
Please add a doc comment for `push_span_with_carry` and ensure any attributes are placed after the doc comment, as per the instructions.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.rs`
**Instructions:**
Place function attributes **after** doc comments.
</details>
</issue_to_address>
### Comment 4
<location> `src/wrap/inline.rs:153` </location>
<code_context>
false
}
+fn push_span_with_carry(
+ buffer: &mut LineBuffer,
+ tokens: &[String],
</code_context>
<issue_to_address>
**issue (review_instructions):** The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the top of this file to describe its purpose, following the review instructions.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.rs`
**Instructions:**
Every module **must** begin with a `//!` comment.
</details>
</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}
leynos
deleted the
terragon/fix-wrap-preserving-code-whitespace-z08fsj
branch
1 participant
Summary
Changes
Core logic
push_span_with_carryto merge carried whitespace with the current token span when wrapping.carried_whitespacestring while wrapping to preserve spaces between tokens across line breaks.carried_whitespaceinstead of dropping them.carried_whitespaceupon attaching punctuation to the previous line to avoid incorrect carries.Internal buffer
push_non_whitespace_span(no longer used) and rely on the new carry-based logic to push tokens.Tests
wrap_preserving_code_preserves_carry_whitespaceto verify whitespace is preserved when wrapping:beta", width 5 -> ["alpha", "beta"]wrap_preserving_code_glues_punctuation_after_codewrap_text_preserves_hyphenated_wordsTest plan
Notes
push_non_whitespace_span) has been removed in favor of the carry-based approach to ensure correct whitespace preservation across line breaks.🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/25d7d280-989e-4c83-8e7b-17cf2f990e61
Summary by Sourcery
Preserve whitespace when wrapping inline code by carrying leading spaces across lines, remove the outdated non-whitespace span logic, and enhance tests to cover the new behavior.
Bug Fixes:
Enhancements:
push_span_with_carryto merge carried whitespace with token spans.push_non_whitespace_spanfunction in favor of the new carry logic.Tests: