Repository navigation
✨ feat: add -format pptx that places each rendered slide on a PowerPoint slide - #88
Conversation
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (14)
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
21424d2 to
6dd8d2c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21424d20d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err != nil { | ||
| return fmt.Errorf("capture slide %d: %w", i+1, err) | ||
| } | ||
| images = append(images, img) |
There was a problem hiding this comment.
Stream slide images instead of buffering the whole deck
For large or photo-heavy decks, every full-resolution screenshot is retained in images until capture completes, after which writePPTX also constructs the entire ZIP in a bytes.Buffer before writing it. This requires memory for all PNGs plus approximately the complete PPTX concurrently, so otherwise valid decks can exhaust memory; write captures through temporary files or directly into a streaming ZIP so each image can be released before processing the next slide.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed on this PR: renderSlidePNGs writes each capture to the working directory as soon as it is taken and returns paths; writePPTX streams the zip into a temporary file beside the destination, copying one image at a time, and renames it into place only when complete. New tests: the embedded image is byte-identical to the captured file, and a missing image fails without leaving any file behind. Re-checked a generated deck with python-pptx.
…int slide -format pptx (or -o *.pptx) renders the document as a deck, captures each slide with headless Chromium at 2x, and writes a PowerPoint file by hand with archive/zip: one full-bleed picture per slide, presenter notes on the notes pages, the front-matter title as the presentation title. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… them Every capture was held in memory until the end, and the whole package was built in a bytes.Buffer before being written. Captures now go to the working directory as they are taken and the zip is streamed into a temporary file renamed into place when complete, so a failure leaves no partial .pptx. Codex review on #88. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6dd8d2c to
b91d731
Compare
Summary
The deliverable of #72:
-format pptx(or-o *.pptx) writes a PowerPoint deck.md2pdf -format pptx deck.md # → deck.pptxEach slide is rendered by the Chromium that prints the PDF and placed on its own slide as a picture (image-based, approach A in #72), so the PPTX looks exactly like the slide-mode PDF. Presenter notes (#77) go to the notes pages; the front-matter
titlebecomes the presentation title. A pptx conversion is always a deck — nomarp: trueneeded.Closes #80
Closes #72
Changes
slidecapture.go:renderSlidePNGs— viewport = slide size at device scale 2, print media, fonts awaited, overflow measured (warnings as for PDF), eachsection.slidecaptured with a clippedPage.captureScreenshot(captureBeyondViewport).pptx.go:writePPTX, hand-written OOXML viaarchive/zip, no new dependency. Minimal package: master, blank layout, theme, one full-bleedp:picper slide; notes master (with its own theme) and notes slides only when some slide has notes.pdf.go: browser start-up and the fonts wait extracted intostartBrowser/awaitFonts, shared by PDF and PPTX.converter.go:FormatPPTX,writeDeckPPTX; pptx forces slide mode (so explicit-page-size/-margin-*are rejected as for any deck)..pptxinference,-format pptx,-slidesaccepted with pptx, stdin needs-o(existing rule), help text.-doctor: pptx needs a Chromium.TestConvert_IntegrationDeck: a deck with a 13-node Mermaid flowchart under a heading, converted with the real mmdc to PDF and PPTX, must not overflow. This also covers what feat: fit Mermaid diagrams within the slide #79 could only test with a stand-in SVG.Tests
pptx_test.go(no browser): slide count andsldSz12192000×6858000 / 9144000×6858000 EMU, full-bleed picture, notes only where there are notes and escaped, no notes parts at all without notes, title escaped incore.xml, empty deck rejected, andTestWritePPTX_PackageIsConsistent— every XML part well-formed, every part has a content type, every override names a real part, every relationship target exists.pptxout_test.go(real Chrome): 3 slides → 3 PNGs of 2560×1440 that differ from each other, notes on slide 2, title; 4:3 → 1920×1440; plain file is still a deck; overflow warning; paper flags rejected.-doctortables extended.go vet ./...,go test ./...,golangci-lint run ./...(0 issues).Not verified
main.Review focus
🤖 Generated with Claude Code