Skip to content

Add footnote conversion support#104

Merged
leynos merged 3 commits intomainfrom
codex/implement-footnote-conversion-feature
Jul 20, 2025
Merged

Add footnote conversion support#104
leynos merged 3 commits intomainfrom
codex/implement-footnote-conversion-feature

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jul 20, 2025

Summary

  • implement convert_footnotes and integrate with processing pipeline
  • expose new --footnotes CLI flag
  • update docs and README
  • add unit and behavioural tests for footnotes

Testing

  • make fmt
  • make lint
  • make test

https://chatgpt.com/codex/tasks/task_e_687b918394188322916bb716f1fa99af

Summary by Sourcery

Add support for converting bare numeric footnote references into GitHub-style footnote links and blocks

New Features:

  • Introduce a new footnotes module with convert_footnotes to transform bare numeric references into Markdown footnote syntax
  • Integrate footnote conversion into the processing pipeline and extend process_stream APIs to accept a footnotes flag
  • Expose a --footnotes CLI option to enable footnote conversion during Markdown processing

Documentation:

  • Update README, library docs, and module relationship diagrams to document footnote support and the new CLI flag

Tests:

  • Add unit tests for inline and block footnote conversion
  • Add integration and CLI tests to verify the --footnotes option and module behavior

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 20, 2025

Summary by CodeRabbit

  • New Features

    • Added a new command-line option (--footnotes) to convert numeric references and numbered lists into GitHub-flavoured Markdown footnote links.
    • Introduced footnote conversion functionality to the processing pipeline.
  • Documentation

    • Updated user documentation to describe the new footnote conversion option and its usage.
    • Enhanced module relationship diagrams and descriptions to include the new footnotes module.
  • Bug Fixes

    • Ensured that footnote conversion is idempotent and avoids false positives.
  • Tests

    • Added integration and unit tests to verify correct footnote conversion and CLI behaviour.

Walkthrough

Introduce a new footnote conversion feature that transforms bare numeric references into GitHub-flavoured Markdown footnotes. Update the CLI, processing pipeline, documentation, and tests to support and verify the --footnotes option. Add a dedicated footnotes module, extend public APIs, and include comprehensive documentation and test coverage.

Changes

File(s) Change Summary
README.md Document the new --footnotes option, update function signatures, and improve formatting.
docs/module-relationships.md Add the footnotes module and its function to the diagram and update related textual descriptions.
src/footnotes.rs Add new module for footnote conversion with public function convert_footnotes.
src/lib.rs Declare and publicly re-export the footnotes module and its function.
src/main.rs Add footnotes flag to CLI and processing, update struct and function signatures accordingly.
src/process.rs Add footnotes parameter to processing functions; integrate footnote conversion into pipeline.
tests/cli.rs Add CLI integration test for the --footnotes option.
tests/footnotes.rs Add integration tests for footnote conversion and idempotency.
tests/data/footnotes_input.txt,
footnotes_expected.txt
Add input and expected output files for footnote conversion tests.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Process
    participant Footnotes

    User->>CLI: Run with --footnotes option
    CLI->>Process: process_stream_opts(..., footnotes=true)
    Process->>Footnotes: convert_footnotes(lines)
    Footnotes-->>Process: Converted lines with footnotes
    Process-->>CLI: Further processed lines
    CLI-->>User: Output with GitHub-flavoured footnotes
Loading

Possibly related PRs

Poem

Footnotes now dance in Markdown delight,
No more plain numbers lurking in sight.
With a flag on the CLI,
They’re converted on the fly—
GitHub-style links shining bright!

📝✨

Note

Reviews paused

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/implement-footnote-conversion-feature

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jul 20, 2025

Reviewer's Guide

This PR adds GitHub-flavoured footnote conversion by introducing a new module, integrating it into the processing pipeline, exposing a CLI flag, updating docs and README examples, and covering the feature with unit and integration tests.

Class diagram for new footnote conversion module

classDiagram
    class footnotes {
        <<module>>
        +convert_footnotes(lines: &[String]) Vec<String>
    }
    class wrap {
        <<module>>
        +tokenize_markdown()
        Token
    }
    footnotes ..> wrap : uses tokenize_markdown, Token
Loading

Class diagram for updated process module and integration of footnotes

classDiagram
    class process {
        <<module>>
        +process_stream_inner(lines, wrap, ellipsis, footnotes) Vec<String>
        +process_stream(lines) Vec<String>
        +process_stream_no_wrap(lines) Vec<String>
        +process_stream_opts(lines, wrap, ellipsis, footnotes) Vec<String>
    }
    class footnotes {
        <<module>>
        +convert_footnotes()
    }
    process ..> footnotes : uses convert_footnotes
Loading

File-Level Changes

Change Details Files
Implement footnote conversion module
  • Add src/footnotes.rs with convert_footnotes logic and regex-based inline/block conversion
  • Export footnotes module and convert_footnotes in lib.rs
  • Extend module-relationships diagram to include footnotes
src/footnotes.rs
src/lib.rs
docs/module-relationships.md
Integrate footnote conversion into processing pipeline
  • Extend process_stream_inner and helpers to accept footnotes flag
  • Invoke convert_footnotes when footnotes=true
  • Update process_stream, process_stream_no_wrap, and process_stream_opts signatures
src/process.rs
Expose --footnotes CLI option
  • Add footnotes field and clap arg in main.rs
  • Pass opts.footnotes into process_stream_opts
src/main.rs
Update documentation and examples
  • Add --footnotes flag to README usage section
  • Adjust code snippets for process_stream_opts signature and examples
  • Refresh formatting in README and related docs
README.md
Add tests and test data for footnotes
  • Unit tests for convert_footnotes in src/footnotes.rs
  • CLI test for --footnotes option in tests/cli.rs
  • Integration tests and input/output fixtures in tests/footnotes.rs and tests/data
src/footnotes.rs
tests/cli.rs
tests/footnotes.rs
tests/data/footnotes_input.txt
tests/data/footnotes_expected.txt

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.

  • Clarify in the README/CLI docs that --footnotes rewrites both inline numeric markers and the trailing numbered list into GitHub-style footnote definitions.
  • Consider adjusting the ordering of wrap/ellipsis/footnote conversion in process_stream_inner so that footnote definitions aren’t inadvertently wrapped or split.
  • Right now convert_block only handles a single trailing list of footnotes; consider extending or documenting this if you need to support multiple footnote blocks in one document.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Clarify in the README/CLI docs that `--footnotes` rewrites both inline numeric markers and the trailing numbered list into GitHub-style footnote definitions.
- Consider adjusting the ordering of wrap/ellipsis/footnote conversion in `process_stream_inner` so that footnote definitions aren’t inadvertently wrapped or split.
- Right now `convert_block` only handles a single trailing list of footnotes; consider extending or documenting this if you need to support multiple footnote blocks in one document.

## Individual Comments

### Comment 1
<location> `tests/footnotes.rs:17` </location>
<code_context>
+}
+
+#[test]
+fn test_idempotent_on_converted() {
+    let expected: Vec<String> = include_lines!("data/footnotes_expected.txt");
+    let output = convert_footnotes(&expected);
+    assert_eq!(output, expected);
+}
</code_context>

<issue_to_address>
Consider adding a test for empty input and input with only whitespace.

Please add integration tests for empty and whitespace-only input to ensure these edge cases are handled correctly.
</issue_to_address>

### Comment 2
<location> `src/footnotes.rs:15` </location>
<code_context>
+    Regex::new(r"^(?P<indent>\s*)(?P<num>\d+)\.\s+(?P<rest>.*)$").unwrap()
+});
+
+fn convert_inline(text: &str) -> String {
+    let mut out = String::with_capacity(text.len());
+    let chars: Vec<char> = text.chars().collect();
</code_context>

<issue_to_address>
Consider rewriting convert_inline and convert_block using regex replacements and iterator helpers for a more declarative and concise approach.

```markdown
convert_inline and convert_block can be made much more declarative with a couple of regex replaces + iterator helpers, removing the manual char‐ and index‐looping:

```rust
use regex::{Captures, Regex};

static INLINE_FN_RE: once_cell::sync::Lazy<Regex> = once_cell::sync::Lazy::new(|| {
    // match punctuation + digits, ensure boundary (\s or end‐of‐string)
    Regex::new(r"(?P<punc>[.!?);:])(?P<num>\d+)(?=\s|$)").unwrap()
});

fn convert_inline(text: &str) -> String {
    // replace all inline digit refs in one shot
    INLINE_FN_RE
        .replace_all(text, |caps: &Captures| {
            format!("{}[^{}]", &caps["punc"], &caps["num"])
        })
        .into_owned()
}
```

```rust
fn convert_block(lines: &mut Vec<String>) {
    // find the last non-empty line, then scan backward for footnote lines
    let end = lines.iter()
                   .rposition(|l| !l.trim().is_empty())
                   .map(|i| i + 1)
                   .unwrap_or(0);

    // find the start of the contiguously numbered lines
    let start = (0..end)
        .rfind(|&i| !FOOTNOTE_LINE_RE.is_match(lines[i].trim_end()))
        .map_or(0, |i| i + 1);

    // skip if already converted or nothing to do
    if start >= end || lines[start].trim_start().starts_with("[^") {
        return;
    }

    // bulk‐replace each line via the same regex
    for line in &mut lines[start..end] {
        *line = FOOTNOTE_LINE_RE
            .replace(line, "${indent}[^${num}] ${rest}")
            .to_string();
    }
}
```

- We get rid of manual `Vec<char>` loops in `convert_inline`.  
- We replace each block line with one `Regex::replace`.  
- We locate `start`/`end` using `rposition`/`rfind` instead of `while` loops.  

All existing tests should pass unchanged.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread tests/footnotes.rs
Comment thread src/footnotes.rs
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🔭 Outside diff range comments (2)
tests/data/footnotes_input.txt (1)

1-28: Fix grammatical and typographical issues in test data.

Address the punctuation and typographical issues to improve test data quality:

-purpose. For instance, an example for
+purpose. For instance, an example for:

-fixing - Swatinem, accessed on July 15, 2025,
+fixing — Swatinem, accessed on July 15, 2025,

The test data structure and content are appropriate for validating footnote conversion functionality.

tests/data/footnotes_expected.txt (1)

1-28: Fix grammatical and typographical issues in expected output.

Apply the same corrections as the input file to maintain consistency:

-purpose. For instance, an example for
+purpose. For instance, an example for:

-fixing - Swatinem, accessed on July 15, 2025,
+fixing — Swatinem, accessed on July 15, 2025,

The footnote conversion format correctly demonstrates the transformation to GitHub-flavoured syntax.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 36c8853 and f0d3503.

📒 Files selected for processing (10)
  • README.md (5 hunks)
  • docs/module-relationships.md (2 hunks)
  • src/footnotes.rs (1 hunks)
  • src/lib.rs (2 hunks)
  • src/main.rs (2 hunks)
  • src/process.rs (3 hunks)
  • tests/cli.rs (1 hunks)
  • tests/data/footnotes_expected.txt (1 hunks)
  • tests/data/footnotes_input.txt (1 hunks)
  • tests/footnotes.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

docs/**/*.md

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md
**/*.md

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🧬 Code Graph Analysis (3)
src/lib.rs (1)
src/footnotes.rs (1)
  • convert_footnotes (80-101)
tests/footnotes.rs (1)
src/footnotes.rs (1)
  • convert_footnotes (80-101)
src/process.rs (1)
src/footnotes.rs (1)
  • convert_footnotes (80-101)
🪛 LanguageTool
tests/data/footnotes_expected.txt

[grammar] ~6-~6: Please add a punctuation mark at the end of paragraph.
Context: ...tem's purpose. For instance, an example for String::clone() should not just show...

(PUNCTUATION_PARAGRAPH_END)


[typographical] ~25-~25: To join two clauses or introduce examples, consider using an em dash.
Context: ...tnote [^2] Rustdoc doctests need fixing - Swatinem, accessed on July 15, 2025, <ht...

(DASH_RULE)

docs/module-relationships.md

[style] ~75-~75: Would you like to use the Oxford spelling “normalization”? The spelling ‘normalisation’ is also correct.
Context: ...es. The ellipsis module performs text normalisation. The process module provides streamin...

(OXFORD_SPELLING_Z_NOT_S)

tests/data/footnotes_input.txt

[grammar] ~6-~6: Please add a punctuation mark at the end of paragraph.
Context: ...tem's purpose. For instance, an example for String::clone() should not just show...

(PUNCTUATION_PARAGRAPH_END)


[typographical] ~25-~25: To join two clauses or introduce examples, consider using an em dash.
Context: ...ootnote 2. Rustdoc doctests need fixing - Swatinem, accessed on July 15, 2025, <ht...

(DASH_RULE)

README.md

[style] ~43-~43: Would you like to use the Oxford spelling “standardize”? The spelling ‘standardise’ is also correct.
Context: ...indentation level. - Use --breaks to standardise thematic breaks to a line of 70 undersc...

(OXFORD_SPELLING_Z_NOT_S)


[style] ~46-~46: Consider using the typographical ellipsis character here instead.
Context: ...ipsis to replace groups of three dots (...) with the ellipsis character (…`). ...

(ELLIPSIS)


[style] ~54-~54: You have used the passive voice repeatedly in nearby sentences. To make your writing clearer and easier to read, consider using active voice.
Context: ...ed, input is read from stdin and output is written to stdout. ### Example: Table Reflow...

(REP_PASSIVE_VOICE)


[style] ~109-~109: Since ownership is already implied, this phrasing may be redundant.
Context: ...for embedding the table reflow logic in your own Rust project: ```rust use mdtablefix::...

(PRP_OWN)


[style] ~139-~139: Would you like to use the Oxford spelling “recognizes”? The spelling ‘recognises’ is also correct.
Context: ...e. ## HTML table support mdtablefix recognises basic HTML <table> elements embedded ...

(OXFORD_SPELLING_Z_NOT_S)


[style] ~141-~141: ‘prior to’ might be wordy. Consider a shorter alternative.
Context: ...sing stage using convert_html_tables, prior to reflow. Only simple tables composed of...

(EN_WORDINESS_PREMIUM_PRIOR_TO)


[locale-violation] ~159-~159: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...th-rstest-fixtures.md) for details. ## License This project is licensed under the ISC...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~161-~161: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... This project is licensed under the ISC License. See the LICENSE file for fu...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~161-~161: LICENSE must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...icensed under the ISC License. See the LICENSE file for full details.

(LICENCE_LICENSE_NOUN_SINGULAR)

🔇 Additional comments (29)
src/lib.rs (3)

10-10: Documentation follows established pattern correctly.

The footnotes module documentation is consistent with other module descriptions in the file.


16-16: Module declaration follows correct pattern.

The footnotes module is declared publicly, consistent with other feature modules.


33-33: Public API export is correctly positioned.

The convert_footnotes function export is alphabetically positioned and follows the established pattern for re-exporting module functions.

tests/cli.rs (1)

137-153: CLI test implementation follows established patterns correctly.

The test properly verifies both command success and output correctness using external test data files. The output formatting with trim_end() and format! correctly handles trailing newlines.

docs/module-relationships.md (3)

39-42: Module documentation correctly added to diagram.

The footnotes module is properly documented with its public function in the Mermaid class diagram.


70-70: Usage relationship accurately reflects code dependency.

The arrow from process to footnotes correctly represents the actual usage in the codebase.


76-78: Text description properly updated to include footnote functionality.

The description accurately reflects the expanded capabilities of the process module. Note that "normalisation" uses correct British English spelling as specified in the coding guidelines (en-GB-oxendict), so the static analysis suggestion can be ignored.

src/main.rs (3)

26-26: Clippy lint reason correctly updated for additional boolean field.

The reason string properly reflects the increase from four to five independent flags.


41-43: New CLI argument follows established pattern.

The footnotes flag is correctly structured with appropriate documentation and follows the same pattern as other boolean options.


47-47: Function call correctly updated with new parameter.

The process_stream_opts call properly includes the footnotes flag whilst maintaining the correct parameter order.

tests/footnotes.rs (3)

1-2: Module documentation clearly explains test purpose.

The documentation concisely describes what the integration tests verify.


8-14: Basic conversion test properly structured.

The test correctly uses external test data files and verifies the convert_footnotes function output against expected results.


16-21: Idempotency test demonstrates good testing practice.

Testing that the conversion function produces the same output when applied to already-converted content is excellent practice for transformation functions.

README.md (5)

29-29: Document the footnotes option correctly.

The CLI option documentation is clear and appropriately placed.


49-50: Document the footnotes option correctly.

The footnotes option description accurately describes the functionality.


122-122: Update function signature documentation correctly.

The updated function signature properly reflects the new footnotes parameter.


130-132: Document the updated function signature accurately.

The documentation correctly describes the new footnotes parameter and function behaviour.


43-43: Use British English spelling as required by coding guidelines.

-to standardise thematic breaks
+to standardize thematic breaks

Likely an incorrect or invalid review comment.

src/footnotes.rs (5)

1-6: Module documentation follows guidelines correctly.

The module documentation properly explains the purpose and utility as required by coding guidelines.


11-13: Use appropriate regex compilation approach.

The LazyLock usage is correct for static regex compilation.


49-76: Function implementation is well-structured.

The convert_block function has good separation of concerns and clear logic flow. The early returns for edge cases are appropriate.


78-101: Public API function is well-implemented.

The convert_footnotes function correctly uses the #[must_use] attribute and handles the empty input case appropriately. The integration with tokenization preserves code blocks correctly.


103-136: Test coverage is comprehensive.

The unit tests cover the main use cases including inline conversion, block conversion, and idempotency. Good test structure using descriptive names.

src/process.rs (6)

5-5: Import statement correctly added.

The footnotes module import is properly placed alphabetically.


12-17: Function signature updated correctly.

The new footnotes parameter is properly added to the function signature with correct typing.


79-82: Footnote conversion integrated correctly in processing pipeline.

The conditional footnote conversion is properly placed after table processing but before wrapping, maintaining logical processing order.


91-93: Wrapper function updated correctly.

The process_stream function correctly passes false for footnotes to maintain existing behaviour.


96-98: Wrapper function updated correctly.

The process_stream_no_wrap function correctly passes false for footnotes to maintain existing behaviour.


101-108: Public API function signature updated correctly.

The process_stream_opts function properly exposes the footnotes parameter to external callers.

Comment thread README.md
Comment thread README.md
Comment thread src/footnotes.rs
Comment on lines +15 to +29
fn convert_inline(text: &str) -> String {
let mut out = String::with_capacity(text.len());
let chars: Vec<char> = text.chars().collect();
let mut i = 0;
while i < chars.len() {
let ch = chars[i];
if matches!(ch, '.' | '!' | '?' | ')' | ';' | ':')
&& (i == 0 || !chars[i - 1].is_ascii_digit())
{
let mut j = i + 1;
while j < chars.len() && chars[j].is_ascii_digit() {
j += 1;
}
if j > i + 1 && (j == chars.len() || chars[j].is_whitespace()) {
out.push(ch);
out.push_str("[^");
for c in &chars[i + 1..j] {
out.push(*c);
}
out.push(']');
if j < chars.len() {
out.push(chars[j]);
j += 1;
}
i = j;
continue;
}
}
out.push(ch);
i += 1;
}
out
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Reduce function complexity and improve readability.

The convert_inline function has high cyclomatic complexity with nested conditions and loops. Extract helper functions to improve maintainability:

+fn should_convert_at_position(chars: &[char], i: usize) -> bool {
+    matches!(chars[i], '.' | '!' | '?' | ')' | ';' | ':')
+        && (i == 0 || !chars[i - 1].is_ascii_digit())
+}
+
+fn find_digit_sequence_end(chars: &[char], start: usize) -> usize {
+    let mut j = start;
+    while j < chars.len() && chars[j].is_ascii_digit() {
+        j += 1;
+    }
+    j
+}
+
+fn is_valid_footnote_context(chars: &[char], start: usize, end: usize) -> bool {
+    end > start && (end == chars.len() || chars[end].is_whitespace())
+}

 fn convert_inline(text: &str) -> String {
     let mut out = String::with_capacity(text.len());
     let chars: Vec<char> = text.chars().collect();
     let mut i = 0;
     while i < chars.len() {
         let ch = chars[i];
-        if matches!(ch, '.' | '!' | '?' | ')' | ';' | ':')
-            && (i == 0 || !chars[i - 1].is_ascii_digit())
-        {
-            let mut j = i + 1;
-            while j < chars.len() && chars[j].is_ascii_digit() {
-                j += 1;
-            }
-            if j > i + 1 && (j == chars.len() || chars[j].is_whitespace()) {
+        if should_convert_at_position(&chars, i) {
+            let j = find_digit_sequence_end(&chars, i + 1);
+            if is_valid_footnote_context(&chars, i + 1, j) {
                 out.push(ch);
                 out.push_str("[^");
                 for c in &chars[i + 1..j] {
                     out.push(*c);
                 }
                 out.push(']');
                 if j < chars.len() {
                     out.push(chars[j]);
-                    j += 1;
                 }
-                i = j;
+                i = if j < chars.len() { j + 1 } else { j };
                 continue;
             }
         }
         out.push(ch);
         i += 1;
     }
     out
 }
📝 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.

Suggested change
fn convert_inline(text: &str) -> String {
let mut out = String::with_capacity(text.len());
let chars: Vec<char> = text.chars().collect();
let mut i = 0;
while i < chars.len() {
let ch = chars[i];
if matches!(ch, '.' | '!' | '?' | ')' | ';' | ':')
&& (i == 0 || !chars[i - 1].is_ascii_digit())
{
let mut j = i + 1;
while j < chars.len() && chars[j].is_ascii_digit() {
j += 1;
}
if j > i + 1 && (j == chars.len() || chars[j].is_whitespace()) {
out.push(ch);
out.push_str("[^");
for c in &chars[i + 1..j] {
out.push(*c);
}
out.push(']');
if j < chars.len() {
out.push(chars[j]);
j += 1;
}
i = j;
continue;
}
}
out.push(ch);
i += 1;
}
out
}
// Helper to decide if we should attempt a footnote conversion at index `i`.
fn should_convert_at_position(chars: &[char], i: usize) -> bool {
matches!(chars[i], '.' | '!' | '?' | ')' | ';' | ':')
&& (i == 0 || !chars[i - 1].is_ascii_digit())
}
// Find the end index of a contiguous digit sequence starting at `start`.
fn find_digit_sequence_end(chars: &[char], start: usize) -> usize {
let mut j = start;
while j < chars.len() && chars[j].is_ascii_digit() {
j += 1;
}
j
}
// Check that there was at least one digit and that the sequence ends
// at either the end of the slice or before whitespace.
fn is_valid_footnote_context(chars: &[char], start: usize, end: usize) -> bool {
end > start && (end == chars.len() || chars[end].is_whitespace())
}
fn convert_inline(text: &str) -> String {
let mut out = String::with_capacity(text.len());
let chars: Vec<char> = text.chars().collect();
let mut i = 0;
while i < chars.len() {
let ch = chars[i];
if should_convert_at_position(&chars, i) {
let j = find_digit_sequence_end(&chars, i + 1);
if is_valid_footnote_context(&chars, i + 1, j) {
out.push(ch);
out.push_str("[^");
for c in &chars[i + 1..j] {
out.push(*c);
}
out.push(']');
if j < chars.len() {
out.push(chars[j]);
}
i = if j < chars.len() { j + 1 } else { j };
continue;
}
}
out.push(ch);
i += 1;
}
out
}
🤖 Prompt for AI Agents
In src/footnotes.rs between lines 15 and 47, the convert_inline function is
complex due to nested conditions and loops. Refactor by extracting parts of the
logic into smaller helper functions, such as one to detect the punctuation and
digit pattern and another to build the footnote string. This will reduce
cyclomatic complexity and improve readability and maintainability.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (2)
tests/footnotes.rs (1)

23-27: Expand false positive testing coverage.

The current false positive tests are minimal. Add tests for empty input, whitespace-only input, and other edge cases as suggested in the past review.

Add these additional test cases:

+#[test]
+fn test_handles_empty_and_whitespace() {
+    // Empty input
+    assert_eq!(convert_footnotes(&[]), Vec::<String>::new());
+    
+    // Whitespace-only input
+    let whitespace_input = lines_vec!("   ", "\t\t", "");
+    assert_eq!(convert_footnotes(&whitespace_input), whitespace_input);
+    
+    // Mixed whitespace and valid content
+    let mixed_input = lines_vec!("", "Text.1", "   ");
+    let mixed_expected = lines_vec!("", "Text.[^1]", "   ");
+    assert_eq!(convert_footnotes(&mixed_input), mixed_expected);
+}
src/footnotes.rs (1)

15-52: Reduce function complexity by extracting helper functions.

The convert_inline function exceeds acceptable cyclomatic complexity with nested conditions and manual character iteration. Extract meaningful helper functions to improve maintainability and readability.

Apply this refactoring to reduce complexity:

+/// Check if character at position should trigger footnote conversion.
+fn should_convert_at_position(chars: &[char], i: usize) -> bool {
+    matches!(chars[i], '.' | '!' | '?' | ')' | ';' | ':')
+        && (i == 0 || !chars[i - 1].is_ascii_digit())
+}
+
+/// Find the end of emphasis markers (*_) starting from position.
+fn skip_emphasis_markers(chars: &[char], start: usize) -> usize {
+    let mut j = start;
+    while j < chars.len() && matches!(chars[j], '*' | '_') {
+        j += 1;
+    }
+    j
+}
+
+/// Find the end of digit sequence starting from position.
+fn find_digit_sequence_end(chars: &[char], start: usize) -> usize {
+    let mut j = start;
+    while j < chars.len() && chars[j].is_ascii_digit() {
+        j += 1;
+    }
+    j
+}
+
+/// Check if the digit sequence represents a valid footnote context.
+fn is_valid_footnote_context(chars: &[char], digits_start: usize, digits_end: usize) -> bool {
+    digits_end > digits_start && (digits_end == chars.len() || chars[digits_end].is_whitespace())
+}

 fn convert_inline(text: &str) -> String {
     let mut out = String::with_capacity(text.len());
     let chars: Vec<char> = text.chars().collect();
     let mut i = 0;
     while i < chars.len() {
         let ch = chars[i];
-        if matches!(ch, '.' | '!' | '?' | ')' | ';' | ':')
-            && (i == 0 || !chars[i - 1].is_ascii_digit())
-        {
-            let mut j = i + 1;
-            while j < chars.len() && matches!(chars[j], '*' | '_') {
-                j += 1;
-            }
-            let digits_start = j;
-            while j < chars.len() && chars[j].is_ascii_digit() {
-                j += 1;
-            }
-            if j > digits_start && (j == chars.len() || chars[j].is_whitespace()) {
+        if should_convert_at_position(&chars, i) {
+            let emphasis_end = skip_emphasis_markers(&chars, i + 1);
+            let digits_end = find_digit_sequence_end(&chars, emphasis_end);
+            
+            if is_valid_footnote_context(&chars, emphasis_end, digits_end) {
                 out.push(ch);
-                out.extend(chars[i + 1..digits_start].iter());
+                out.extend(chars[i + 1..emphasis_end].iter());
                 out.push_str("[^");
-                for c in &chars[digits_start..j] {
+                for c in &chars[emphasis_end..digits_end] {
                     out.push(*c);
                 }
                 out.push(']');
-                if j < chars.len() {
-                    out.push(chars[j]);
-                    j += 1;
+                if digits_end < chars.len() {
+                    out.push(chars[digits_end]);
+                    i = digits_end + 1;
+                } else {
+                    i = digits_end;
                 }
-                i = j;
                 continue;
             }
         }
         out.push(ch);
         i += 1;
     }
     out
 }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0d3503 and 26fa3c7.

📒 Files selected for processing (2)
  • src/footnotes.rs (1 hunks)
  • tests/footnotes.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🔇 Additional comments (6)
tests/footnotes.rs (3)

16-21: Idempotency test correctly verifies function stability.

The test ensures that applying convert_footnotes to already-converted output produces identical results, which is crucial for pipeline stability.


29-34: Edge case test correctly handles punctuation in emphasis.

The test verifies that footnotes immediately following emphasised text with punctuation are correctly converted, which is a valuable edge case.


8-14: Approve code changes – confirmed test data files exist

Found the following test data files:

  • tests/data/footnotes_input.txt
  • tests/data/footnotes_expected.txt
src/footnotes.rs (3)

1-6: Module documentation meets standards.

The module documentation clearly explains the purpose and utility, adhering to the requirement for //! doc comments at module start.


7-13: Use appropriate regex construction pattern.

The static regex uses LazyLock correctly for one-time initialisation, and the regex pattern is well-structured for capturing footnote line components.


83-106: Public API function correctly documented and structured.

The function includes proper #[must_use] attribute and Rustdoc comments. The implementation correctly handles empty input and processes tokens appropriately.

Comment thread tests/footnotes.rs
Comment on lines +1 to +6
//! Integration tests for footnote conversion.

use mdtablefix::convert_footnotes;

#[macro_use]
mod prelude;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add module documentation and verify macro imports.

The module lacks comprehensive documentation explaining the test strategy and scope. Also verify that the prelude module properly exports the include_lines! and lines_vec! macros used throughout the tests.

Apply this diff to improve documentation:

-//! Integration tests for footnote conversion.
+//! Integration tests for footnote conversion.
+//!
+//! Tests verify that the `convert_footnotes` function correctly transforms
+//! bare numeric references into GitHub-flavoured Markdown footnote syntax,
+//! handles edge cases, and maintains idempotency.
📝 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.

Suggested change
//! Integration tests for footnote conversion.
use mdtablefix::convert_footnotes;
#[macro_use]
mod prelude;
//! Integration tests for footnote conversion.
//!
//! Tests verify that the `convert_footnotes` function correctly transforms
//! bare numeric references into GitHub-flavoured Markdown footnote syntax,
//! handles edge cases, and maintains idempotency.
use mdtablefix::convert_footnotes;
#[macro_use]
mod prelude;
🤖 Prompt for AI Agents
In tests/footnotes.rs at lines 1 to 6, add detailed module-level documentation
describing the purpose, strategy, and scope of the footnote conversion
integration tests. Additionally, verify that the prelude module correctly
exports the macros include_lines! and lines_vec! used in the tests to ensure
they are available for use. Update the module documentation and confirm macro
exports accordingly.

Comment thread src/footnotes.rs
Comment on lines +54 to +49
fn convert_block(lines: &mut [String]) {
let mut end = lines.len();
while end > 0 && lines[end - 1].trim().is_empty() {
end -= 1;
}
let mut start = end;
while start > 0 {
if FOOTNOTE_LINE_RE.is_match(lines[start - 1].trim_end()) {
start -= 1;
} else {
break;
}
}
if start >= end {
return;
}
if lines[start].trim_start().starts_with("[^") {
return;
}
for line in lines.iter_mut().take(end).skip(start) {
if let Some(cap) = FOOTNOTE_LINE_RE.captures(line.as_str()) {
let indent = cap.name("indent").unwrap().as_str();
let num = cap.name("num").unwrap().as_str();
let rest = cap.name("rest").unwrap().as_str();
*line = format!("{indent}[^{num}] {rest}");
}
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Function structure is acceptable but improve variable naming.

The convert_block function has reasonable complexity, but variable names could be more descriptive to improve readability.

Apply this diff to improve naming:

 fn convert_block(lines: &mut [String]) {
-    let mut end = lines.len();
-    while end > 0 && lines[end - 1].trim().is_empty() {
-        end -= 1;
+    let mut last_non_empty = lines.len();
+    while last_non_empty > 0 && lines[last_non_empty - 1].trim().is_empty() {
+        last_non_empty -= 1;
     }
-    let mut start = end;
-    while start > 0 {
-        if FOOTNOTE_LINE_RE.is_match(lines[start - 1].trim_end()) {
-            start -= 1;
+    let mut footnote_block_start = last_non_empty;
+    while footnote_block_start > 0 {
+        if FOOTNOTE_LINE_RE.is_match(lines[footnote_block_start - 1].trim_end()) {
+            footnote_block_start -= 1;
         } else {
             break;
         }
     }
-    if start >= end {
+    if footnote_block_start >= last_non_empty {
         return;
     }
-    if lines[start].trim_start().starts_with("[^") {
+    if lines[footnote_block_start].trim_start().starts_with("[^") {
         return;
     }
-    for line in lines.iter_mut().take(end).skip(start) {
+    for line in lines.iter_mut().take(last_non_empty).skip(footnote_block_start) {
         if let Some(cap) = FOOTNOTE_LINE_RE.captures(line.as_str()) {
             let indent = cap.name("indent").unwrap().as_str();
             let num = cap.name("num").unwrap().as_str();
             let rest = cap.name("rest").unwrap().as_str();
             *line = format!("{indent}[^{num}] {rest}");
         }
     }
 }
📝 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.

Suggested change
fn convert_block(lines: &mut [String]) {
let mut end = lines.len();
while end > 0 && lines[end - 1].trim().is_empty() {
end -= 1;
}
let mut start = end;
while start > 0 {
if FOOTNOTE_LINE_RE.is_match(lines[start - 1].trim_end()) {
start -= 1;
} else {
break;
}
}
if start >= end {
return;
}
if lines[start].trim_start().starts_with("[^") {
return;
}
for line in lines.iter_mut().take(end).skip(start) {
if let Some(cap) = FOOTNOTE_LINE_RE.captures(line.as_str()) {
let indent = cap.name("indent").unwrap().as_str();
let num = cap.name("num").unwrap().as_str();
let rest = cap.name("rest").unwrap().as_str();
*line = format!("{indent}[^{num}] {rest}");
}
}
}
fn convert_block(lines: &mut [String]) {
let mut last_non_empty = lines.len();
while last_non_empty > 0 && lines[last_non_empty - 1].trim().is_empty() {
last_non_empty -= 1;
}
let mut footnote_block_start = last_non_empty;
while footnote_block_start > 0 {
if FOOTNOTE_LINE_RE.is_match(lines[footnote_block_start - 1].trim_end()) {
footnote_block_start -= 1;
} else {
break;
}
}
if footnote_block_start >= last_non_empty {
return;
}
if lines[footnote_block_start].trim_start().starts_with("[^") {
return;
}
for line in lines.iter_mut().take(last_non_empty).skip(footnote_block_start) {
if let Some(cap) = FOOTNOTE_LINE_RE.captures(line.as_str()) {
let indent = cap.name("indent").unwrap().as_str();
let num = cap.name("num").unwrap().as_str();
let rest = cap.name("rest").unwrap().as_str();
*line = format!("{indent}[^{num}] {rest}");
}
}
}
🤖 Prompt for AI Agents
In src/footnotes.rs lines 54 to 81, the variable names 'start' and 'end' in the
convert_block function are not descriptive enough. Rename 'end' to 'trimmed_end'
to indicate it marks the end of non-empty lines, and rename 'start' to
'footnote_start' to clarify it marks the start of the footnote block. Update all
references accordingly to improve code readability without changing logic.

Comment thread src/footnotes.rs
Comment on lines +108 to +109
#[cfg(test)]
mod tests {
use super::*;

#[test]
fn converts_inline_numbers() {
let input = vec!["See the docs.2".to_string()];
let expected = vec!["See the docs.[^2]".to_string()];
assert_eq!(convert_footnotes(&input), expected);
}

#[test]
fn converts_final_list() {
let input = vec![
"Text.".to_string(),
String::new(),
" 1. First".to_string(),
" 2. Second".to_string(),
];
let expected = vec![
"Text.".to_string(),
String::new(),
" [^1] First".to_string(),
" [^2] Second".to_string(),
];
assert_eq!(convert_footnotes(&input), expected);
}

#[test]
fn idempotent_on_existing_block() {
let input = vec![" [^1] First".to_string()];
assert_eq!(convert_footnotes(&input), input);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Unit tests provide good basic coverage.

The unit tests cover key scenarios including inline conversion, block conversion, and idempotency. However, expand test coverage to match the integration tests' thoroughness.

Add these additional unit test cases:

+    #[test]
+    fn handles_empty_input() {
+        assert_eq!(convert_footnotes(&[]), Vec::<String>::new());
+    }
+
+    #[test]
+    fn preserves_code_blocks() {
+        let input = vec!["Text `code.1` more.2".to_string()];
+        let expected = vec!["Text `code.1` more.[^2]".to_string()];
+        assert_eq!(convert_footnotes(&input), expected);
+    }
+
+    #[test]
+    fn handles_emphasis_markers() {
+        let input = vec!["Text.**1".to_string()];
+        let expected = vec!["Text.**[^1]".to_string()];
+        assert_eq!(convert_footnotes(&input), expected);
+    }
📝 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.

Suggested change
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn converts_inline_numbers() {
let input = vec!["See the docs.2".to_string()];
let expected = vec!["See the docs.[^2]".to_string()];
assert_eq!(convert_footnotes(&input), expected);
}
#[test]
fn converts_final_list() {
let input = vec![
"Text.".to_string(),
String::new(),
" 1. First".to_string(),
" 2. Second".to_string(),
];
let expected = vec![
"Text.".to_string(),
String::new(),
" [^1] First".to_string(),
" [^2] Second".to_string(),
];
assert_eq!(convert_footnotes(&input), expected);
}
#[test]
fn idempotent_on_existing_block() {
let input = vec![" [^1] First".to_string()];
assert_eq!(convert_footnotes(&input), input);
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn converts_inline_numbers() {
let input = vec!["See the docs.2".to_string()];
let expected = vec!["See the docs.[^2]".to_string()];
assert_eq!(convert_footnotes(&input), expected);
}
#[test]
fn converts_final_list() {
let input = vec![
"Text.".to_string(),
String::new(),
" 1. First".to_string(),
" 2. Second".to_string(),
];
let expected = vec![
"Text.".to_string(),
String::new(),
" [^1] First".to_string(),
" [^2] Second".to_string(),
];
assert_eq!(convert_footnotes(&input), expected);
}
#[test]
fn idempotent_on_existing_block() {
let input = vec![" [^1] First".to_string()];
assert_eq!(convert_footnotes(&input), input);
}
+
+ #[test]
+ fn handles_empty_input() {
+ assert_eq!(convert_footnotes(&[]), Vec::<String>::new());
+ }
+
+ #[test]
+ fn preserves_code_blocks() {
+ let input = vec!["Text `code.1` more.2".to_string()];
+ let expected = vec!["Text `code.1` more.[^2]".to_string()];
+ assert_eq!(convert_footnotes(&input), expected);
+ }
+
+ #[test]
+ fn handles_emphasis_markers() {
+ let input = vec!["Text.**1".to_string()];
+ let expected = vec!["Text.**[^1]".to_string()];
+ assert_eq!(convert_footnotes(&input), expected);
+ }
}
🤖 Prompt for AI Agents
In src/footnotes.rs around lines 108 to 141, the current unit tests cover basic
scenarios but lack the thoroughness of the integration tests. Add more unit
tests to cover edge cases such as multiple inline footnotes in one line,
footnotes with non-numeric identifiers, empty input, and mixed content lines.
This will ensure the convert_footnotes function is robust and behaves correctly
in all expected scenarios.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Jul 20, 2025

@coderabbitai pause

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 20, 2025

✅ Actions performed

Reviews paused.

@leynos leynos force-pushed the codex/implement-footnote-conversion-feature branch from 80a64fb to a99cecd Compare July 20, 2025 09:11
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Jul 20, 2025

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 20, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (2)
tests/footnotes.rs (2)

1-6: Module documentation needs enhancement per coding guidelines.


36-48: Empty and whitespace input tests provide essential edge case coverage.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 26fa3c7 and a99cecd.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • Cargo.toml (1 hunks)
  • README.md (3 hunks)
  • docs/module-relationships.md (2 hunks)
  • src/footnotes.rs (1 hunks)
  • src/lib.rs (2 hunks)
  • src/main.rs (2 hunks)
  • src/process.rs (2 hunks)
  • tests/cli.rs (1 hunks)
  • tests/data/footnotes_expected.txt (1 hunks)
  • tests/data/footnotes_input.txt (1 hunks)
  • tests/footnotes.rs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs

Instructions used from:

Sources:
⚙️ CodeRabbit Configuration File

**/*.md

Instructions used from:

Sources:
⚙️ CodeRabbit Configuration File

🧬 Code Graph Analysis (3)
src/lib.rs (1)
src/footnotes.rs (1)
  • convert_footnotes (53-74)
tests/footnotes.rs (1)
src/footnotes.rs (1)
  • convert_footnotes (53-74)
src/process.rs (2)
src/footnotes.rs (1)
  • convert_footnotes (53-74)
src/html.rs (2)
  • convert_html_tables (299-342)
  • lines (176-179)
🪛 LanguageTool
docs/module-relationships.md

[style] ~75-~75: Would you like to use the Oxford spelling “normalization”? The spelling ‘normalisation’ is also correct.
Context: ...es. The ellipsis module performs text normalisation. The process module provides streamin...

(OXFORD_SPELLING_Z_NOT_S)

tests/data/footnotes_input.txt

[grammar] ~6-~6: Please add a punctuation mark at the end of paragraph.
Context: ...tem's purpose. For instance, an example for String::clone() should not just show...

(PUNCTUATION_PARAGRAPH_END)


[typographical] ~25-~25: To join two clauses or introduce examples, consider using an em dash.
Context: ...ootnote 2. Rustdoc doctests need fixing - Swatinem, accessed on July 15, 2025, <ht...

(DASH_RULE)

tests/data/footnotes_expected.txt

[grammar] ~6-~6: Please add a punctuation mark at the end of paragraph.
Context: ...tem's purpose. For instance, an example for String::clone() should not just show...

(PUNCTUATION_PARAGRAPH_END)


[typographical] ~25-~25: To join two clauses or introduce examples, consider using an em dash.
Context: ...tnote [^2] Rustdoc doctests need fixing - Swatinem, accessed on July 15, 2025, <ht...

(DASH_RULE)

🔇 Additional comments (30)
Cargo.toml (1)

10-10: LGTM: Dependency addition is appropriate.

The once_cell dependency supports lazy static regex initialization in the footnotes module, which is a common and efficient pattern.

src/lib.rs (3)

10-10: LGTM: Module documentation follows established pattern.

The footnotes module description is consistent with other module descriptions in the list.


16-16: LGTM: Module declaration follows conventions.

The footnotes module is correctly declared as public, consistent with other feature modules.


33-33: LGTM: Public API re-export is consistent.

The convert_footnotes function is properly re-exported alongside other module functions, maintaining API consistency.

tests/cli.rs (1)

137-153: LGTM: CLI test follows established patterns.

The test structure is consistent with other CLI tests, properly validates both exit status and output content, and correctly handles the expected output formatting with trailing newlines.

docs/module-relationships.md (3)

39-42: LGTM: Module addition follows diagram conventions.

The footnotes module is correctly added to the Mermaid diagram with appropriate structure and public function listing.


70-70: LGTM: Relationship accurately reflects code structure.

The usage arrow from process to footnotes correctly represents the integration in the processing pipeline.


77-78: LGTM: Description accurately reflects functionality.

The updated text correctly describes the expanded role of the process module. Note: "normalisation" spelling is correct per en-GB guidelines.

src/main.rs (3)

26-26: LGTM: Clippy lint reason correctly updated.

The reason accurately reflects the increase from four to five boolean flags in the struct.


41-44: LGTM: CLI flag follows established pattern.

The footnotes flag is properly structured with clear documentation that accurately describes its functionality.


48-48: LGTM: Function call correctly updated.

The process_stream_opts call properly includes the new footnotes parameter, maintaining the correct argument order.

tests/data/footnotes_input.txt (1)

1-28: Test data structure is well-designed for comprehensive footnote conversion testing.

The input file effectively covers multiple scenarios:

  • Inline footnote references after punctuation (lines 4, 9, 13, 22)
  • Final numbered list conversion (lines 24-27)
  • Duplicate footnote numbers for testing edge cases
  • Mixed content to verify selective conversion
tests/data/footnotes_expected.txt (1)

1-28: Expected output correctly demonstrates GitHub-flavoured footnote syntax.

The conversion logic is properly represented:

  • Inline references correctly transformed to [^num] format
  • Block footnotes maintain indentation and convert to [^num] content format
  • Existing footnote syntax preserved (line 24)
README.md (4)

29-29: Command synopsis correctly updated with new footnotes option.

The --footnotes flag is properly integrated into the usage pattern alongside existing options.


50-52: Footnotes feature description is clear and accurate.

The description correctly explains the conversion of bare numeric references and final numbered lists to GitHub-flavoured footnote syntax.


123-123: Library usage example correctly updated for new API signature.

The process_stream_opts call properly demonstrates the new footnotes boolean parameter.


131-133: Function documentation accurately reflects updated signature.

The process_stream_opts description correctly includes footnote conversion alongside existing capabilities.

tests/footnotes.rs (4)

8-14: Integration test correctly verifies end-to-end conversion.

The test properly uses file-based input and expected output to verify the complete footnote conversion pipeline.


16-21: Idempotency test ensures conversion stability.

This test correctly verifies that applying footnote conversion to already-converted content produces no changes, which is essential for this type of transformation.


23-27: False positive test prevents unwanted conversions.

The test cases effectively verify that phone numbers and version numbers are not incorrectly converted to footnotes.


29-34: Edge case test handles formatting complexity correctly.

Testing footnotes within bold text ensures the conversion works with markdown formatting elements.

src/footnotes.rs (6)

1-6: Module documentation follows guidelines and clearly explains purpose.

The documentation properly uses British English ("normalisation", "flavoured") and explains the module's functionality and scope clearly.


9-16: Regex patterns correctly identify footnote references and blocks.

The inline pattern properly captures punctuation, emphasis markers, and numeric sequences with appropriate boundaries. The footnote line pattern correctly matches numbered list format with indentation preservation.


20-29: Inline conversion implementation is clean and efficient.

The regex-based approach properly handles the complex matching requirements whilst maintaining readability. The capture group replacement correctly preserves all formatting elements.


31-49: Block conversion logic correctly identifies and transforms footnote lists.

The implementation properly:

  • Finds the last non-empty line
  • Identifies contiguous footnote blocks from the end
  • Skips already-converted content
  • Preserves indentation in the transformation

51-74: Public API function correctly orchestrates the conversion process.

The function properly:

  • Handles empty input
  • Integrates with existing markdown tokenisation
  • Applies inline conversion only to text tokens
  • Preserves code blocks and fenced content
  • Applies block conversion to the final result

The #[must_use] attribute is appropriate for a pure function.


76-109: Unit tests provide adequate coverage of core functionality.

The tests cover the essential scenarios:

  • Inline footnote conversion
  • Block footnote conversion with indentation
  • Idempotency verification
src/process.rs (3)

5-5: Import statement is correctly positioned and necessary.

The import follows the existing pattern and is required for the footnote conversion functionality.


12-17: Function signature extension follows existing patterns.

The addition of the footnotes parameter maintains consistency with the existing boolean flag pattern for optional processing steps.


83-85: Footnote conversion is correctly positioned in the processing pipeline.

Placing footnote conversion as the final step after wrapping and ellipsis replacement follows logical processing order and maintains consistency with the existing conditional processing pattern.

Comment thread src/process.rs
Comment on lines +90 to 107
pub fn process_stream(lines: &[String]) -> Vec<String> {
process_stream_inner(lines, true, false, false)
}

#[must_use]
pub fn process_stream_no_wrap(lines: &[String]) -> Vec<String> {
process_stream_inner(lines, false, false)
process_stream_inner(lines, false, false, false)
}

#[must_use]
pub fn process_stream_opts(lines: &[String], wrap: bool, ellipsis: bool) -> Vec<String> {
process_stream_inner(lines, wrap, ellipsis)
pub fn process_stream_opts(
lines: &[String],
wrap: bool,
ellipsis: bool,
footnotes: bool,
) -> Vec<String> {
process_stream_inner(lines, wrap, ellipsis, footnotes)
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Wrapper function updates maintain backward compatibility whilst extending functionality.

The implementation correctly preserves existing behaviour by defaulting footnotes to false and properly forwards the new parameter through process_stream_opts.

Verify that the API changes are reflected in the documentation as required by the coding guidelines:


🏁 Script executed:

#!/bin/bash
# Description: Check if documentation has been updated to reflect the new footnotes parameter in process_stream_opts
# Expected: Documentation should mention the new footnotes parameter

# Check for mentions of footnotes parameter in documentation
rg -i "footnotes.*parameter|process_stream_opts.*footnotes" docs/

Length of output: 67


Update documentation to include the new footnotes parameter

The search returned no references to footnotes or process_stream_opts in the docs. Add or update the relevant documentation to describe the new footnotes flag:

  • docs/… (e.g. usage guide or API reference): document the footnotes boolean parameter in process_stream_opts along with its default behaviour.
  • README.md or examples: show how to call process_stream_opts with footnotes = true and explain its effect.
🤖 Prompt for AI Agents
In src/process.rs around lines 90 to 107, the new boolean parameter `footnotes`
in the function `process_stream_opts` is not documented. Update the relevant
documentation files such as the usage guide or API reference to describe the
`footnotes` parameter, including its purpose and default behavior. Additionally,
update README.md or example files to demonstrate how to call
`process_stream_opts` with `footnotes` set to true and explain the effect this
flag has on the function's output.

@leynos leynos merged commit 2e3395a into main Jul 20, 2025
2 checks passed
@leynos leynos deleted the codex/implement-footnote-conversion-feature branch July 20, 2025 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant