Conversation
Preserves indentation from orphaned language lines and accepts spaces in comma-separated signifiers.
Summary by CodeRabbit
WalkthroughUpdate the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MarkdownLines
participant attach_orphan_specifiers
User->>MarkdownLines: Provide markdown lines (possibly with orphan specifier)
MarkdownLines->>attach_orphan_specifiers: Pass lines for processing
attach_orphan_specifiers->>attach_orphan_specifiers: Detect orphan specifier line
attach_orphan_specifiers->>attach_orphan_specifiers: Normalise specifiers (trim, lowercase, join with commas)
attach_orphan_specifiers->>attach_orphan_specifiers: Preserve indentation from orphan line if fence is unindented
attach_orphan_specifiers->>MarkdownLines: Return processed lines
MarkdownLines->>User: Output with attached specifiers and correct indentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 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:
⏰ Context from checks skipped due to timeout of 240000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (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 (
|
Reviewer's GuideThis PR refines the orphan specifier attachment in fence processing by preserving indentation, normalizing comma-separated specifiers (with spaces), updates related documentation, and adds tests for these edge cases. Class diagram for updated fence specifier handlingclassDiagram
class Fences {
+compress_fences(lines: &[String]) -> Vec<String>
+attach_orphan_specifiers(lines: &[String]) -> Vec<String>
}
Fences : +Preserves indentation from orphaned specifier lines
Fences : +Normalizes comma-separated specifiers with spaces
Fences : +Removes orphaned specifier lines and applies to following fence
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![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:
- Extract the comma-separated specifier normalization (split, trim, filter, join) into a helper function to improve readability and maintainability.
- Consider adding an explicit example of comma-separated specifiers with spaces in docs/architecture.md to illustrate the new normalization behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the comma-separated specifier normalization (split, trim, filter, join) into a helper function to improve readability and maintainability.
- Consider adding an explicit example of comma-separated specifiers with spaces in docs/architecture.md to illustrate the new normalization behavior.
## Individual Comments
### Comment 1
<location> `tests/fences.rs:184` </location>
<code_context>
assert_eq!(out, lines_vec![" \t```rust", " \tfn main() {}", " \t```"]);
}
+
+#[test]
+fn attaches_orphan_specifier_uses_candidate_indent_when_fence_unindented() {
+ let input = lines_vec![" Rust", "", "```", "fn main() {}", "```"];
+ let out = attach_orphan_specifiers(&compress_fences(&input));
+ assert_eq!(out, lines_vec![" ```rust", "fn main() {}", "```"]);
+}
+
</code_context>
<issue_to_address>
Consider adding a test for mixed indentation between orphan specifier and fence.
Adding tests with varying indentation types or levels between the orphan specifier and the fence (e.g., tabs vs. spaces, or different space counts) will help verify the robustness of the indentation handling.
</issue_to_address>
### Comment 2
<location> `src/fences.rs:91` </location>
<code_context>
if idx > 0 {
- let candidate = out[idx - 1].trim().to_string();
- if ORPHAN_LANG_RE.is_match(&candidate)
+ let candidate_raw = out[idx - 1].as_str();
+ let candidate_trimmed = candidate_raw.trim();
+ let candidate_clean = candidate_trimmed
</code_context>
<issue_to_address>
Consider extracting the specifier normalization and indentation logic into a helper function to simplify the loop body.
```suggestion
You can collapse the “raw → trimmed → cleaned” logic and the indent‐extraction into a small helper, which makes the loop body much flatter. For example:
```rust
/// Returns (cleaned_specifier, leading_indent)
fn normalize_specifier(line: &str) -> (String, String) {
// capture leading whitespace
let indent: String = line.chars().take_while(|c| c.is_whitespace()).collect();
// trim, split on commas, re‐join, lowercase
let cleaned = line
.trim()
.split(',')
.map(str::trim)
.filter(|s| !s.is_empty())
.collect::<Vec<_>>()
.join(",")
.to_lowercase();
(cleaned, indent)
}
// ... inside attach_orphan_specifiers ...
if lang_present.is_empty() {
// back up over blank lines
let mut idx = out.len();
while idx > 0 && out[idx - 1].trim().is_empty() {
idx -= 1;
}
if idx > 0 {
let (candidate_clean, candidate_indent) = normalize_specifier(&out[idx - 1]);
if ORPHAN_LANG_RE.is_match(&candidate_clean)
&& (idx == 1 || out[idx - 2].trim().is_empty())
{
let final_indent = if indent.is_empty() {
&candidate_indent
} else {
indent
};
out.truncate(idx - 1);
out.push(format!("{final_indent}```{}", candidate_clean));
in_fence = true;
continue;
}
}
}
```
This keeps identical behavior (split/trim/filter/join, indent fallback) but moves all the peeling logic into `normalize_specifier`, making the main flow more readable.
</issue_to_address>
### Comment 3
<location> `docs/architecture.md:24` </location>
<code_context>
- `fences::compress_fences` and `attach_orphan_specifiers` normalize code block
- delimiters.
+ delimiters. `attach_orphan_specifiers` keeps indentation from the language
+ line when the fence lacks it and tolerates spaces between comma-separated
+ languages.
</code_context>
<issue_to_address>
Bullet point text exceeds 80 columns and should be wrapped accordingly.
Please wrap the bullet point text so that no line exceeds 80 columns, as per the documentation guidelines.
</issue_to_address>
### Comment 4
<location> `docs/architecture.md:25` </location>
<code_context>
- `fences::compress_fences` and `attach_orphan_specifiers` normalize code block
- delimiters.
+ delimiters. `attach_orphan_specifiers` keeps indentation from the language
+ line when the fence lacks it and tolerates spaces between comma-separated
+ languages.
- `html::convert_html_tables` transforms basic HTML tables into Markdown so \
</code_context>
<issue_to_address>
Bullet point text exceeds 80 columns and should be wrapped accordingly.
Please ensure that each line in the bullet point does not exceed 80 columns for readability and consistency.
</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}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/fences.rs (1)
191-196: Good test for mismatched indentation handling.This test correctly verifies that when the orphan specifier and fence lines have different indentation styles (spaces vs tabs), the fence line retains its original indentation whilst still attaching the specifier. This addresses the past review comment about testing mixed indentation scenarios.
docs/architecture.md (1)
24-26: Wrap the bullet point text to meet the 80-column requirement.The bullet point text exceeds 80 columns and must be wrapped according to the documentation guidelines.
-- `fences::compress_fences` and `attach_orphan_specifiers` normalize code block - delimiters. The latter keeps indentation from the language line when the - fence lacks it. It also tolerates spaces within comma-separated specifiers, - e.g. `TOML, Ini` becomes `toml,ini`. +- `fences::compress_fences` and `attach_orphan_specifiers` normalize code block + delimiters. The latter keeps indentation from the language line when the + fence lacks it. It also tolerates spaces within comma-separated + specifiers, e.g. `TOML, Ini` becomes `toml,ini`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
docs/architecture.md(1 hunks)src/fences.rs(3 hunks)tests/fences.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
docs/**/*.md: Reference: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Update: When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in thedocs/directory to reflect the latest state.
Documentation must use en-GB-oxendict spelling and grammar (with the exception of "license" which is to be left unchanged for community consistency).
Files:
docs/architecture.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks must be wrapped at 120 columns.
Tables and headings must not be wrapped.
Use dashes (-) for list bullets.
Use GitHub-flavoured Markdown footnotes ([^1]) for references.
Files:
docs/architecture.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -our) spelling and grammar
- Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
- Code blocks should be wrapped to 120 columns.
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/architecture.md
**/*.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:
src/fences.rstests/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:
src/fences.rstests/fences.rs
🧬 Code Graph Analysis (1)
tests/fences.rs (1)
src/fences.rs (2)
attach_orphan_specifiers(97-142)compress_fences(56-73)
🪛 GitHub Actions: CI
src/fences.rs
[error] 25-27: Rust compiler error E0603: function normalize_specifier is private but used in doctest at line 25. The function is defined at line 31 but not public, causing doctest compilation failure.
🔇 Additional comments (5)
src/fences.rs (3)
31-42: Excellent implementation of the specifier normalisation logic.The function correctly extracts leading indentation and normalises the specifier by splitting on commas, trimming whitespace, filtering empty strings, rejoining, and converting to lowercase. This addresses the past review comment about extracting this logic into a helper function.
117-127: Good use of the helper function with proper indentation handling.The logic correctly uses
normalize_specifierto process the candidate line and applies the appropriate indentation rules: using the candidate's indentation when the fence is unindented, otherwise preserving the fence's original indentation.
79-81: Documentation accurately reflects the enhanced behaviour.The updated documentation correctly describes that indentation from the specifier line is preserved when the fence lacks indentation, and that specifiers containing spaces are accepted and normalised.
tests/fences.rs (2)
184-189: Excellent test for candidate indentation behaviour.This test properly validates that when a fence line is unindented but the orphan specifier line has indentation, the specifier's indentation is applied to the fence line after attachment.
198-203: Proper test for space-separated specifier normalisation.This test validates that orphan specifiers containing spaces and multiple comma-separated identifiers are correctly normalised by removing spaces and converting to lowercase when attached to the fence line.
1 participant
Summary
Testing
make fmtmake lintmake testmake markdownlintmake nixie(fails: too many arguments)https://chatgpt.com/codex/tasks/task_e_6883f8be90ac8322999b36de77ba4f56
Summary by Sourcery
Improve code fence specifier handling by preserving indent from orphaned language lines and supporting comma-separated specifiers with spaces, update documentation, and add tests for edge cases
Enhancements:
Documentation:
Tests: