Conversation
Reviewer's GuideThis PR refactors the footnote conversion logic into reusable helpers, exposes and documents the optional Class diagram for refactored footnote conversion helpersclassDiagram
class FootnoteConverter {
+convert_inline(text: &str) String
+convert_block(lines: &mut [String])
+convert_footnotes(lines: &[String]) Vec<String>
}
FootnoteConverter : <<helper functions>>
Class diagram for process_stream_opts API with footnotes flagclassDiagram
class ProcessStreamOpts {
+footnotes: bool
+process_stream_opts(input: Stream, opts: ProcessStreamOpts) Output
}
ProcessStreamOpts --> FootnoteConverter : uses if footnotes == true
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughUpdate the README to include a reference link for detailed footnote conversion instructions. Refactor variable names in the footnote conversion implementation for clarity and add a trailing comma in a macro call. Expand unit and integration test coverage, including new tests for inline references, identifier handling, and macro export validation. No changes to public interfaces. Changes
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 🪛 GitHub Actions: CIsrc/footnotes.rs[error] 66-66: Rust compiler error E0425: cannot find value [error] 66-66: Rust compiler error E0425: cannot find value [error] 66-66: Rust compiler error E0425: cannot find value [error] 70-70: Rust compiler error E0425: cannot find value [error] 70-70: Rust compiler error E0425: cannot find value 🔇 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/footnotes.rs:36` </location>
<code_context>
+ format!("{pre}{punc}{style}[^{num}]{boundary}")
+}
+
fn convert_inline(text: &str) -> String {
INLINE_FN_RE
.replace_all(text, |caps: &Captures| {
</code_context>
<issue_to_address>
Consider inlining the footnote parsing and building logic directly into the closure to avoid unnecessary helper functions.
```suggestion
// You can simplify by inlining the parse + build logic back into the closure,
// removing the two trivial helpers `parse_inline_caps` and `build_inline_footnote`.
fn convert_inline(text: &str) -> String {
INLINE_FN_RE
.replace_all(text, |caps: &Captures| {
- let (pre, punc, style, num, boundary) = parse_inline_caps(caps);
- build_inline_footnote(pre, punc, style, num, boundary)
+ format!(
+ "{}{}{}[^{}]{}",
+ &caps["pre"],
+ &caps["punc"],
+ &caps["style"],
+ &caps["num"],
+ &caps["boundary"],
+ )
})
.into_owned()
}
```
After applying this, you can delete the now‐unused `parse_inline_caps` and `build_inline_footnote` helpers. This keeps the logic in one place and reduces indirection without losing any functionality.
</issue_to_address>
### Comment 2
<location> `docs/footnote-conversion.md:3` </location>
<code_context>
+# Footnote Conversion
+
+`mdtablefix` can optionally convert bare numeric references into
+GitHub-flavoured Markdown footnotes. The `convert_footnotes` function performs
+this operation and is exposed via the higher level `process_stream_opts` helper.
</code_context>
<issue_to_address>
Some paragraphs exceed the 80 column wrapping limit.
Lines such as 'GitHub-flavoured Markdown footnotes. The `convert_footnotes` function performs this operation and is exposed via the higher level `process_stream_opts` helper.' exceed 80 columns. Please wrap paragraphs to 80 columns as per the 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.
![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)
README.md(3 hunks)docs/footnote-conversion.md(1 hunks)src/footnotes.rs(2 hunks)src/process.rs(1 hunks)tests/footnotes.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
**/*.rs
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
⚙️ CodeRabbit Configuration File
docs/**/*.md
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- AGENTS.md
🪛 LanguageTool
docs/footnote-conversion.md
[uncategorized] ~9-~9: Use a comma before ‘so’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...put is transformed into a footnote block so the document remains readable on GitHub...
(COMMA_COMPOUND_SENTENCE_2)
🔇 Additional comments (11)
README.md (3)
51-52: Excellent documentation enhancement.The reference link to the detailed footnote conversion guide improves discoverability and provides users with comprehensive information about the feature.
124-124: Good example update demonstrating the footnotes feature.Changing the example to enable footnotes shows users how to use this optional feature in practice.
133-135: Clear and informative parameter documentation.The expanded description properly explains the footnotes parameter's default behaviour and functionality, helping users understand when and how to use it.
src/process.rs (1)
100-105: Excellent API documentation addition.The documentation comment clearly explains the function's purpose and each parameter, following Rust documentation standards. This improves the public API's usability and aligns with the coding guidelines requiring clear documentation for public APIs.
docs/footnote-conversion.md (1)
1-13: Comprehensive and clear documentation.The documentation effectively explains the footnote conversion feature, its purpose, and how to enable it. The explanations are concise yet thorough, making it easy for users to understand the functionality.
tests/footnotes.rs (2)
2-9: Excellent module documentation.The comprehensive documentation clearly explains the purpose of integration tests, distinguishes them from unit tests, and describes the helper macros. This follows the coding guideline requiring module-level comments explaining purpose and utility.
58-62: Good verification of macro exports.The test ensures that the helper macros are properly exported and accessible, which is important for the public API's reliability.
src/footnotes.rs (4)
20-34: Excellent refactoring for clarity.Extracting the regex capture parsing and footnote building into separate helper functions improves readability and follows the single responsibility principle. The functions are well-documented and have clear purposes.
39-40: Clean integration of helper functions.The refactored convert_inline function is now more readable and easier to understand, demonstrating good use of the extracted helper functions.
46-58: Improved variable naming for clarity.The variable renames (
endtotrimmed_end,starttofootnote_start) make the code more self-documenting and easier to understand without changing the logic.
124-148: Comprehensive test coverage enhancements.The new unit tests cover important edge cases including multiple inline references, non-numeric identifiers, empty input, and mixed content with code. This improves the robustness of the footnote conversion functionality.
leynos
force-pushed
the
codex/refactor-convert_inline-and-improve-tests
branch
from
c25803f to
2cee277
Compare
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Consider extracting the trimmed-range logic in convert_block (the rposition/rfind steps for start/end) into a shared helper to reduce duplication and improve readability.
- The new footnote conversion docs would benefit from a minimal before-and-after example directly in docs/footnote-conversion.md to illustrate both inline rewriting and the final block transformation.
- The inline regex could still catch numbers inside code spans or parentheses; consider adding tests and/or adjusting the pattern to explicitly skip backticked sections and common delimiters.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider extracting the trimmed-range logic in convert_block (the rposition/rfind steps for start/end) into a shared helper to reduce duplication and improve readability.
- The new footnote conversion docs would benefit from a minimal before-and-after example directly in docs/footnote-conversion.md to illustrate both inline rewriting and the final block transformation.
- The inline regex could still catch numbers inside code spans or parentheses; consider adding tests and/or adjusting the pattern to explicitly skip backticked sections and common delimiters.
## Individual Comments
### Comment 1
<location> `docs/footnote-conversion.md:4` </location>
<code_context>
+# Footnote Conversion
+
+`mdtablefix` can optionally convert bare numeric references into
+GitHub-flavoured Markdown footnotes. The `convert_footnotes` function performs
+this operation and is exposed via the higher level `process_stream_opts` helper.
+
</code_context>
<issue_to_address>
Consider hyphenating 'higher level' to 'higher-level' as it is a compound adjective.
In this context, 'higher-level' should be hyphenated as it modifies 'helper.'
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
this operation and is exposed via the higher level `process_stream_opts` helper.
=======
this operation and is exposed via the higher-level `process_stream_opts` helper.
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| `mdtablefix` can optionally convert bare numeric references into | ||
| GitHub-flavoured Markdown footnotes. The `convert_footnotes` function performs | ||
| this operation and is exposed via the higher level `process_stream_opts` helper. |
There was a problem hiding this comment.
suggestion (typo): Consider hyphenating 'higher level' to 'higher-level' as it is a compound adjective.
In this context, 'higher-level' should be hyphenated as it modifies 'helper.'
| this operation and is exposed via the higher level `process_stream_opts` helper. | |
| this operation and is exposed via the higher-level `process_stream_opts` helper. |
Address review comment by inlining footnote parsing logic and remove the helpers. Fix grammar in documentation.
leynos
force-pushed
the
codex/refactor-convert_inline-and-improve-tests
branch
from
2cee277 to
a481f2f
Compare
|
B0rked |
1 participant
Summary
convert_blockprocess_stream_optsTesting
make lintmake testhttps://chatgpt.com/codex/tasks/task_e_687cb5d1f5e88322beb126919eaab665
Summary by Sourcery
Refactor and document the footnote conversion feature by reorganizing conversion helpers, updating variable names, and enhancing documentation and examples, while also expanding test coverage for various edge cases and exported macros.
Enhancements:
Documentation:
footnotesparameter in the process_stream_opts APITests: