Skip to content

Implement builder defaults and expand fence tests#110

Merged
leynos merged 4 commits intomainfrom
codex/address-code-review-comments
Jul 20, 2025
Merged

Implement builder defaults and expand fence tests#110
leynos merged 4 commits intomainfrom
codex/address-code-review-comments

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jul 20, 2025

Summary

  • simplify Options construction with Default and builder methods
  • refactor attach_orphan_specifiers to a single pass implementation
  • normalise fence options in example documentation
  • add CLI test for tilde fences
  • cover mixed fences and orphan specifier edge cases

Testing

  • make lint
  • make test

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

Summary by Sourcery

Simplify and unify Options construction with Default and builder methods, refactor fence handling to a single-pass approach, normalize documentation example, and expand fence-related test coverage

Bug Fixes:

  • Prevent incorrect compression of mixed code fence markers
  • Fix attachment of orphan language specifiers in single-pass implementation

Enhancements:

  • Derive Default for Options and add builder methods for wrap, ellipsis, fences, and footnotes
  • Refactor attach_orphan_specifiers into a single-pass algorithm
  • Simplify process_stream functions to use Options::default with builders

Documentation:

  • Update README example to use Options::default and builder methods for configuration

Tests:

  • Add tests for mixed fence markers and tilde fence compression
  • Add tests for multiple and non-orphan specifier edge cases
  • Add CLI test for tilde fence normalization

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jul 20, 2025

Reviewer's Guide

This PR introduces Default and builder methods for Options to streamline configuration, refactors attach_orphan_specifiers into a single-pass algorithm, updates example docs for fence normalization, and expands fence testing (mixed tilde/backtick and orphan specifier cases), including a new CLI tilde-fence test.

Class diagram for updated Options struct with builder methods and Default

classDiagram
    class Options {
        +bool wrap
        +bool ellipsis
        +bool fences
        +bool footnotes
        +with_wrap(wrap: bool) Options
        +with_ellipsis(ellipsis: bool) Options
        +with_fences(fences: bool) Options
        +with_footnotes(footnotes: bool) Options
        +default() Options
    }
Loading

File-Level Changes

Change Details Files
Streamline Options construction with Default and builder methods
  • Derive Default on the Options struct
  • Implement with_wrap, with_ellipsis, with_fences, with_footnotes builder methods
  • Refactor process_stream and process_stream_no_wrap to use Options::default() and builders
  • Update README example to use the new builder pattern
src/process.rs
README.md
Refactor attach_orphan_specifiers to a single-pass implementation
  • Replace peekable iterator logic with a simple for loop and in_fence flag
  • Remove nested peeking and blank-line skip loops
  • Trim trailing blank lines and detect orphan specifiers inline
  • Toggle fence state and rewrite specifier lines in one pass
src/fences.rs
Expand fence-related tests and add CLI tilde-fence test
  • Add tests for mixed backtick/tilde fences in compress_fences
  • Add tests for multiple and non-orphan specifier edge cases
  • Add a CLI integration test for tilde fences normalization
tests/fences.rs
tests/cli.rs

Possibly linked issues

  • #0: The PR refactors the attach_orphan_specifiers function to a single-pass implementation, directly addressing the complexity and state management concerns in the issue.
  • Implement markdown table fixer #1: The PR refactors attach_orphan_specifiers to a single-pass implementation, removing complex lookaheads and buffer manipulation, as requested by the issue.

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jul 20, 2025

Summary by CodeRabbit

  • Documentation
    • Updated the README example to use struct update syntax for options, improving clarity and conciseness.
  • Refactor
    • Simplified the logic for handling orphaned language specifiers and fence lines for code blocks, improving maintainability.
    • Introduced a constant for wrap column width and streamlined default option handling.
  • New Features
    • Added documentation and an example for a function that processes streams without wrapping.
  • Bug Fixes
    • Improved handling of mixed fence delimiters and orphaned language specifiers in code blocks.
  • Tests
    • Added new tests for fence normalisation, orphaned specifier attachment, and CLI handling of tilde fences.

Walkthrough

Refactor the Options struct to derive Default and add builder-style setter methods. Update all usage examples and internal calls to use the new builder pattern. Simplify the attach_orphan_specifiers function by removing the Peekable iterator and streamlining its logic. Expand test coverage for fence handling and CLI behaviour.

Changes

File(s) Change Summary
README.md Update example code to use struct update syntax with Options::default(); remove explicit footnotes setting.
src/process.rs Derive Default for Options; simplify process_stream and process_stream_no_wrap to use Options::default().
src/fences.rs Refactor attach_orphan_specifiers to remove Peekable iterator; simplify logic to a single for-loop.
tests/cli.rs Add integration test for CLI --fences option with tilde fence normalisation.
tests/fences.rs Add tests for mixed fence normalisation, orphan specifier attachment, and non-orphan line behaviour.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant Processor
    participant Fences

    User->>CLI: Run mdtablefix with --fences and input
    CLI->>Processor: Call process_stream with Options::default().with_fences(true)
    Processor->>Fences: Call attach_orphan_specifiers on input lines
    Fences-->>Processor: Return processed lines
    Processor-->>CLI: Output normalised markdown
    CLI-->>User: Display result
Loading

Possibly related issues

Possibly related PRs

Poem

In the code’s bright garden, fences align,
Orphaned specs now find their sign.
Builder patterns bloom with pride,
Default traits walk by their side.
Tests now dance, CLI sings—
Markdown magic, code with wings!
🦋


📜 Recent review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 63254b3 and 57269c1.

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

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🔇 Additional comments (6)
src/process.rs (6)

12-13: LGTM! Constant extraction addresses magic number concern.

The introduction of WRAP_COLS appropriately addresses the previous feedback about hardcoded values and improves maintainability.


20-20: LGTM! Default trait enables builder pattern usage.

The addition of Default to the derive macro correctly supports the simplified option construction used elsewhere in the codebase.


102-106: LGTM! Proper usage of the new constant.

The replacement of the hardcoded value with WRAP_COLS ensures consistency and maintainability.


120-122: LGTM! Clean struct update syntax implementation.

The use of ..Default::default() with explicit wrap: true is more concise and readable than explicit field construction.


127-134: LGTM! Comprehensive documentation addresses public API requirement.

The Rustdoc comment with example correctly documents the public function as required by the coding guidelines.


136-138: LGTM! Simplified implementation with appropriate inlining.

The addition of #[inline] and use of Options::default() correctly addresses previous feedback about optimising this trivial wrapper.

✨ 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/address-code-review-comments

🪧 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.

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.

Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments

### Comment 1
<location> `src/process.rs:29` </location>
<code_context>
     pub footnotes: bool,
 }

+impl Options {
+    /// Set `wrap` option and return the updated struct.
+    #[must_use]
</code_context>

<issue_to_address>
Consider removing the redundant with_* setter methods and using struct update syntax with Default when modifying Options fields.

```rust
// 1. Drop the entire `impl Options { … }` block; the four `with_*` setters are redundant.
// 2. Use struct update syntax with Default where you need to tweak a flag:

// Before:
pub fn process_stream(lines: &[String]) -> Vec<String> {
    process_stream_inner(lines, Options::default().with_wrap(true))
}

// After:
pub fn process_stream(lines: &[String]) -> Vec<String> {
    process_stream_inner(lines, Options { wrap: true, ..Default::default() })
}

// Likewise, `process_stream_no_wrap` can just call `Default::default()`:
pub fn process_stream_no_wrap(lines: &[String]) -> Vec<String> {
    process_stream_inner(lines, Options::default())
}
```
</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 src/process.rs Outdated
@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.

@github-actions
Copy link
Copy Markdown

@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

🔭 Outside diff range comments (3)
README.md (1)

122-128: Align example with stated behaviour – enable footnotes or fix the prose

The snippet now turns on wrap, ellipsis, and fences, yet the paragraph later on claims that footnote conversion is enabled. Prevent reader confusion by either adding the flag below or amending the documentation text.

-        fences: true,
-        ..Default::default()
+        fences: true,
+        footnotes: true,
+        ..Default::default()
src/process.rs (2)

17-27: Shrink the trait surface – drop Copy

Options is four booleans; cloning is effectively free. Deriving Copy silently spreads implicit copies and can mask bugs that rely on move semantics. Retain Clone and Default only.

-#[derive(Clone, Copy, Default)]
+#[derive(Clone, Default)]

111-116: Leverage Options::default() consistently

Maintain stylistic unity and guard against future field additions by updating from the actual default rather than an ad-hoc literal.

-        Options {
-            wrap: true,
-            ..Default::default()
-        },
+        Options { wrap: true, ..Options::default() },
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between b5d4f0e and efd2131.

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

Instructions used from:

Sources:
⚙️ CodeRabbit Configuration File

**/*.rs

Instructions used from:

Sources:
⚙️ CodeRabbit Configuration File

Comment thread src/process.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: 2

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between efd2131 and 63254b3.

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

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🔇 Additional comments (1)
src/process.rs (1)

17-17: Derive Default is spot-on
Adding Default lets callers rely on zero-cost initialisation and removes the need for redundant builder setters.

Comment thread src/process.rs
Comment thread src/process.rs
@leynos leynos merged commit eb9e1cc into main Jul 20, 2025
2 checks passed
@leynos leynos deleted the codex/address-code-review-comments branch July 20, 2025 22:43
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