Reset list numbers after breaks and headings#106
Conversation
Reviewer's GuideEnhances the list renumbering logic to reset counters on encountering Markdown headings or thematic breaks, improves regex initialization for safety, extends integration tests and data fixtures for the new behavior, and updates the README to reflect these changes. Class diagram for regex and renumbering logic changesclassDiagram
class renumber_lists {
+Vec<String> renumber_lists(lines: &[String])
-counters: Vec<usize>
-prev_blank: bool
}
class HEADING_RE {
+Regex
}
class THEMATIC_BREAK_RE {
+Regex
}
renumber_lists --> HEADING_RE : uses
renumber_lists --> THEMATIC_BREAK_RE : uses
Flow diagram for list renumbering with heading and break resetsflowchart TD
A[Start renumbering lists] --> B{Line is heading or thematic break?}
B -- Yes --> C[Clear list counters]
C --> D[Output line as-is]
D --> E[Continue to next line]
B -- No --> F[Handle paragraph restart and indentation]
F --> G[Output line]
G --> E
E --> B
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughIntroduce logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant renumber_lists
participant HeadingOrBreakDetector
User->>renumber_lists: Process markdown lines
loop For each line
renumber_lists->>HeadingOrBreakDetector: Check if line is heading or break
alt Is heading or break
HeadingOrBreakDetector-->>renumber_lists: true
renumber_lists->>renumber_lists: Clear list counters, output line unchanged
else Not heading or break
HeadingOrBreakDetector-->>renumber_lists: false
renumber_lists->>renumber_lists: Continue normal list renumbering logic
end
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (2)**/*.mdInstructions used from: Sources:
⚙️ CodeRabbit Configuration File **/*.rsInstructions used from: Sources:
⚙️ CodeRabbit Configuration File 🧬 Code Graph Analysis (1)src/lists.rs (1)
🔇 Additional comments (5)
✨ 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 they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/lists.rs:12` </location>
<code_context>
+// Lines starting with optional indentation followed by '#' characters denote
+// Markdown ATX headings. A space or end of line must follow the hashes.
+static HEADING_RE: std::sync::LazyLock<Regex> =
+ std::sync::LazyLock::new(|| Regex::new(r"^[ ]{0,3}#{1,6}(?:\s|$)").unwrap());
+
+// Matches Markdown thematic breaks using '*', '-' or '_' characters.
</code_context>
<issue_to_address>
Consider using a more precise regex for ATX headings.
The current regex also matches headings without a space after the hashes (e.g., '###foo'), which is invalid in CommonMark. Update the regex to require at least one space after the hashes, unless the heading is empty. For example: r"^[ ]{0,3}#{1,6}(?:[ \t]+|$)".
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
static HEADING_RE: std::sync::LazyLock<Regex> =
std::sync::LazyLock::new(|| Regex::new(r"^[ ]{0,3}#{1,6}(?:\s|$)").unwrap());
=======
static HEADING_RE: std::sync::LazyLock<Regex> =
std::sync::LazyLock::new(|| Regex::new(r"^[ ]{0,3}#{1,6}(?:[ \t]+|$)").unwrap());
>>>>>>> REPLACE
</suggested_fix>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 (8)
src/lists.rs(2 hunks)tests/data/renumber_break_heading_restart_expected.txt(1 hunks)tests/data/renumber_break_heading_restart_input.txt(1 hunks)tests/data/renumber_break_restart_expected.txt(1 hunks)tests/data/renumber_break_restart_input.txt(1 hunks)tests/data/renumber_heading_restart_expected.txt(1 hunks)tests/data/renumber_heading_restart_input.txt(1 hunks)tests/lists.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 (9)
tests/data/renumber_break_restart_input.txt (1)
1-8: Test data structure is correct for break restart scenario.The input properly demonstrates a numbered list continuing after a thematic break, which will validate the reset behaviour.
tests/data/renumber_break_restart_expected.txt (1)
1-8: Expected output correctly demonstrates list reset after thematic break.The numbering properly resets to "1. Next" after the break, validating the intended behaviour.
tests/data/renumber_heading_restart_input.txt (1)
1-8: Test data appropriately structured for heading restart validation.The input demonstrates a numbered list continuing after a heading, providing proper test coverage for the reset behaviour.
tests/data/renumber_heading_restart_expected.txt (1)
1-8: Expected output correctly shows list reset after heading.The numbering appropriately resets to "1. Next" following the heading, confirming the desired behaviour.
src/lists.rs (1)
126-131: Logic correctly implements counter reset behaviour.The conditional properly detects headings and thematic breaks, clears counters to reset numbering state, and maintains correct flow control.
tests/data/renumber_break_heading_restart_input.txt (1)
1-10: LGTM: Test input correctly demonstrates the scenario.The test input properly sets up a numbered list interrupted by both a thematic break and heading, with the continuation item incorrectly numbered as "3" to test the renumbering functionality.
tests/data/renumber_break_heading_restart_expected.txt (1)
1-10: LGTM: Expected output correctly demonstrates list restart behaviour.The expected output properly shows the list restarting at "1. Next" after the thematic break and heading, which aligns with the PR objectives.
tests/lists.rs (2)
45-49: LGTM: Function rename improves clarity and expected output is correct.The function name
restart_after_top_headingis more descriptive than the previous name, and the updated expected output correctly demonstrates list restart behaviour after a heading.
115-126: LGTM: New test cases provide comprehensive coverage.The new parameterised test cases properly cover the three scenarios: restart after break, restart after heading, and restart after both break and heading. The use of external test data files follows the established pattern and maintains good separation of test data from test logic.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@sourcery-ai review |
1 participant
Summary
Testing
make fmtmake lintmake testhttps://chatgpt.com/codex/tasks/task_e_687cb8e8fe8483229002165161799248
Summary by Sourcery
Reset list numbering to restart after Markdown headings and thematic breaks and cover the new behavior with integration tests
New Features:
Enhancements:
Documentation:
Tests: