Conversation
Reviewer's GuideRefactored Sequence Diagram:
|
| Change | Details | Files |
|---|---|---|
| Improve escaped pipe handling in split_cells |
|
' for | escapes|, push current cell to vector and clear buffer |
| Refine inconsistent row detection in reflow_table |
|
src/lib.rs |
| Defer cell padding and preserve indentation in output |
|
src/lib.rs |
Tips and commands
Interacting with Sourcery
- Trigger a new review: Comment
@sourcery-ai reviewon the pull request. - Continue discussions: Reply directly to Sourcery's review comments.
- Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with@sourcery-ai issueto create an issue from it. - Generate a pull request title: Write
@sourcery-aianywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai titleon the pull request to (re-)generate the title at any time. - Generate a pull request summary: Write
@sourcery-ai summaryanywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment@sourcery-ai summaryon the pull request to
(re-)generate the summary at any time. - Generate reviewer's guide: Comment
@sourcery-ai guideon the pull
request to (re-)generate the reviewer's guide at any time. - Resolve all Sourcery comments: Comment
@sourcery-ai resolveon the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore. - Dismiss all Sourcery reviews: Comment
@sourcery-ai dismisson the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai reviewto trigger a new review!
Customizing Your Experience
Access your dashboard to:
- Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others. - Change the review language.
- Add, remove or edit custom review instructions.
- Adjust other review settings.
Getting Help
- Contact our support team for questions or feedback.
- Visit our documentation for detailed guides and information.
- Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.
WalkthroughThe changes revise the internal logic of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant reflow_table
participant split_cells
User->>reflow_table: Call with markdown table input
reflow_table->>split_cells: Parse each row for cells
split_cells-->>reflow_table: Return split cells (escaped pipes as delimiters)
reflow_table->>reflow_table: Check for split_within_line and row consistency
reflow_table->>reflow_table: Format rows with indentation and separator alignment
reflow_table-->>User: Return formatted table output
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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 - here's some feedback:
- Consider extracting the separator-line formatting into its own helper to reduce code duplication and improve readability.
- Simplify the inconsistent row-length check by comparing each row’s length to the first row instead of building a HashSet of all lengths.
- Add a clarifying comment or rename variables around the
split_cellschange so it’s obvious that\|now acts as a cell separator rather than producing a literal|.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the separator-line formatting into its own helper to reduce code duplication and improve readability.
- Simplify the inconsistent row-length check by comparing each row’s length to the first row instead of building a HashSet of all lengths.
- Add a clarifying comment or rename variables around the `split_cells` change so it’s obvious that `\|` now acts as a cell separator rather than producing a literal `|`.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
🧹 Nitpick comments (1)
src/lib.rs (1)
134-143: Improved handling of inconsistent row lengthsThe refined logic correctly allows processing to continue when splits occur within lines, even if row lengths appear inconsistent. This addresses the PR objective of handling tables with internal splits.
For improved readability, consider extracting the distinct length check:
+ let distinct_row_lengths = cleaned + .iter() + .map(Vec::len) + .collect::<std::collections::HashSet<_>>() + .len(); + if !split_within_line - && cleaned - .iter() - .map(Vec::len) - .collect::<std::collections::HashSet<_>>() - .len() - > 1 + && distinct_row_lengths > 1 { return lines.to_vec(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/lib.rs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: coverage
- GitHub Check: build-test
🔇 Additional comments (3)
src/lib.rs (3)
98-98: Good addition for detecting split patterns within linesThe
split_within_lineboolean correctly identifies when the sentinel regex splits content within lines rather than between lines, which helps handle malformed tables appropriately.
152-162: Excellent: Indentation preservation maintains document structureThe addition of indentation preservation ensures that reflowed tables maintain their position within the document hierarchy, which is particularly important for nested content or consistently formatted documents.
165-194: Well-implemented separator line handling with alignment preservationThe separator line processing correctly:
- Pads to match the column count
- Preserves alignment markers (
:) for left/right/centre alignment- Ensures minimum separator width of 3 characters (markdown standard)
- Handles edge cases when no data rows exist
This implementation successfully addresses the PR objective of ensuring header rows are padded to match column widths.
| if ch == '\\' { | ||
| if let Some(&next) = chars.peek() { | ||
| if next == '|' { | ||
| current.push('|'); | ||
| chars.next(); | ||
| cells.push(current.trim().to_string()); | ||
| current.clear(); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Critical: Incorrect handling of escaped pipe characters
The change to the escape handling logic is incorrect. When \| is encountered in a markdown table cell, it represents a literal pipe character that should be included in the cell content, not a cell delimiter. The current implementation incorrectly treats \| as a cell boundary.
Apply this diff to fix the escape handling:
if let Some(&next) = chars.peek() {
if next == '|' {
chars.next();
- cells.push(current.trim().to_string());
- current.clear();
+ current.push('|');
continue;
}
}This ensures that \| is correctly interpreted as a literal pipe character within the cell content.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ch == '\\' { | |
| if let Some(&next) = chars.peek() { | |
| if next == '|' { | |
| current.push('|'); | |
| chars.next(); | |
| cells.push(current.trim().to_string()); | |
| current.clear(); | |
| continue; | |
| } | |
| if ch == '\\' { | |
| if let Some(&next) = chars.peek() { | |
| if next == '|' { | |
| chars.next(); | |
| current.push('|'); | |
| continue; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/lib.rs around lines 35 to 42, the current code incorrectly treats the
escaped pipe sequence `\|` as a cell delimiter instead of a literal pipe
character. To fix this, modify the escape handling logic so that when `\|` is
encountered, it appends a literal pipe character to the current cell content
rather than splitting the cell. This involves consuming the escape character and
the pipe, then adding a pipe character to the current string instead of pushing
and clearing it.
1 participant
Summary
split_cells\| |splits linesTesting
cargo clippy -- -D warningscargo testhttps://chatgpt.com/codex/tasks/task_e_684c959012d08322990f2437328054da
Summary by Sourcery
Fix table reflow to properly process escaped pipes, maintain consistent column widths with padding for headers and separators, and preserve original indentation
Bug Fixes:
Enhancements:
Summary by CodeRabbit