feat(cli): add PNG output with converter-style format selection#12
Conversation
Wires resolve_format, Format::ext, and svg_to_png into main() via a new write_page helper; adds --format/-f flag to Cli; removes all transient #[allow(dead_code)] attrs; adds renders_sample_to_valid_png end-to-end test.
|
Warning Review limit reached
More reviews will be available in 44 minutes and 12 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdded optional ChangesSVG/PNG Format Selection and Output
Sequence DiagramsequenceDiagram
participant User as User / CLI
participant Resolve as resolve_format()
participant Ext as Format::ext()
participant Write as write_page()
participant Conv as svg_to_png()
participant FS as File System
User->>Resolve: --format flag + output path
Resolve->>Resolve: Check explicit flag first
alt Flag provided
Resolve->>Resolve: Use flag value
else No flag
Resolve->>Resolve: Infer from output extension
Resolve->>Resolve: Default to Svg if unknown
end
Resolve-->>User: Resolved Format (Svg/Png)
User->>Ext: Get file extension
Ext-->>User: .svg or .png
User->>Write: (path, svg_text, format)
alt Format::Svg
Write->>FS: Write SVG text directly
else Format::Png
Write->>Conv: svg_to_png(svg_text)
Conv-->>Write: PNG bytes
Write->>FS: Write PNG bytes
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds PNG output support to sdocx-cli, selecting output format ffmpeg-style (infer from -o extension, override with -f/--format) by rasterizing the existing SVG render via resvg.
Changes:
- Introduces
Format+--formatflag and extension-based format inference (resolve_format). - Adds SVG→PNG rasterization (
svg_to_png) and routes output writing throughwrite_page. - Adds unit tests covering format resolution and basic PNG validity, and adds the
resvgdependency.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/sdocx-cli/src/main.rs | Adds format selection, SVG→PNG rendering, output writing logic, and related tests. |
| crates/sdocx-cli/Cargo.toml | Adds resvg dependency needed for PNG output. |
| Cargo.lock | Locks new transitive dependencies introduced by resvg. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match output.and_then(|p| p.extension()).and_then(|e| e.to_str()) { | ||
| Some("svg") => Ok(Format::Svg), | ||
| Some("png") => Ok(Format::Png), |
| fn svg_to_png(svg: &str) -> Result<Vec<u8>, String> { | ||
| let mut opt = resvg::usvg::Options::default(); | ||
| // Load system fonts so <text> elements render instead of being silently dropped. | ||
| opt.fontdb_mut().load_system_fonts(); |
|
|
||
| #[test] | ||
| fn renders_sample_to_valid_png() { | ||
| let doc = sdocx::parse("../../samples/handwritten.sdocx").expect("parse sample"); |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/sdocx-cli/src/main.rs (1)
645-669:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize output extension to the resolved format.
Line 645 keeps the user-provided extension unchanged, so
--formatcan produce content/extension mismatches (e.g., SVG bytes in.pngfiles). This breaks converter-style expectations and can confuse downstream tools.Proposed fix
- let output_base = cli - .output - .unwrap_or_else(|| cli.path.with_extension(format.ext())); + let output_base = cli + .output + .unwrap_or_else(|| cli.path.with_extension(format.ext())) + .with_extension(format.ext()); @@ - let ext = output_base - .extension() - .and_then(|e| e.to_str()) - .unwrap_or(format.ext()); - let path = output_base.with_file_name(format!("{stem}_page{i}.{ext}")); + let path = output_base.with_file_name(format!("{stem}_page{i}.{}", format.ext()));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/sdocx-cli/src/main.rs` around lines 645 - 669, The output path currently preserves the user-provided extension (output_base) which can mismatch the resolved format; change code that constructs output paths to normalize the extension to the resolved format.ext(): when computing output_base (from cli.output or cli.path) call with_extension(format.ext()) so single-page write_page gets the correct extension, and in the multi-page loop derive ext from format.ext() (not output_base.extension()) when building path for each page (see symbols: output_base, cli.output, cli.path, format.ext(), write_page, render_page_svg). Ensure all generated file names use the format.ext() extension.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/sdocx-cli/src/main.rs`:
- Around line 26-31: The extension matching in main.rs currently only accepts
lowercase "svg"/"png"; update the match on output.and_then(|p|
p.extension()).and_then(|e| e.to_str()) to be case-insensitive by either mapping
the extension string to a canonical form (e.g., call to_ascii_lowercase() on the
&str before matching) or by using conditional arms with eq_ignore_ascii_case
(e.g., Some(ext) if ext.eq_ignore_ascii_case("svg") => Ok(Format::Svg), similar
for "png"); leave the error branch to report the original extension value if
present. Ensure the match still returns Ok(Format::Svg)/Ok(Format::Png) and
Err(...) on unknown values.
---
Outside diff comments:
In `@crates/sdocx-cli/src/main.rs`:
- Around line 645-669: The output path currently preserves the user-provided
extension (output_base) which can mismatch the resolved format; change code that
constructs output paths to normalize the extension to the resolved format.ext():
when computing output_base (from cli.output or cli.path) call
with_extension(format.ext()) so single-page write_page gets the correct
extension, and in the multi-page loop derive ext from format.ext() (not
output_base.extension()) when building path for each page (see symbols:
output_base, cli.output, cli.path, format.ext(), write_page, render_page_svg).
Ensure all generated file names use the format.ext() extension.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c644553-3e2c-4129-bc1a-29c98de1925c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
crates/sdocx-cli/Cargo.tomlcrates/sdocx-cli/src/main.rs
Adds PNG output to sdocx-cli, with the output format chosen ffmpeg-style — inferred from the
-oextension and overridable with a-f/--formatflag — by rasterizing the existing SVG render through resvg.Summary by CodeRabbit
Release Notes
New Features
--formatflag to specify output as SVG or PNGTests