Conversation
Reviewer's GuideThis PR introduces a lazy_regex! macro to streamline regex initialization with custom compile error messages, refactors prefix-based line handling via a PrefixHandler abstraction in wrap_text, and updates documentation formatting via make fmt. Class diagram for PrefixHandler abstraction in wrap.rsclassDiagram
class PrefixHandler {
+&'static LazyLock<Regex> re
+bool is_bq
+fn(&Captures) -> String build_prefix
+usize rest_group
+static fn build_bullet_prefix(&Captures) -> String
+static fn build_footnote_prefix(&Captures) -> String
+static fn build_blockquote_prefix(&Captures) -> String
}
PrefixHandler --> LazyLock
PrefixHandler --> Regex
PrefixHandler --> Captures
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the handling of prefixed lines in Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant wrap_text
participant PrefixHandler
loop For each line
wrap_text->>PrefixHandler: Attempt match with each handler's regex
alt Prefix matches
PrefixHandler-->>wrap_text: Build prefix, extract rest, metadata
wrap_text->>wrap_text: handle_prefix_line(prefix, rest, is_bq)
wrap_text-->>User: Continue to next line
else No matches
wrap_text-->>User: Process as normal line
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🧰 Additional context used📓 Path-based instructions (2)**/*.md📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
**/*.rs📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (1)AGENTS.md (10)Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR Learnt from: CR 🧬 Code Graph Analysis (1)src/ellipsis.rs (1)
🪛 LanguageToolAGENTS.md[locale-violation] ~47-~47: LICENSE must be spelled with a “c” when used as a noun in British English. Use “licence”. (LICENCE_LICENSE_NOUN_SINGULAR) [uncategorized] ~161-~161: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA) 🔇 Additional comments (18)
✨ 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 (
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.
- You could apply the new lazy_regex! macro to FENCE_RE, BULLET_RE, FOOTNOTE_RE, and BLOCKQUOTE_RE as well to keep all your regex statics consistent and DRY.
- The large documentation formatting updates are orthogonal to the regex refactor—consider moving them into a separate commit or PR to keep this diff more focused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You could apply the new lazy_regex! macro to FENCE_RE, BULLET_RE, FOOTNOTE_RE, and BLOCKQUOTE_RE as well to keep all your regex statics consistent and DRY.
- The large documentation formatting updates are orthogonal to the regex refactor—consider moving them into a separate commit or PR to keep this diff more focused.
## Individual Comments
### Comment 1
<location> `AGENTS.md:31` </location>
<code_context>
+ examples demonstrating the usage and outcome of the function. Test
+ documentation should omit examples where the example serves only to reiterate
+ the test logic.
+- **Keep file size managable.** No single code file may be longer than 400
+ lines.
Long switch statements or dispatch tables should be broken up by feature and
</code_context>
<issue_to_address>
Typo: 'managable' should be 'manageable'.
Please correct the spelling to 'manageable'.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
- **Keep file size managable.** No single code file may be longer than 400
lines.
=======
- **Keep file size manageable.** No single code file may be longer than 400
lines.
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/wrap.rs:419` </location>
<code_context>
- let rest = cap.get(2).unwrap().as_str();
- handle_prefix_line(&mut out, &mut buf, &mut indent, width, prefix, rest, true);
- continue;
+ for handler in HANDLERS {
+ if let Some(cap) = handler.re.captures(line) {
+ let prefix = (handler.build_prefix)(&cap);
</code_context>
<issue_to_address>
Consider replacing the PrefixHandler struct and handler loop with three explicit prefix checks for clarity and simplicity.
Here’s a way to collapse the `PrefixHandler` indirection back into three simple, explicit checks—no `struct` or function‐pointer loops, and it keeps all the existing behavior:
```rust
fn wrap_preserving_code(text: &str, width: usize) -> Vec<String> {
use unicode_width::UnicodeWidthStr;
let mut lines = Vec::new();
let mut current = String::new();
let mut current_width = 0;
let mut last_split: Option<usize> = None;
for token in tokenize_inline(text) {
let token_width = UnicodeWidthStr::width(token.as_str());
// only three special prefixes—handle them here, up front:
if current.is_empty() {
// 1) code‐fence backticks
if token.starts_with("```") {
current.push_str(&token);
current_width += token_width;
continue;
}
// 2) blockquote marker
if token == "> " {
lines.push(current.clone());
current.clear();
current_width = 0;
continue;
}
// 3) list item bullet
if token == "- " {
current.push_str(&token);
current_width += token_width;
continue;
}
}
// rest of your existing wrap logic...
if current_width + token_width <= width {
current.push_str(&token);
current_width += token_width;
if token.chars().all(char::is_whitespace) {
last_split = Some(current.len());
}
continue;
}
// etc…
}
lines
}
```
This preserves all three prefix behaviors, keeps the loop linear, and drops the extra indirection.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary
lazy_regex!macro for conciseLazyLockregexesmake fmtto update documentation formattingTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_6888f66a0744832287eda00ac83afb5a
Summary by Sourcery
Introduce a macro to simplify Regex lazy initialization, refactor markdown prefix handling with unified handlers, apply the macro across modules for regex definitions, and update documentation formatting.
New Features:
Enhancements:
Documentation:
Chores: