Conversation
Reviewer's GuideRefactor fence specifier attachment in src/fences.rs to preserve existing languages, lowercase orphaned specifiers, simplify orphan regex matching, and update corresponding unit and CLI tests. Class diagram for updated fence specifier logic in fences.rsclassDiagram
class fences {
+attach_orphan_specifiers(lines: &[String]) -> Vec<String>
-FENCE_RE: Regex
-ORPHAN_LANG_RE: Regex
}
fences : attach_orphan_specifiers
fences : FENCE_RE
fences : ORPHAN_LANG_RE
Flow diagram for attach_orphan_specifiers logic updateflowchart TD
A[Start: Iterate lines] --> B{Line matches FENCE_RE?}
B -- No --> C[Push line to output]
B -- Yes --> D{In fence?}
D -- Yes --> E[Set in_fence = false]
D -- No --> F{Language present?}
F -- Yes --> G[Set in_fence = true]
F -- No --> H{Previous line is orphan specifier?}
H -- Yes --> I[Pop orphan, push fence with lowercased orphan]
H -- No --> J[Set in_fence = true]
E --> C
G --> C
I --> C
J --> C
C --> A
A --> K[End]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughStreamline the detection and handling of orphaned language specifiers in fenced code blocks by adopting regex-based fence line matching and stricter language validation. Update test cases to expect lowercased language specifiers and add new tests to ensure existing fences are preserved unaltered. Changes
Estimated code review effort2 (10–30 minutes) Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.rs📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
⚙️ CodeRabbit Configuration File
Files:
🧬 Code Graph Analysis (1)tests/fences.rs (1)
🔇 Additional comments (14)
✨ 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:
- The new ORPHAN_LANG_RE ("^\w+(?:,\w+)*$") no longer allows
+,-, or.in language names, so specifiers likec++orjs-x.ywon’t be attached—consider restoring those characters or otherwise handling them. - You’re running the fence regex against
trimmed, so the captured indent will always be empty—if you need to preserve leading spaces, match against the original line instead of the trimmed one. - When attaching orphan specifiers you always emit exactly three backticks; to avoid changing existing fence styles you might want to reuse the original fence marker and count from the regex capture.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new ORPHAN_LANG_RE ("^\w+(?:,\w+)*$") no longer allows `+`, `-`, or `.` in language names, so specifiers like `c++` or `js-x.y` won’t be attached—consider restoring those characters or otherwise handling them.
- You’re running the fence regex against `trimmed`, so the captured indent will always be empty—if you need to preserve leading spaces, match against the original line instead of the trimmed one.
- When attaching orphan specifiers you always emit exactly three backticks; to avoid changing existing fence styles you might want to reuse the original fence marker and count from the regex capture.
## Individual Comments
### Comment 1
<location> `src/fences.rs:14` </location>
<code_context>
-static ORPHAN_LANG_RE: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^[A-Za-z0-9_+.-]+(?:,[A-Za-z0-9_+.-]+)*$").unwrap());
+static ORPHAN_LANG_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^\w+(?:,\w+)*$").unwrap());
/// Compress backtick fences to exactly three backticks.
</code_context>
<issue_to_address>
The new regex for ORPHAN_LANG_RE is more restrictive than the previous version.
The new regex excludes language specifiers with characters like '+', '.', and '-', which may affect users relying on names such as 'c++' or 'objective-c'. Please confirm if this restriction is intentional.
</issue_to_address>
### Comment 2
<location> `src/fences.rs:91` </location>
<code_context>
+ if ORPHAN_LANG_RE.is_match(&lang_owned) {
out.pop();
- out.push(format!("```{lang}"));
+ out.push(format!("{indent}```{}", lang_owned.to_lowercase()));
in_fence = true;
continue;
</code_context>
<issue_to_address>
Lowercasing the language specifier may cause issues for case-sensitive tools.
Consider whether normalizing the language specifier to lowercase could break compatibility with tools that distinguish between cases, such as 'C' vs 'c' or 'R' vs 'r'.
</issue_to_address>
### Comment 3
<location> `tests/cli.rs:231` </location>
<code_context>
+ let output = Command::cargo_bin("mdtablefix")
+ .expect("Failed to create cargo command for mdtablefix")
+ .arg("--fences")
+ .write_stdin("ruby\n```rust\nfn main() {}\n```\n")
+ .output()
+ .expect("Failed to execute mdtablefix command");
+ assert!(output.status.success());
+ assert_eq!(
+ String::from_utf8_lossy(&output.stdout),
+ "ruby\n```rust\nfn main() {}\n```\n"
);
}
</code_context>
<issue_to_address>
Consider adding a CLI test for orphaned specifiers with extra whitespace or unusual formatting.
Adding tests with leading/trailing whitespace or indented fences will help ensure the CLI handles varied input formats reliably.
</issue_to_address>
### Comment 4
<location> `src/fences.rs:14` </location>
<code_context>
-static ORPHAN_LANG_RE: LazyLock<Regex> =
- LazyLock::new(|| Regex::new(r"^[A-Za-z0-9_+.-]+(?:,[A-Za-z0-9_+.-]+)*$").unwrap());
+static ORPHAN_LANG_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^\w+(?:,\w+)*$").unwrap());
/// Compress backtick fences to exactly three backticks.
</code_context>
<issue_to_address>
The module does not begin with a `//!` comment as required by the review instructions.
Please add a `//!` module-level comment at the top of this 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.
| .write_stdin("ruby\n```rust\nfn main() {}\n```\n") | ||
| .output() | ||
| .expect("Failed to execute mdtablefix command"); | ||
| assert!(output.status.success()); | ||
| assert_eq!( | ||
| String::from_utf8_lossy(&output.stdout), | ||
| "ruby\n```rust\nfn main() {}\n```\n" |
There was a problem hiding this comment.
suggestion (testing): Consider adding a CLI test for orphaned specifiers with extra whitespace or unusual formatting.
Adding tests with leading/trailing whitespace or indented fences will help ensure the CLI handles varied input formats reliably.
|
|
||
| static ORPHAN_LANG_RE: LazyLock<Regex> = | ||
| LazyLock::new(|| Regex::new(r"^[A-Za-z0-9_+.-]+(?:,[A-Za-z0-9_+.-]+)*$").unwrap()); | ||
| static ORPHAN_LANG_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"^\w+(?:,\w+)*$").unwrap()); |
There was a problem hiding this comment.
issue (review_instructions): The module does not begin with a //! comment as required by the review instructions.
Please add a //! module-level comment at the top of this file to describe its purpose, as per the review instructions.
Review instructions:
Path patterns: **/*.rs
Instructions:
Every module must begin with a //! comment.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/fences.rs (1)
64-64: Fix the doctest to match the lowercase conversion behaviour.The doctest assertion expects "
Rust" but the implementation now produces "rust" due to the lowercase normalisation introduced on line 91.Update the doctest assertion:
-/// assert_eq!(fixed[0], "```Rust"); +/// assert_eq!(fixed[0], "```rust");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
src/fences.rs(2 hunks)tests/cli.rs(2 hunks)tests/fences.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Comments must use en-GB-oxendict spelling and grammar.
Function documentation must include clear examples.
Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Clippy warnings MUST be disallowed.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Use predicate functions for conditional criteria with more than two branches.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Comment why, not what. Explain assumptions, edge cases, trade-offs, or complexity. Don't echo the obvious.
Use functions and composition. Avoid repetition by extracting reusable logic. Prefer generators or comprehensions, and declarative code to imperative repetition when readable.
Small, meaningful functions. Functions must be small, clear in purpose, single responsibility, and obey command/query segregation.
Name things precisely. Use clear, descriptive variable and function names. For booleans, prefer names withis,has, orshould.
Structure log...
Files:
tests/fences.rstests/cli.rssrc/fences.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:
tests/fences.rstests/cli.rssrc/fences.rs
🧬 Code Graph Analysis (1)
tests/fences.rs (1)
src/fences.rs (2)
attach_orphan_specifiers(67-106)compress_fences(28-45)
🪛 GitHub Actions: CI
src/fences.rs
[error] 55-55: Doctest failure in function 'attach_orphan_specifiers': assertion left == right failed (left: "rust", right: "Rust"). Test executable failed with exit status 101.
🔇 Additional comments (10)
tests/cli.rs (3)
196-196: Test expectation correctly updated to reflect lowercase normalisation.The change from "Rust" to "rust" aligns with the updated behaviour where orphaned language specifiers are converted to lowercase when attached to fences.
222-224: Test expectation correctly updated for consistency.The expected output properly reflects the lowercase normalisation of the orphaned "Rust" specifier to "rust".
226-239: Valuable test addition for preserving existing fence languages.This test ensures that the
--fencesoption correctly preserves existing language specifiers without modification, which is crucial behaviour. The test structure follows the established pattern and validates the preservation of "rust" whilst the orphaned "ruby" line remains unchanged.tests/fences.rs (6)
50-50: Test assertion correctly updated for lowercase normalisation.The expected output properly reflects the new behaviour where orphaned language specifiers are converted to lowercase.
57-57: Unit test assertion updated consistently.The expectation correctly matches the lowercase normalisation of the "Rust" orphan specifier.
64-64: Blank line handling test updated appropriately.The test correctly expects lowercase "rust" even when there's a blank line between the orphan specifier and fence.
71-71: Combined fence compression and orphan attachment test updated.The assertion properly reflects the lowercase normalisation when both
compress_fencesandattach_orphan_specifiersare applied.
90-96: Multiple orphan specifiers test updated comprehensively.Both language specifiers are correctly expected in lowercase ("rust" and "python"), demonstrating consistent normalisation across multiple orphaned specifiers.
116-121: Essential test addition for fence preservation behaviour.This test validates that
attach_orphan_specifiersdoes not modify existing fenced code blocks with language specifiers. The test properly verifies that "ruby" remains as an orphan line whilst the existing "```rust" fence is preserved unchanged. This behaviour is crucial for maintaining existing fence specifications.src/fences.rs (1)
73-101: Function refactoring improves correctness and control flow.The refactored
attach_orphan_specifiersfunction correctly uses regex captures for fence detection and implements proper state management with explicitcontinuestatements. The lowercase conversion of orphaned specifiers (line 91) provides consistent normalisation. The logic properly preserves existing fence language specifiers by only processing fences wherelang_present.is_empty().
1 participant
Summary
attach_orphan_specifierskeeps existing fence languagesTesting
make fmtmake lintmake testmake markdownlintmake nixiehttps://chatgpt.com/codex/tasks/task_e_687e237a9eb88322aad5e66b106e7da8
Summary by Sourcery
Improve orphaned fence specifier handling by preserving existing languages, lowercasing specifiers, and refining regex detection
Bug Fixes:
Enhancements:
Tests: