Conversation
Reviewer's GuideRefactors all regex initializations to use expect with descriptive messages instead of unwrap, and adjusts closure formatting to satisfy clippy linting. Class diagram for updated regex initialization patternsclassDiagram
class LazyLock~Regex~ {
+new(fn() -> Regex)
}
class Regex {
+new(pattern: &str) Result<Regex, Error>
}
class Error
LazyLock~Regex~ o-- Regex
Regex --> Error : Result
%% Example static usage
class FENCE_RE {
<<static>>
+Regex
}
class BULLET_RE {
<<static>>
+Regex
}
class FOOTNOTE_RE {
<<static>>
+Regex
}
class BLOCKQUOTE_RE {
<<static>>
+Regex
}
class TABLE_START_RE {
<<static>>
+Regex
}
class TABLE_END_RE {
<<static>>
+Regex
}
class DOT_RE {
<<static>>
+Regex
}
class SENTINEL_RE {
<<static>>
+Regex
}
class SEP_RE {
<<static>>
+Regex
}
FENCE_RE --|> LazyLock~Regex~
BULLET_RE --|> LazyLock~Regex~
FOOTNOTE_RE --|> LazyLock~Regex~
BLOCKQUOTE_RE --|> LazyLock~Regex~
TABLE_START_RE --|> LazyLock~Regex~
TABLE_END_RE --|> LazyLock~Regex~
DOT_RE --|> LazyLock~Regex~
SENTINEL_RE --|> LazyLock~Regex~
SEP_RE --|> LazyLock~Regex~
%% Note: All statics now use expect() instead of unwrap() for error handling
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughRefactor the lazy initialisation of static regex variables across multiple modules by introducing and applying a new Changes
Sequence Diagram(s)sequenceDiagram
participant CallSite as Any Module
participant Macro as lazy_regex! Macro
participant LazyLock as LazyLock<Regex>
participant Regex as Regex Engine
CallSite->>Macro: Invoke lazy_regex!(pattern, message)
Macro->>LazyLock: Create LazyLock with Regex::new(pattern)
LazyLock->>Regex: Compile pattern on first access
Regex-->>LazyLock: Return Regex or panic with message
LazyLock-->>CallSite: Provide Regex reference
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
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 - here's some feedback:
- Most of the diffs repeat the same LazyLock::new(|| Regex::new(...).expect(...)) pattern—consider extracting that into a small helper or macro to cut down on boilerplate.
- You’re fully qualifying std::sync::LazyLock in every file; importing LazyLock at the top of each module (or using a common prelude) would make the initializations more concise.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Most of the diffs repeat the same LazyLock::new(|| Regex::new(...).expect(...)) pattern—consider extracting that into a small helper or macro to cut down on boilerplate.
- You’re fully qualifying std::sync::LazyLock in every file; importing LazyLock at the top of each module (or using a common prelude) would make the initializations more concise.
## Individual Comments
### Comment 1
<location> `src/wrap.rs:24` </location>
<code_context>
+ Regex::new(r"^(\s*(?:>\s*)+)(.*)$").expect("valid blockquote regex")
+});
/// Markdown token emitted by [`tokenize_markdown`].
#[derive(Debug, PartialEq)]
--- a/src/fences.rs
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the very top of the file to describe its purpose, as per the review instructions.
</issue_to_address>
### Comment 2
<location> `src/fences.rs:20` </location>
<code_context>
+ .expect("valid language regex")
});
/// Normalise a potential language specifier.
--- a/src/html.rs
+++ b/src/html.rs
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the very top of the file to describe its purpose, as per the review instructions.
</issue_to_address>
### Comment 3
<location> `src/html.rs:17` </location>
<code_context>
/// Matches the start of an HTML `<table>` tag, ignoring case.
static TABLE_START_RE: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"(?i)^<table(?:\s|>|$)").unwrap());
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the very top of the file to describe its purpose, as per the review instructions.
</issue_to_address>
### Comment 4
<location> `src/ellipsis.rs:15` </location>
<code_context>
- std::sync::LazyLock::new(|| Regex::new(r"\.{3,}").unwrap());
+ std::sync::LazyLock::new(|| Regex::new(r"\.{3,}").expect("valid dot regex"));
/// Replace `...` with `…` outside code spans and fences.
#[must_use]
--- a/src/reflow.rs
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the very top of the file to describe its purpose, as per the review instructions.
</issue_to_address>
### Comment 5
<location> `src/reflow.rs:13` </location>
<code_context>
- std::sync::LazyLock::new(|| Regex::new(r"\|\s*\|\s*").unwrap());
+ std::sync::LazyLock::new(|| Regex::new(r"\|\s*\|\s*").expect("valid sentinel regex"));
pub(crate) fn parse_rows(trimmed: &[String]) -> (Vec<Vec<String>>, bool) {
let raw = trimmed.join(" ");
--- a/src/table.rs
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required.
Please add a `//!` module-level comment at the very top of the file to describe its purpose, as per the review instructions.
</issue_to_address>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}
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
src/breaks.rs(1 hunks)src/ellipsis.rs(1 hunks)src/fences.rs(1 hunks)src/footnotes.rs(1 hunks)src/html.rs(1 hunks)src/lib.rs(1 hunks)src/lists.rs(2 hunks)src/macros.rs(1 hunks)src/reflow.rs(1 hunks)src/table.rs(2 hunks)src/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/macros.rssrc/footnotes.rssrc/reflow.rssrc/ellipsis.rssrc/breaks.rssrc/lib.rssrc/fences.rssrc/lists.rssrc/table.rssrc/wrap.rssrc/html.rs
🧬 Code Graph Analysis (3)
src/reflow.rs (1)
src/table.rs (2)
format_separator_cells(49-73)split_cells(16-47)
src/ellipsis.rs (1)
src/wrap.rs (1)
tokenize_markdown(141-178)
src/breaks.rs (1)
src/wrap.rs (1)
is_fence(259-259)
🔇 Additional comments (24)
src/lib.rs (1)
15-16: LGTM! Clean macro module addition.The
#[macro_use]attribute correctly enables thelazy_regex!macro throughout the crate. The placement after documentation and before other modules follows standard conventions.src/ellipsis.rs (1)
8-8: LGTM! Import addition supports type annotation.The explicit
LazyLockimport is necessary for the type annotation in the static declaration.src/footnotes.rs (1)
7-7: LGTM! Import addition supports type annotations.The explicit
LazyLockimport is necessary for the type annotations in the static declarations.src/macros.rs (3)
1-2: LGTM! Proper module documentation.The module documentation clearly explains its purpose and follows the coding guideline requirement for
//!comments.
3-13: LGTM! Comprehensive macro documentation.The documentation includes a clear example showing usage with
LazyLocktype annotation and demonstrates testing the regex. This follows Rust documentation best practices.
14-19: LGTM! Well-implemented macro solution.The macro perfectly addresses the PR objectives by:
- Replacing
unwrap()withexpect()for better error handling- Standardising lazy regex initialization across the codebase
- Enabling descriptive panic messages
- Following Rust macro conventions with
#[macro_export]The implementation is clean, concise, and type-safe.
src/fences.rs (2)
11-14: LGTM: Clean macro-based regex initialization.The
lazy_regex!macro usage correctly replaces the manualLazyLockinitialization whilst preserving the original regex pattern and providing a descriptive error message.
16-19: LGTM: Consistent regex initialization pattern.The macro usage follows the same pattern as
FENCE_REwith an appropriate descriptive error message for the language regex.src/wrap.rs (5)
7-8: LGTM: Necessary import addition.The explicit
LazyLockimport is required since the type is still used in the static variable declarations.
11-11: LGTM: Clean fence regex initialization.The
lazy_regex!macro usage correctly replaces manual initialization with a descriptive error message.
13-14: LGTM: Consistent bullet regex pattern.Good use of the macro with descriptive error messaging. The trailing comma is acceptable Rust style.
16-17: LGTM: Proper footnote regex initialization.The macro usage follows the established pattern with appropriate error messaging.
19-20: LGTM: Blockquote regex follows pattern.Consistent with the other regex initializations and includes a descriptive error message.
src/lists.rs (3)
3-4: LGTM: Required import for LazyLock type.The explicit import is necessary since
LazyLockis still used in the type annotations.
14-14: LGTM: Heading regex uses consistent pattern.The
lazy_regex!macro correctly replaces manual initialization with a descriptive error message.
17-18: LGTM: Function-scoped regex initialization.The macro usage is correct for the function-scoped static variable with appropriate error messaging.
src/reflow.rs (3)
1-5: LGTM: Proper module documentation.The conversion to
//!doc comments correctly follows Rust documentation conventions and resolves the previous review feedback.
6-6: LGTM: Required LazyLock import.The explicit import is necessary for the type annotation in the static declaration.
12-12: LGTM: Sentinel regex follows established pattern.The
lazy_regex!macro usage is consistent with other files and includes a descriptive error message.src/breaks.rs (3)
3-3: LGTM: Clean import consolidation.Adding
LazyLockto the import allows for cleaner usage without fully qualified paths.
11-14: LGTM: Thematic break regex follows pattern.The
lazy_regex!macro usage correctly replaces manual initialization whilst preserving the complex regex pattern and providing a descriptive error message.
16-16: LGTM: Cleaner LazyLock usage.Using the imported
LazyLockinstead of the fully qualified path improves readability.src/table.rs (2)
7-7: LGTM!The
LazyLockimport remains necessary for the type annotation, even with the macro-based initialisation.
94-94: Excellent refactoring that improves consistency and error handling.The migration to the
lazy_regex!macro provides better error messaging and standardises regex initialisation across the codebase. The descriptive error message "valid separator regex" will aid debugging if the regex compilation fails.
|
|
||
| static DOT_RE: std::sync::LazyLock<Regex> = | ||
| std::sync::LazyLock::new(|| Regex::new(r"\.{3,}").unwrap()); | ||
| static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "valid dot regex"); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error message specificity.
Replace the generic "valid dot regex" with a more descriptive message that explains the regex's purpose.
-static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "valid dot regex");
+static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "ellipsis pattern regex should compile");📝 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.
| static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "valid dot regex"); | |
| -static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "valid dot regex"); | |
| +static DOT_RE: LazyLock<Regex> = lazy_regex!(r"\.{3,}", "ellipsis pattern regex should compile"); |
🤖 Prompt for AI Agents
In src/ellipsis.rs at line 14, the static regex DOT_RE uses a generic
description "valid dot regex" in the lazy_regex macro. Replace this string with
a more descriptive message that clearly explains the regex's purpose, such as
indicating it matches sequences of three or more dots, to improve error message
clarity.
| static INLINE_FN_RE: LazyLock<Regex> = lazy_regex!( | ||
| r"(?P<pre>^|[^0-9])(?P<punc>[.!?);:])(?P<style>[*_]*)(?P<num>\d+)(?P<boundary>\s|$)", | ||
| "valid inline footnote regex", | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error message specificity.
Replace the generic error message with one that describes the regex's purpose.
static INLINE_FN_RE: LazyLock<Regex> = lazy_regex!(
r"(?P<pre>^|[^0-9])(?P<punc>[.!?);:])(?P<style>[*_]*)(?P<num>\d+)(?P<boundary>\s|$)",
- "valid inline footnote regex",
+ "inline footnote reference pattern should compile",
);📝 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.
| static INLINE_FN_RE: LazyLock<Regex> = lazy_regex!( | |
| r"(?P<pre>^|[^0-9])(?P<punc>[.!?);:])(?P<style>[*_]*)(?P<num>\d+)(?P<boundary>\s|$)", | |
| "valid inline footnote regex", | |
| ); | |
| static INLINE_FN_RE: LazyLock<Regex> = lazy_regex!( | |
| r"(?P<pre>^|[^0-9])(?P<punc>[.!?);:])(?P<style>[*_]*)(?P<num>\d+)(?P<boundary>\s|$)", | |
| "inline footnote reference pattern should compile", | |
| ); |
🤖 Prompt for AI Agents
In src/footnotes.rs around lines 11 to 14, the error message for the regex
initialization is generic. Update the error message string to specifically
describe the regex's purpose, such as indicating it is for matching valid inline
footnotes, to improve clarity when errors occur.
| static FOOTNOTE_LINE_RE: LazyLock<Regex> = lazy_regex!( | ||
| r"^(?P<indent>\s*)(?P<num>\d+)\.\s+(?P<rest>.*)$", | ||
| "valid footnote line regex", | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error message specificity.
Replace the generic error message with one that describes the regex's purpose.
static FOOTNOTE_LINE_RE: LazyLock<Regex> = lazy_regex!(
r"^(?P<indent>\s*)(?P<num>\d+)\.\s+(?P<rest>.*)$",
- "valid footnote line regex",
+ "footnote line pattern should compile",
);📝 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.
| static FOOTNOTE_LINE_RE: LazyLock<Regex> = lazy_regex!( | |
| r"^(?P<indent>\s*)(?P<num>\d+)\.\s+(?P<rest>.*)$", | |
| "valid footnote line regex", | |
| ); | |
| static FOOTNOTE_LINE_RE: LazyLock<Regex> = lazy_regex!( | |
| r"^(?P<indent>\s*)(?P<num>\d+)\.\s+(?P<rest>.*)$", | |
| - "valid footnote line regex", | |
| + "footnote line pattern should compile", | |
| ); |
🤖 Prompt for AI Agents
In src/footnotes.rs around lines 16 to 19, the error message for the
FOOTNOTE_LINE_RE regex is generic. Update the error message string to
specifically describe that this regex matches a valid footnote line, including
details like indentation, footnote number, and the rest of the line, to improve
clarity and debugging.
| /// Matches the start of an HTML `<table>` tag, ignoring case. | ||
| static TABLE_START_RE: LazyLock<Regex> = | ||
| LazyLock::new(|| Regex::new(r"(?i)^<table(?:\s|>|$)").unwrap()); | ||
| lazy_regex!(r"(?i)^<table(?:\s|>|$)", "valid table start regex"); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error message specificity.
Replace the generic error message with one that describes the regex's purpose.
static TABLE_START_RE: LazyLock<Regex> =
- lazy_regex!(r"(?i)^<table(?:\s|>|$)", "valid table start regex");
+ lazy_regex!(r"(?i)^<table(?:\s|>|$)", "HTML table start pattern should compile");📝 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.
| lazy_regex!(r"(?i)^<table(?:\s|>|$)", "valid table start regex"); | |
| static TABLE_START_RE: LazyLock<Regex> = | |
| lazy_regex!(r"(?i)^<table(?:\s|>|$)", "HTML table start pattern should compile"); |
🤖 Prompt for AI Agents
In src/html.rs at line 19, the lazy_regex macro uses a generic error message
"valid table start regex". Replace this message with a more descriptive one that
clearly states the regex is intended to match the start of a table tag in HTML,
improving clarity and maintainability.
| lazy_regex!(r"(?i)^<table(?:\s|>|$)", "valid table start regex"); | ||
| /// Matches the end of an HTML `</table>` tag, ignoring case. | ||
| static TABLE_END_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(?i)</table>").unwrap()); | ||
| static TABLE_END_RE: LazyLock<Regex> = lazy_regex!(r"(?i)</table>", "valid table end regex"); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Improve error message specificity.
Replace the generic error message with one that describes the regex's purpose.
-static TABLE_END_RE: LazyLock<Regex> = lazy_regex!(r"(?i)</table>", "valid table end regex");
+static TABLE_END_RE: LazyLock<Regex> = lazy_regex!(r"(?i)</table>", "HTML table end pattern should compile");🤖 Prompt for AI Agents
In src/html.rs at line 21, the error message for the TABLE_END_RE regex is too
generic. Update the error message string to clearly describe the regex's
purpose, such as indicating it matches the closing table tag in a
case-insensitive manner, to improve clarity and debugging.
|
Superseded by #144 |
1 participant
Summary
expectrather thanunwrapwhen constructing regexesTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_6888f708fc6483228ad42b3b2cb97b1f
Summary by Sourcery
Refactor regex construction by replacing unwrap calls with expect and descriptive panic messages, and harmonize lazy_static regex initializers for improved safety and clippy compliance.
Enhancements: