Conversation
Reviewer's GuideThis PR introduces a new Class diagram for the new process_tokens helper and its usageclassDiagram
class textproc {
+process_tokens(lines: &[String], f: FnMut(Token, &mut String)) Vec<String>
}
class wrap {
<<module>>
+Token
+tokenize_markdown(input: &str) -> Iterator<Token>
}
class ellipsis {
+replace_ellipsis(lines: &[String]) Vec<String>
}
class footnotes {
+convert_footnotes(lines: &[String]) Vec<String>
}
textproc ..> wrap : uses Token, tokenize_markdown
ellipsis ..> textproc : uses process_tokens
footnotes ..> textproc : uses process_tokens
Class diagram for Token enum and its usage in process_tokensclassDiagram
class Token {
<<enum>>
Text(&str)
Code(&str)
Fence(&str)
Newline
}
textproc ..> Token : processes
ellipsis ..> Token : matches
footnotes ..> Token : matches
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughUpdate the architecture documentation to include a new Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as ellipsis/footnotes
participant TextProc as textproc::process_tokens
participant Tokenizer as wrap::tokenize_markdown
Caller->>TextProc: process_tokens(lines, closure)
TextProc->>Tokenizer: tokenize_markdown(joined_lines)
Tokenizer-->>TextProc: Tokens
loop For each token
TextProc->>TextProc: Apply closure to token, append to output buffer
end
TextProc-->>Caller: Output lines (with trailing blanks preserved)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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:
- Add a unit test for the empty input case in process_tokens to confirm it returns an empty vector.
- Be mindful that joining lines and splitting on '\n' may change trailing blank lines—consider adding tests or handling for trailing newlines to preserve original line structure.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a unit test for the empty input case in process_tokens to confirm it returns an empty vector.
- Be mindful that joining lines and splitting on '\n' may change trailing blank lines—consider adding tests or handling for trailing newlines to preserve original line structure.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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
docs/architecture.md(2 hunks)src/ellipsis.rs(1 hunks)src/footnotes.rs(2 hunks)src/lib.rs(1 hunks)src/textproc.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
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.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
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.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
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.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
src/lib.rssrc/textproc.rssrc/footnotes.rssrc/ellipsis.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/lib.rssrc/textproc.rssrc/footnotes.rssrc/ellipsis.rs
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. Ensure the documentation remains accurate and current.
Files:
docs/architecture.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar. (EXCEPTION: the naming of the "LICENSE" file, which is to be left unchanged for community consistency.)
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 and footnotes.
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
🧠 Learnings (1)
src/footnotes.rs (1)
Learnt from: CR
PR: leynos/mdtablefix#0
File: docs/architecture.md:0-0
Timestamp: 2025-07-29T12:45:00.466Z
Learning: Applies to docs/src/footnotes.rs : The convert_footnotes function should convert bare numeric references into GitHub-flavoured Markdown footnotes, ignoring numbers inside inline code or parentheses, and only process the final contiguous list of numeric references.
🧬 Code Graph Analysis (2)
src/textproc.rs (1)
src/wrap.rs (1)
tokenize_markdown(140-177)
src/ellipsis.rs (1)
src/textproc.rs (1)
process_tokens(39-58)
🔇 Additional comments (12)
src/textproc.rs (4)
1-7: Excellent module documentation follows Rust guidelines.The module-level documentation clearly explains the purpose and utility, adhering to the coding guideline requiring every module to begin with a
//!comment. The explanation of how the helper works (join, tokenise, transform, split) is clear and helpful.
8-8: Import statement is correct and properly scoped.The import correctly brings in the required
Tokentype andtokenize_markdownfunction from the wrap module.
10-42: Comprehensive function documentation with useful example.The Rustdoc comment thoroughly documents the function's behaviour, parameters, and includes a working example. The
#[must_use]attribute is appropriately applied since ignoring the return value would be meaningless.
39-58: Verify trailing blank line preservation
Run the generated test stub intest_trailing_blanks.rsto confirm thatprocess_tokenspreserves trailing blanks:
- Compile and execute the test:
rustc test_trailing_blanks.rs && ./test_trailing_blanks- Ensure
input.len() == result.len()and that the count of trailing empty lines in the result matches the input.
Fix the blank-line handling insrc/textproc.rs:39-58if the assertions fail.src/lib.rs (1)
25-25: Module declaration is correctly positioned and scoped.The
mod textproc;declaration is properly placed in alphabetical order and correctly marked as private for internal use, consistent with the other internal modules likehtmlandreflow.docs/architecture.md (3)
228-231: Module diagram correctly represents the new textproc module.The addition of the
textprocclass with itsprocess_tokens()function accurately represents the new module's interface.
256-256: Dependency arrows correctly show textproc usage.The dotted arrows from
ellipsisandfootnotestotextprocaccurately represent the "uses process_tokens" relationships established by the refactoring.Also applies to: 263-263
268-272: Text description accurately explains module responsibilities.The updated description clearly delineates the responsibilities:
ellipsisfor text normalisation,footnotesfor bare reference conversion, andtextprocfor shared token-processing helpers.src/footnotes.rs (2)
18-18: Import correctly updated for the refactoring.The import statement properly replaces the direct use of
tokenize_markdownwithprocess_tokensfrom the newtextprocmodule, maintaining access to theTokentype.
96-108: Refactoring successfully simplifies token processing logic.The conversion to
process_tokenseliminates the manual joining, tokenisation, iteration, and splitting while preserving the exact same token handling logic. The inline footnote conversion viaconvert_inline(t)and the unchanged token matching patterns ensure behavioural equivalence.src/ellipsis.rs (2)
10-10: Import correctly updated for process_tokens usage.The import statement properly brings in
process_tokensfrom thetextprocmodule while maintaining access to theTokentype needed for pattern matching.
18-36: Refactoring maintains identical ellipsis replacement behaviour.The conversion to
process_tokenspreserves the exact regex replacement logic forToken::Textand unchanged handling of code spans, fenced blocks, and newlines. The streamlined approach eliminates manual string manipulation while maintaining behavioural equivalence.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/textproc.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/textproc.rs
🧬 Code Graph Analysis (1)
src/textproc.rs (1)
src/wrap.rs (1)
tokenize_markdown(140-177)
🔇 Additional comments (2)
src/textproc.rs (2)
55-59: Handle the case where the transformation intentionally appends additional trailing blanks.The current logic only pads when
out_blanks < trailing_blanks. If the closure adds more trailing blank lines than the input had, leave them as-is to respect the caller’s intent. Document this behaviour explicitly in the doc comment.Confirm that this is the desired contract; otherwise adjust the loop to truncate when
out_blanks > trailing_blanks.
38-43: Move#[must_use]directly above the function signature.Place the attribute immediately before
fn process_tokens(after the doc comment) to satisfy the style rule “function attributes after doc comments”.-#[must_use] -pub(crate) fn process_tokens<F>(lines: &[String], mut f: F) -> Vec<String> +#[must_use] +pub(crate) fn process_tokens<F>(lines: &[String], mut f: F) -> Vec<String>Likely an incorrect or invalid review comment.
|
@coderabbitai Have these now been resolved?
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos Yes, both of these issues have been resolved in the current implementation: ✅ Empty input test caseThe #[test]
fn test_empty_input() {
let input: Vec<String> = vec![];
let result = process_tokens(&input, |_token, _output| {});
assert_eq!(result, Vec::<String>::new());
}✅ Trailing blank lines handlingThe // Count trailing blank lines in input
let trailing_blanks_input = lines.iter().rev().take_while(|line| line.is_empty()).count();
// ... processing logic ...
// Preserve trailing blank lines from input
let trailing_blanks_output = result.iter().rev().take_while(|line| line.is_empty()).count();
if trailing_blanks_input > trailing_blanks_output {
for _ in 0..(trailing_blanks_input - trailing_blanks_output) {
result.push(String::new());
}
}Additionally, there's a dedicated test Both concerns have been thoroughly addressed with proper implementation and test coverage. 🏁 Script executed: Length of output: 4907 @leynos Yes, both issues have been thoroughly resolved: ✅ Empty input test caseThe test #[test]
fn empty_input_returns_empty_vector() {
let lines: Vec<String> = Vec::new();
let out = process_tokens(&lines, |_tok, _out| unreachable!());
assert!(out.is_empty());
}✅ Trailing blank lines handlingThe implementation includes comprehensive logic to preserve trailing blank lines:
Additionally, there are two dedicated tests for trailing blank line scenarios:
Both concerns have been comprehensively addressed with robust implementation and thorough test coverage. |
leynos
force-pushed
the
codex/create-process_tokens-function-and-refactor
branch
from
6e3357a to
191360c
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
docs/architecture.md(2 hunks)src/ellipsis.rs(1 hunks)src/footnotes.rs(2 hunks)src/lib.rs(1 hunks)src/textproc.rs(1 hunks)src/wrap.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Clippy warnings MUST be disallowed.
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.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
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.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
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.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Files:
src/lib.rssrc/footnotes.rssrc/wrap.rssrc/textproc.rssrc/ellipsis.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/lib.rssrc/footnotes.rssrc/wrap.rssrc/textproc.rssrc/ellipsis.rs
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. Ensure the documentation remains accurate and current.
Files:
docs/architecture.md
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar. (EXCEPTION: the naming of the "LICENSE" file, which is to be left unchanged for community consistency.)
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 and footnotes.
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
🧠 Learnings (1)
src/footnotes.rs (1)
Learnt from: CR
PR: leynos/mdtablefix#0
File: docs/architecture.md:0-0
Timestamp: 2025-07-29T12:45:00.466Z
Learning: Applies to docs/src/footnotes.rs : The convert_footnotes function should convert bare numeric references into GitHub-flavoured Markdown footnotes, ignoring numbers inside inline code or parentheses, and only process the final contiguous list of numeric references.
🧬 Code Graph Analysis (2)
src/footnotes.rs (1)
src/textproc.rs (1)
process_tokens(40-105)
src/textproc.rs (1)
src/wrap.rs (1)
is_fence(266-266)
🔇 Additional comments (11)
src/textproc.rs (4)
1-8: Module documentation follows Rust conventions well.The module-level documentation provides a clear summary and explains the module's purpose, utility, and key behaviour like trailing blank line preservation.
9-9: Import is appropriate and follows crate structure.Using the
wrapmodule'sTokenenum andis_fencefunction aligns with the existing architecture.
11-46: Function documentation is comprehensive and includes examples.The documentation clearly explains the tokenisation behaviour, parameter expectations, and return value. The example demonstrates proper usage patterns.
107-163: Test coverage addresses key scenarios effectively.The tests verify identity transformation, empty input handling, content removal, and trailing blank line preservation. The test suite appears comprehensive based on the past review comments indicating edge cases were already addressed.
src/lib.rs (1)
32-32: Module declaration follows established patterns.The
textprocmodule is correctly declared as internal alongside other private modules likehtmlandreflow. This maintains the clean separation between public and internal APIs.docs/architecture.md (3)
228-231: Architecture diagram accurately reflects the new module structure.The addition of the
textprocmodule with itsprocess_tokens()function properly documents the new shared utility.
255-255: Dependency relationships correctly updated.The diagram now shows
ellipsisandfootnotesdepending ontextprocinstead of directly onwrapfor tokenisation, which accurately reflects the refactoring.Also applies to: 262-262
267-271: Module descriptions align with implementation.The textual description correctly identifies
textprocas containing shared token-processing helpers used by bothellipsisandfootnotesmodules.src/wrap.rs (1)
74-74: Token enum documentation updated appropriately.The documentation comment correctly reflects that tokens are now emitted by "token-processing helpers" rather than the specific
tokenize_markdownfunction that was removed.src/footnotes.rs (2)
21-21: Import updated correctly to use new textproc module.The change from
tokenize_markdowntoprocess_tokensand addingtextproc::import aligns with the architectural refactoring.
99-108: Token processing refactoring preserves functionality effectively.The refactored code using
process_tokensmaintains the same token handling logic while eliminating manual line joining/splitting. The closure properly handles all token types (Text,Code,Fence,Newline) with equivalent behaviour to the previous implementation.
| use regex::Regex; | ||
|
|
||
| use crate::wrap::{Token, tokenize_markdown}; | ||
| use crate::{textproc::process_tokens, wrap::Token}; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Reduce coupling to wrap by re-exporting Token via textproc.
Importing Token directly from wrap keeps ellipsis tied to an implementation detail that the new textproc helper is trying to abstract away. Re-export Token from the textproc module (e.g. pub use crate::wrap::Token;) and import it here via crate::textproc::Token instead. This makes future refactors (or a replacement tokenizer) localised to textproc.
🤖 Prompt for AI Agents
In src/ellipsis.rs at line 12, the import statement directly imports Token from
the wrap module, creating tight coupling. To fix this, modify the textproc
module to re-export Token by adding 'pub use crate::wrap::Token;' there, then
change the import in ellipsis.rs to import Token from crate::textproc instead of
crate::wrap. This reduces coupling and localizes future tokenizer changes to
textproc.
| process_tokens(lines, |token, out| match token { | ||
| Token::Text(t) => { | ||
| let replaced = DOT_RE.replace_all(t, |caps: ®ex::Captures<'_>| { | ||
| let len = caps[0].len(); | ||
| let ellipses = "…".repeat(len / 3); | ||
| let leftover = ".".repeat(len % 3); | ||
| format!("{ellipses}{leftover}") | ||
| }); | ||
| out.push_str(&replaced); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid unnecessary allocation when no replacement is needed.
Regex::replace_all always allocates, even if the input contains no match. For the common case where a Token::Text chunk has no "...", exit early to spare an allocation and copy:
Token::Text(t) => {
- let replaced = DOT_RE.replace_all(t, |caps: ®ex::Captures<'_>| {
+ if !DOT_RE.is_match(t) {
+ out.push_str(t);
+ return;
+ }
+
+ let replaced = DOT_RE.replace_all(t, |caps: ®ex::Captures<'_>| {
let len = caps[0].len();
let ellipses = "…".repeat(len / 3);
let leftover = ".".repeat(len % 3);
format!("{ellipses}{leftover}")
});
out.push_str(&replaced);
}The early return keeps the hot path allocation-free and silences a forthcoming clippy replaced_with_default lint.
📝 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.
| process_tokens(lines, |token, out| match token { | |
| Token::Text(t) => { | |
| let replaced = DOT_RE.replace_all(t, |caps: ®ex::Captures<'_>| { | |
| let len = caps[0].len(); | |
| let ellipses = "…".repeat(len / 3); | |
| let leftover = ".".repeat(len % 3); | |
| format!("{ellipses}{leftover}") | |
| }); | |
| out.push_str(&replaced); | |
| process_tokens(lines, |token, out| match token { | |
| Token::Text(t) => { | |
| if !DOT_RE.is_match(t) { | |
| out.push_str(t); | |
| return; | |
| } | |
| let replaced = DOT_RE.replace_all(t, |caps: ®ex::Captures<'_>| { | |
| let len = caps[0].len(); | |
| let ellipses = "…".repeat(len / 3); | |
| let leftover = ".".repeat(len % 3); | |
| format!("{ellipses}{leftover}") | |
| }); | |
| out.push_str(&replaced); |
🤖 Prompt for AI Agents
In src/ellipsis.rs around lines 19 to 27, the code uses Regex::replace_all which
always allocates even if no matches are found. To avoid unnecessary allocation,
first check if the input text contains the pattern "..." using a quick contains
or is_match call. If no match is found, push the original text directly to the
output and return early. Only call replace_all if a match is present to minimize
allocations and improve performance.
| out.push('`'); | ||
| out.push_str(c); | ||
| out.push('`'); | ||
| } | ||
| Token::Fence(f) => out.push_str(f), | ||
| Token::Newline => out.push('\n'), |
There was a problem hiding this comment.
Push a newline after fences to preserve line structure.
Token::Fence(f) writes the raw fence text but omits the trailing newline that was present in the original lines slice. This breaks round-tripping for inputs where the fence is on its own line ("```"). Append '\n' to align with the handling in wrap::tokenize_markdown:
- Token::Fence(f) => out.push_str(f),
+ Token::Fence(f) => {
+ out.push_str(f);
+ out.push('\n');
+ },Verify the existing tests still pass and add one that checks for exact line preservation around fenced blocks.
🤖 Prompt for AI Agents
In src/ellipsis.rs around lines 30 to 35, the Token::Fence(f) arm currently
pushes the fence string without a trailing newline, which breaks line structure
preservation. Modify this arm to append a newline character after pushing the
fence string to maintain the original line breaks. After making this change, run
existing tests to ensure no regressions and add a new test case that verifies
exact line preservation for inputs with fenced code blocks on their own lines.
|
|
||
| let trailing_blanks = lines.iter().rev().take_while(|l| l.is_empty()).count(); | ||
| if trailing_blanks == lines.len() { | ||
| return vec![String::new(); lines.len()]; | ||
| } | ||
|
|
||
| let mut out = String::new(); | ||
| let mut in_fence = false; | ||
| let last_idx = lines.len() - 1; | ||
| for (i, line) in lines.iter().enumerate() { | ||
| let trimmed = line.as_str(); | ||
| if is_fence(trimmed) { | ||
| f(Token::Fence(trimmed), &mut out); | ||
| if i < last_idx { | ||
| f(Token::Newline, &mut out); | ||
| } | ||
| in_fence = !in_fence; | ||
| continue; | ||
| } | ||
| if in_fence { | ||
| f(Token::Fence(trimmed), &mut out); | ||
| if i < last_idx { | ||
| f(Token::Newline, &mut out); | ||
| } | ||
| continue; | ||
| } | ||
| let mut rest = trimmed; | ||
| while let Some(pos) = rest.find('`') { | ||
| if pos > 0 { | ||
| f(Token::Text(&rest[..pos]), &mut out); | ||
| } | ||
| if let Some(end) = rest[pos + 1..].find('`') { | ||
| f(Token::Code(&rest[pos + 1..pos + 1 + end]), &mut out); | ||
| rest = &rest[pos + end + 2..]; | ||
| } else { | ||
| f(Token::Text(&rest[pos..]), &mut out); | ||
| rest = ""; | ||
| break; | ||
| } | ||
| } | ||
| if !rest.is_empty() { | ||
| f(Token::Text(rest), &mut out); | ||
| } | ||
| if i < last_idx { | ||
| f(Token::Newline, &mut out); | ||
| } | ||
| } | ||
|
|
||
| if out.is_empty() { | ||
| return Vec::new(); | ||
| } | ||
|
|
||
| let mut result: Vec<String> = out.split('\n').map(str::to_string).collect(); | ||
| let out_blanks = result.iter().rev().take_while(|l| l.is_empty()).count(); | ||
| for _ in out_blanks..trailing_blanks { | ||
| result.push(String::new()); | ||
| } | ||
| result | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Token processing logic is sound but verify fence state handling.
The implementation correctly handles empty input, preserves trailing blank lines, and processes tokens appropriately. However, verify that the fence state tracking correctly handles nested or malformed fence sequences.
🏁 Script executed:
#!/bin/bash
# Description: Search for fence-related logic in tests to ensure comprehensive coverage
# Expected: Find tests that verify fence state handling edge cases
echo "Searching for fence-related test cases:"
rg -A 10 -B 2 "fence" src/textproc.rs
echo -e "\nSearching for fence validation logic in wrap module:"
rg -A 5 -B 2 "is_fence" src/wrap.rsLength of output: 1703
Add fence state handling tests
No existing tests verify transitions of in_fence or malformed fence sequences. Add unit tests covering:
- Opening and closing code fences (
is_fencetoggle) - Nested backticks inside a fence (should remain in_fence)
- Malformed fences (unmatched backticks, language identifiers)
Target locations:
- src/textproc.rs: process_tokens loop around lines 47–105
- src/wrap.rs: is_fence regex in FENCE_RE
Write tests in tests/textproc.rs (or equivalent) to confirm correct token emission and fence state tracking.
🤖 Prompt for AI Agents
In src/textproc.rs between lines 47 and 105, add unit tests in tests/textproc.rs
to verify the fence state handling in the process_tokens function. Write tests
that check the toggling of in_fence when encountering opening and closing fences
using is_fence, ensure nested backticks inside fences do not toggle the fence
state, and handle malformed fences such as unmatched backticks or fences with
language identifiers. Also, include tests that confirm the correct tokens are
emitted during these scenarios to validate fence state tracking and token
processing.
|
Superseded by #150 |
1 participant
Summary
process_tokensprocess_tokensin ellipsis and footnote modulesTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_6888f6ff67208322b8d20d3c1e042e70
Summary by Sourcery
Centralize markdown token processing by introducing a reusable helper and refactor existing modules to leverage it
Enhancements:
Tests: