Conversation
Reviewer's GuideThis PR refines escaped pipe handling in split_cells to treat “|” as a literal character, introduces a format_separator_cells helper to centralize separator row formatting, simplifies the row-length consistency check in reflow_table, and updates the integration test to align with the new escaped-pipe behavior. Sequence Diagram:
|
| Change | Details | Files |
|---|---|---|
| Treat escaped pipes as literals in split_cells |
|
' directly to the current cell instead of splitting |
| Extract format_separator_cells helper |
|
src/lib.rs |
| Simplify row-length consistency check in reflow_table |
|
src/lib.rs |
| Update integration test for escaped pipe behavior |
|
' inclusion |
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.
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 46 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe changes revise the handling of escaped pipe characters in the Markdown table parser, ensuring they are included in cell content rather than splitting cells. A new helper function for formatting separator rows is introduced, and row length consistency checks are simplified. Corresponding test expectations are updated for improved column alignment. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant reflow_table
participant split_cells
participant format_separator_cells
User->>reflow_table: Call with Markdown table
reflow_table->>split_cells: Split each row into cells (handles escaped pipes)
reflow_table->>format_separator_cells: Format separator row cells for alignment
reflow_table-->>User: Return reflowed table with correct alignment
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 (
|
![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 (2)
src/lib.rs (2)
38-41: Escaped-pipe handling looks correct but silently drops the backslashThe new branch correctly converts
\|to a literal|, but the backslash itself is discarded.
That’s fine for escaped pipes, yet it will also eat the backslash if the input was\\|(intended to render “|”). Consider pushing the backslash when two consecutive backslashes are encountered so that double escapes survive a single pass.
Not critical for the announced PR scope, just a corner-case to keep in mind.
157-163: Simplified consistency check is fine but still O(n²)-ishReplacing the
HashSetwith a first-row comparison is clearer; however theanyscan is still executed for every call. Minor, but you could early-exit on the first mismatch:-if cleaned.iter().any(|row| row.len() != first_len) { +if cleaned[1..].iter().any(|row| row.len() != first_len) {Avoids one needless comparison.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib.rs(4 hunks)tests/integration.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build-test
- GitHub Check: coverage
🔇 Additional comments (1)
tests/integration.rs (1)
97-99: Test expectation conceals a double-split bugThe expected line
| a | b | 1 |implies three cells, yet the source line has only two real cells ("a | b"and"1").
The extra split afteraand beforebwould re-introduce the very bug this PR is fixing. Please verify the golden output withreflow_table; if two cells are returned, adjust the expectation to| a | b |1 |(or whatever spacing emerges).
![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 they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `tests/integration.rs:96` </location>
<code_context>
#[rstest]
fn test_reflow_handles_escaped_pipes(escaped_pipe_table: Vec<String>) {
- let expected = vec!["| X | Y |", "| a | b | 1 |", "| 2 | 3 |"];
+ let expected = vec!["| X | Y |", "| a | b | 1 |", "| 2 | 3 |"];
</code_context>
<issue_to_address>
Consider clarifying the expected output structure and fixture interaction for this test.
A brief comment in the test explaining the fixture's structure and its effect on the output, especially regarding row-length consistency, would make the test easier to understand and maintain.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
1 participant
Summary
Testing
cargo clippy -- -D warningscargo testhttps://chatgpt.com/codex/tasks/task_e_684c9b7b66a48322a60bfe10a1f9976b
Summary by Sourcery
Fix escaped pipe handling in table reflow, refactor separator line formatting into a helper, simplify row-length consistency checks, and update tests for the new behavior
Bug Fixes:
\|) as literal characters in table cell splitting.Enhancements:
format_separator_cellshelper to centralize separator row formatting.reflow_tableby comparing each row length to the first.Tests:
Summary by CodeRabbit
\|) in Markdown tables, ensuring correct cell content and alignment.