Conversation
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 4 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 make the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant reflow_table
participant split_cells
User->>reflow_table: Provide table lines
reflow_table->>split_cells: Split each line into cells
reflow_table->>reflow_table: Calculate max width per column
reflow_table->>reflow_table: Pad each cell to column width
reflow_table-->>User: Return aligned table lines
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 (
|
Reviewer's GuideThis PR refactors table handling to enforce uniform column widths by computing and applying padding, makes key utilities public for doctests, updates example snippets, and adds integration tests for width consistency. Updated Class Diagram for
|
| Change | Details | Files |
|---|---|---|
| Expose split_cells for external use |
|
src/lib.rs |
| Refactor reflow_table to pad columns uniformly |
|
src/lib.rs |
| Correct and extend doctest examples |
|
src/lib.rs |
| Add integration tests for uniform width behavior |
|
tests/integration.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.
![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 merging the row-cleanup and width-calculation loops into one pass to reduce temporary allocations and simplify the logic.
- Current padding uses String.len(), which can miscompute widths for multi-byte (Unicode) characters—consider using a unicode-width library for accurate alignment.
- The two integration tests duplicate the same column-width assertion logic; extracting that into a shared helper would make them more concise and maintainable.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider merging the row-cleanup and width-calculation loops into one pass to reduce temporary allocations and simplify the logic.
- Current padding uses String.len(), which can miscompute widths for multi-byte (Unicode) characters—consider using a unicode-width library for accurate alignment.
- The two integration tests duplicate the same column-width assertion logic; extracting that into a shared helper would make them more concise and maintainable.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
🔭 Outside diff range comments (1)
src/lib.rs (1)
86-95:⚠️ Potential issue
max_colscalculation silently discards blank cells, shifting column indexesCounting only non-empty cells (
filter(|c| !c.is_empty())) means a legitimately empty column (e.g.| foo | | bar |) is ignored.
If any row contains an empty cell in the middle, the column indexes of everything to the right shift left, producing mis-aligned output or false “inconsistent column” rejections.- let max_cols = rows - .iter() - .map(|r| r.iter().filter(|c| !c.is_empty()).count()) + // Count every cell, even if it is empty, to preserve column positions + let max_cols = rows + .iter() + .map(|r| r.len()) .max() .unwrap_or(0);The subsequent consistency check (
count != 0 && count != max_cols) must use the same logic:- let count = r.iter().filter(|c| !c.is_empty()).count(); + let count = r.len();
🧹 Nitpick comments (3)
tests/integration.rs (3)
123-125: Use the direct capture informat!to silence Clippy- writeln!(f, "{}", line).unwrap(); + writeln!(f, "{line}").unwrap();This removes the needless temporary string formatting and fixes the CI failure.
🧰 Tools
🪛 GitHub Actions: CI
[error] 124-124: Clippy error: variables can be used directly in the
format!string. Suggested fix: changewriteln!(f, "{}", line).unwrap();towriteln!(f, "{line}").unwrap();.
147-151: Redundant closure – replace withstr::len- .map(|c| c.len()) + .map(str::len)Applying this quiets Clippy and reads better.
🧰 Tools
🪛 GitHub Actions: CI
[error] 150-150: Clippy error: redundant closure detected. Suggested fix: replace
.map(|c| c.len())with.map(str::len).
171-175: Same optimisation applies here:- .map(|c| c.len()) + .map(str::len)🧰 Tools
🪛 GitHub Actions: CI
[error] 174-174: Clippy error: redundant closure detected. Suggested fix: replace
.map(|c| c.len())with.map(str::len).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib.rs(5 hunks)tests/integration.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/integration.rs (1)
src/lib.rs (1)
reflow_table(57-126)
🪛 GitHub Actions: CI
tests/integration.rs
[error] 124-124: Clippy error: variables can be used directly in the format! string. Suggested fix: change writeln!(f, "{}", line).unwrap(); to writeln!(f, "{line}").unwrap();.
[error] 150-150: Clippy error: redundant closure detected. Suggested fix: replace .map(|c| c.len()) with .map(str::len).
[error] 174-174: Clippy error: redundant closure detected. Suggested fix: replace .map(|c| c.len()) with .map(str::len).
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: coverage
leynos
force-pushed
the
codex/uniformly-pad-table-columns
branch
from
ba45a3e to
954fb00
Compare
1 participant
Summary
split_cellsfor doctestsTesting
cargo clippy -- -D warningscargo testhttps://chatgpt.com/codex/tasks/task_e_684c6cef66708322aba3799b34b2a8f8
Summary by Sourcery
Pad Markdown table columns to uniform width in reflow_table, expose split_cells publicly, update doctest imports, and add integration tests to verify uniform column widths.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit