Conversation
Re-export convert_html_tables so integration tests can call it. Added basic tests exercising HTML conversion and ensuring code fences are ignored.
Reviewer's GuideThis PR makes the HTML-to-Markdown table converter available publicly and bolsters the test suite with fixtures and integration tests—covering basic, attribute-rich, colspan, header-less, empty and unclosed tables, as well as behavior inside and outside fenced code blocks. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded@leynos has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update publicly re-exports the Changes
Sequence Diagram(s)sequenceDiagram
participant TestSuite
participant convert_html_tables
participant MarkdownOutput
TestSuite->>convert_html_tables: Provide input (Vec<String>)
convert_html_tables-->>MarkdownOutput: Process and convert HTML tables
convert_html_tables-->>TestSuite: Return converted Markdown lines
Possibly related PRs
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- The rstest
test_convert_html_table_basic(html_table: Vec<String>)references a fixturehtml_tablethat isn’t defined—either provide the input directly or add the fixture definition. - I don’t see
convert_html_tablesbeing publicly exported inlib.rs, so the integration tests won’t compile—please add the necessarypub useorpub fnexport.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The rstest `test_convert_html_table_basic(html_table: Vec<String>)` references a fixture `html_table` that isn’t defined—either provide the input directly or add the fixture definition.
- I don’t see `convert_html_tables` being publicly exported in `lib.rs`, so the integration tests won’t compile—please add the necessary `pub use` or `pub fn` export.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: 0
🧹 Nitpick comments (3)
src/lib.rs (1)
7-8: Public re-export looks good, but document it for discoverabilityRe-exporting
convert_html_tablesat the crate root is a sensible ergonomic win.
Consider adding a short mention in the module-level docs (//! …) so downstream users spot the new API without diving into the code.tests/integration.rs (2)
348-359: PreferStringliterals for consistencyMost other tests use
Stringvalues forexpected, keeping element types identical to the function return type (Vec<String>).
Switching here avoids the implicit&str→Stringcomparison and makes the intent explicit.- let expected = vec!["| A | B |", "| --- | --- |", "| 1 | 2 |"]; + let expected = vec![ + "| A | B |".to_string(), + "| --- | --- |".to_string(), + "| 1 | 2 |".to_string(), + ];
360-384: Consider covering more fence flavoursThis test is great for back-tick fences.
For full coverage add a tilde-based fence variant (~~~) – the helper already supports it and it will hard-fail if future edits accidentally break one flavour.If you want, I can draft the additional test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib.rs(1 hunks)tests/integration.rs(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/lib.rs (1)
src/html.rs (1)
convert_html_tables(190-233)
🔇 Additional comments (1)
tests/integration.rs (1)
2-2: Import line is fineNothing to add – the newly re-exported item is imported correctly.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Add tests for tables with HTML attributes (e.g.,
<table class="...">or<th colspan="2">) to ensure those attributes are handled or stripped correctly. - Add tests for edge cases like empty or malformed tables (e.g., missing header rows or unmatched tags) to validate how convert_html_tables handles them.
- Add a test for fenced code blocks with language specifiers (e.g., ```rust) to verify that tables inside such blocks are still preserved.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add tests for tables with HTML attributes (e.g., `<table class="...">` or `<th colspan="2">`) to ensure those attributes are handled or stripped correctly.
- Add tests for edge cases like empty or malformed tables (e.g., missing header rows or unmatched tags) to validate how convert_html_tables handles them.
- Add a test for fenced code blocks with language specifiers (e.g., ```rust) to verify that tables inside such blocks are still preserved.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes - here's some feedback:
- Add a test using the existing html_table_uppercase fixture to ensure convert_html_tables handles uppercase tags case-insensitively.
- Reduce duplication in test fixtures by introducing a small helper or macro to build Vec from &str slices.
- Consider normalizing column counts in colspan scenarios (e.g., duplicating cells for colspan spans) or adding a test to document the current uneven-row behavior.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Add a test using the existing html_table_uppercase fixture to ensure convert_html_tables handles uppercase tags case-insensitively.
- Reduce duplication in test fixtures by introducing a small helper or macro to build Vec<String> from &str slices.
- Consider normalizing column counts in colspan scenarios (e.g., duplicating cells for colspan spans) or adding a test to document the current uneven-row behavior.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@coderabbitai please create the following github issues: Add a test using the existing html_table_uppercase fixture to ensure convert_html_tables handles uppercase tags case-insensitively. Reduce duplication in test fixtures by introducing a small helper or macro to build Vec from &str slices. Consider normalizing column counts in colspan scenarios (e.g., duplicating cells for colspan spans) or adding a test to document the current uneven-row behavior. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@leynos I'll create those GitHub issues for you to track these improvements. 🏁 Script executed: Length of output: 2999 Perfect! I've successfully created three GitHub issues for you:
Each issue includes:
The issues are now ready for you to prioritize and work on at your convenience! 🐰 |
1 participant
Summary
convert_html_tablesTesting
cargo clippy -- -D warningscargo testhttps://chatgpt.com/codex/tasks/task_e_684cf2fff2a8832286cff24c92bc74f2
Summary by Sourcery
Export the HTML table conversion function and add comprehensive tests to validate its behavior across basic, attribute-rich, colspan, headerless, empty, unclosed, and fenced code scenarios.
New Features:
convert_html_tablesfunction for external use.Tests:
Summary by CodeRabbit
New Features
Tests