Conversation
Reviewer's GuideRefactors diff formatting in format_comment_diff by introducing a helper to crop and align line numbers to four digits and replacing the dual-number display with a single-number display separated by a '|' before the text. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 34 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 (1)
Summary by CodeRabbit
WalkthroughIntroduce a new helper function Changes
Poem
✨ 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.
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/main.rs:564` </location>
<code_context>
- let old_disp = o.map_or(String::from(" "), |n| format!("{n:>4}"));
- let new_disp = n.map_or(String::from(" "), |n| format!("{n:>4}"));
- writeln!(&mut out, "{old_disp} {new_disp} {text}")?;
+ let num = if let Some(num) = n {
+ *num
+ } else if let Some(num) = o {
</code_context>
<issue_to_address>
Consider collapsing the fallback logic for line number display into a one-liner using combinators.
Here’s a minimal tweak to collapse the fallback logic into a one‐liner and keep the same behavior:
```rust
for (o, n, text) in &lines[start..end] {
// prefer new over old, or use four spaces if neither exists
let disp = n
.or(*o)
.map(num_disp)
.unwrap_or_else(|| " ".to_string());
writeln!(&mut out, "{disp}|{text}")?;
}
```
Specifically:
1. Replace the `if let` chain with `n.or(o).map(num_disp).unwrap_or_else(...)`.
2. This removes the manual branching and gives you the same single‐column display.
3. You still get the rightmost‐4‐chars formatting from `num_disp` and the `" "` fallback.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| let old_disp = o.map_or(String::from(" "), |n| format!("{n:>4}")); | ||
| let new_disp = n.map_or(String::from(" "), |n| format!("{n:>4}")); | ||
| writeln!(&mut out, "{old_disp} {new_disp} {text}")?; | ||
| let num = if let Some(num) = n { |
There was a problem hiding this comment.
issue (complexity): Consider collapsing the fallback logic for line number display into a one-liner using combinators.
Here’s a minimal tweak to collapse the fallback logic into a one‐liner and keep the same behavior:
for (o, n, text) in &lines[start..end] {
// prefer new over old, or use four spaces if neither exists
let disp = n
.or(*o)
.map(num_disp)
.unwrap_or_else(|| " ".to_string());
writeln!(&mut out, "{disp}|{text}")?;
}Specifically:
- Replace the
if letchain withn.or(o).map(num_disp).unwrap_or_else(...). - This removes the manual branching and gives you the same single‐column display.
- You still get the rightmost‐4‐chars formatting from
num_dispand the" "fallback.
![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 (1)
src/main.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (1)
src/main.rs (1)
564-576: Excellent improvement to diff readability.The single-column line number display with pipe separator significantly improves readability compared to the previous two-column approach. The logic correctly prioritises new line numbers over old ones and handles edge cases appropriately.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main.rs (1)
516-523: Stop truncating line numbers; this confuses users
The helper keeps only the last four digits (e.g.12345→2345). Users lose the real location. Either display the full number right-aligned, or accompany truncated output with an indicator (e.g.+prefix) so meaning is not lost.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/main.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
| let disp = n | ||
| .or(*o) | ||
| .map(num_disp) | ||
| .unwrap_or_else(|| " ".to_string()); | ||
|
|
||
| writeln!(&mut out, "{disp}|{text}")?; |
There was a problem hiding this comment.
Fix compilation failure in line-number selection
n and o are &Option<i32> so the call to .or(*o) on n does not compile. Dereference or copy before using combinators.
- let disp = n
- .or(*o)
+ let disp = (*n)
+ .or(*o)
.map(num_disp)
.unwrap_or_else(|| " ".to_string());📝 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.
| let disp = n | |
| .or(*o) | |
| .map(num_disp) | |
| .unwrap_or_else(|| " ".to_string()); | |
| writeln!(&mut out, "{disp}|{text}")?; | |
| let disp = (*n) | |
| .or(*o) | |
| .map(num_disp) | |
| .unwrap_or_else(|| " ".to_string()); | |
| writeln!(&mut out, "{disp}|{text}")?; |
🤖 Prompt for AI Agents
In src/main.rs around lines 565 to 570, the code attempts to call `.or(*o)` on
`n` where both `n` and `o` are `&Option<i32>`, causing a compilation error. To
fix this, dereference or clone the `Option<i32>` values before using `.or()`,
for example by using `(*n).or(*o)` or `n.cloned().or(o.cloned())` to combine the
options correctly.
1 participant
Summary
|separatorTesting
cargo clippy -- -D warningsRUSTFLAGS="-D warnings" cargo testmarkdownlint README.md docs/GITHUB_TOKEN.mdnixie README.md docs/GITHUB_TOKEN.mdhttps://chatgpt.com/codex/tasks/task_e_687d675f5e788322821797f295f90031
Summary by Sourcery
Enhancements: