Conversation
Reviewer's GuideRefactor integration tests to leverage the lines_vec! macro for concise Vec initialization, eliminating manual conversions and standardizing macro syntax. File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughUpdate the test code in Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🧬 Code Graph Analysis (1)tests/integration.rs (6)
✨ 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 - here's some feedback:
- Some of the doc comments still use
lines_vec!withString::from(...)wrappers—remove those and pass bare string literals tolines_vec!to keep the examples correct and concise. - The trailing‐comma style in your
lines_vec!invocations is inconsistent—either include a comma after every element list or omit it uniformly to match project formatting conventions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Some of the doc comments still use `lines_vec!` with `String::from(...)` wrappers—remove those and pass bare string literals to `lines_vec!` to keep the examples correct and concise.
- The trailing‐comma style in your `lines_vec!` invocations is inconsistent—either include a comma after every element list or omit it uniformly to match project formatting conventions.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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/integration.rs(27 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (1)
tests/integration.rs (1)
185-190: Macro migration looks soundAll converted test inputs/expectations now use
lines_vec![]. No functional, stylistic, or ownership issues detected.Also applies to: 352-359, 378-384, 642-645, 654-655, 666-670, 678-681, 689-691, 699-701, 708-712, 726-728, 736-738, 750-758, 769-769, 776-779, 783-786, 856-856, 883-892, 910-915, 918-922, 929-932, 938-941, 947-950, 969-997
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/integration.rs(24 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
🔇 Additional comments (1)
tests/integration.rs (1)
1-945: Macro adoption looks solidReplacing the ad-hoc
vec![]constructions withlines_vec![]everywhere tightens the tests and removes boiler-plate. No functional issues spotted.
| let lines = lines_vec!("| A | B | |", "| 1 | 2 | | 3 | 4 |",); | ||
| lines | ||
| } | ||
| fn broken_table() -> Vec<String> { return lines_vec!["| A | B | |", "| 1 | 2 | | 3 | 4 |"]; } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Drop the redundant return in single-expression fixtures
The coding guidelines forbid return in one-liner functions. Strip it to keep idiomatic Rust across all the affected fixtures.
-fn broken_table() -> Vec<String> { return lines_vec![/* … */]; }
+fn broken_table() -> Vec<String> { lines_vec![/* … */] }Apply the same tweak to malformed_table, indented_table, html_table_empty, and html_table_unclosed.
Also applies to: 30-30, 44-44, 118-118, 121-121
🤖 Prompt for AI Agents
In tests/integration.rs at lines 23, 30, 44, 118, and 121, remove the redundant
`return` keyword from the single-expression functions broken_table,
malformed_table, indented_table, html_table_empty, and html_table_unclosed.
Change these functions to directly return the expression without using `return`
to comply with idiomatic Rust style guidelines.
| let input = lines_vec![ | ||
| "# Title", | ||
| String::new(), | ||
| "Para text.".to_string(), | ||
| "Para text.", | ||
| String::new(), | ||
| "| a | b |".to_string(), | ||
| "| 1 | 22 |".to_string(), | ||
| "| a | b |", | ||
| "| 1 | 22 |", | ||
| String::new(), | ||
| "* bullet".to_string(), | ||
| "* bullet", | ||
| String::new(), | ||
| ]; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use "" instead of String::new() for blank lines
Blank lines don’t need an owned String. Supplying "" avoids the extra heap allocation without changing semantics.
"# Title",
- String::new(),
+ "",
"Para text.",
- String::new(),
+ "",
"| a | b |",
"| 1 | 22 |",
- String::new(),
+ "",
"* bullet",
- String::new(),
+ "",📝 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 input = lines_vec![ | |
| "# Title", | |
| String::new(), | |
| "Para text.".to_string(), | |
| "Para text.", | |
| String::new(), | |
| "| a | b |".to_string(), | |
| "| 1 | 22 |".to_string(), | |
| "| a | b |", | |
| "| 1 | 22 |", | |
| String::new(), | |
| "* bullet".to_string(), | |
| "* bullet", | |
| String::new(), | |
| ]; | |
| let input = lines_vec![ | |
| "# Title", | |
| "", | |
| "Para text.", | |
| "", | |
| "| a | b |", | |
| "| 1 | 22 |", | |
| "", | |
| "* bullet", | |
| "", | |
| ]; |
🤖 Prompt for AI Agents
In tests/integration.rs around lines 340 to 350, replace all instances of
String::new() used for blank lines with "" to avoid unnecessary heap allocations
while keeping the same semantics. This change improves efficiency by using
string slices instead of owned Strings for empty lines.
* Handle formatted paragraph restarts (#86) * Define shared formatting markers
1 participant
Summary
lines_vec![]throughout.to_string()and.into_iter().map()Testing
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_6878543dbae4832294706f8f08bece12
Summary by Sourcery
Refactor test code to use the lines_vec! macro for constructing Vec line vectors and remove redundant manual conversions
Enhancements: