Conversation
Extract helpers and implement clearer state machine using lookahead to pair orphan specifiers with fences.
Reviewer's GuideThis PR reorganizes the fence specifier attachment logic by pulling out specialized helper functions for detecting and combining specifiers and rewrites the main attach_orphan_specifiers loop as a two‐state peekable iterator state machine, improving clarity and maintainability. Sequence diagram for orphan specifier attachment with peekable state machinesequenceDiagram
participant Iterator
participant StateMachine
participant HelperFunctions
Iterator->>StateMachine: next(line)
StateMachine->>HelperFunctions: is_orphan_specifier(line)
HelperFunctions-->>StateMachine: bool
StateMachine->>Iterator: peek()
Iterator-->>StateMachine: next_line
StateMachine->>HelperFunctions: is_opening_fence_without_specifier(next_line)
HelperFunctions-->>StateMachine: bool
StateMachine->>HelperFunctions: attach_specifier_to_fence(fence, spec, indent)
HelperFunctions-->>StateMachine: combined_line
StateMachine->>Iterator: advance past blanks and fence
StateMachine->>Iterator: next(line)
Note over StateMachine,Iterator: State transitions between LookingForSpecifier and InsideFence
Class diagram for new helper functions in fences.rsclassDiagram
class fences {
+compress_fences(lines: &[String]) Vec<String>
+attach_orphan_specifiers(lines: &[String]) Vec<String>
+is_orphan_specifier(line: &str) bool
+is_opening_fence_without_specifier(line: &str) bool
+attach_specifier_to_fence(fence_line: &str, specifier: &str, spec_indent: &str) String
}
fences <|-- State
class State {
LookingForSpecifier
InsideFence
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 59 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 (2)
✨ Finishing touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Hey there - 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/fences.rs:212` </location>
<code_context>
+ iter.next();
+ }
+ let fence = iter.next().expect("peeked fence");
+ out.push(attach_specifier_to_fence(fence, &spec, &indent));
+ state = State::InsideFence;
continue;
</code_context>
<issue_to_address>
attach_specifier_to_fence may not handle all indentation edge cases.
Please review how final_indent is determined when the fence and specifier have different indentation, as this may cause inconsistent formatting. Update the logic or documentation to address these cases.
</issue_to_address>
### Comment 2
<location> `src/fences.rs:178` </location>
<code_context>
/// ```
#[must_use]
pub fn attach_orphan_specifiers(lines: &[String]) -> Vec<String> {
- let mut out: Vec<String> = Vec::with_capacity(lines.len());
- let mut in_fence = false;
</code_context>
<issue_to_address>
Consider replacing the state machine and peekable iterator with a simple index-driven loop to streamline the function.
Consider dropping the two‐state machine and peekable iterator in favor of a simple index‐driven `while` loop. It lets you
• get rid of `State`, the `peekable().clone()`, and blank counters
• inline your fence‐without‐lang check and reduce helper bloat
For example:
```rust
pub fn attach_orphan_specifiers(lines: &[String]) -> Vec<String> {
let mut out = Vec::with_capacity(lines.len());
let mut i = 0;
while i < lines.len() {
let line = &lines[i];
// detect orphan specifier (inlining `is_orphan_specifier`)
let (spec, indent) = normalize_specifier(line);
if ORPHAN_LANG_RE.is_match(&spec)
&& out.last().map_or(true, |l| l.trim().is_empty())
{
// skip the specifier line
i += 1;
// skip any blank lines
while i < lines.len() && lines[i].trim().is_empty() {
i += 1;
}
// if next is an opening fence without lang
if i < lines.len() {
if let Some(cap) = FENCE_RE.captures(&lines[i]) {
let lang = cap.get(3).map_or("", |m| m.as_str());
if is_null_lang(lang) {
out.push(attach_specifier_to_fence(&lines[i], &spec, &indent));
i += 1;
continue;
}
}
}
// fallback: no fence to attach to
out.push(line.clone());
continue;
}
// default path: just push and advance
out.push(line.clone());
i += 1;
}
out
}
```
This preserves all behavior but removes the nested state machine and helper proliferation.
</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
Testing
make fmtmake lintmake testcloses #109
https://chatgpt.com/codex/tasks/task_e_68c2e577d2d48322a55a0bc6846ce3c0
Summary by Sourcery
Improve readability and robustness of fence specifier attachment by extracting helper functions and employing a state machine in attach_orphan_specifiers.
Enhancements:
Documentation: