Skip to content

fix: Fixes pdf export via cli (on windows) - #235

Merged
RDMillen merged 5 commits into
KovaMD:mainfrom
florian-hubertSE:bugfix/pdf-export-on-cli
Sep 2, 2026
Merged

fix: Fixes pdf export via cli (on windows)#235
RDMillen merged 5 commits into
KovaMD:mainfrom
florian-hubertSE:bugfix/pdf-export-on-cli

Conversation

@florian-hubertSE

@florian-hubertSE florian-hubertSE commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Fixes missing images (if used with relative pathes) and diagrams in pdf while using the cli, instead of the export button.

Before

image

after

image

@RDMillen RDMillen self-assigned this Sep 1, 2026
@RDMillen RDMillen added the bug Something isn't working label Sep 1, 2026
@RDMillen

RDMillen commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Hi @florian-hubertSE,

Thank you for catching this and submitting a PR. Will get this reviewed and tested for the next release.

Thank for your submissions,

Ross

RDMillen added a commit to florian-hubertSE/Kova that referenced this pull request Sep 1, 2026
Drop the zero-diagram onAllDiagramsReady guard added here. It is
unrelated to showing a bg image behind a bare title slide, and the
same export-readiness change is being handled in KovaMD#235.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@RDMillen

RDMillen commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Hi @florian-hubertSE,

Thanks for this, the \\?\ fix in cli.rs is spot on. A few things to sort before it can go in:

  1. hasPendingLocalMedia in App.tsx only checks for asset://, but on Windows convertFileSrc returns http://asset.localhost/..., so the gate never fires on the platform this PR targets. Match both forms, or derive the pending state from the resolved URL map in useResolvedSlides rather than sniffing the placeholder scheme.

  2. Nothing bounds the wait. If read_file_b64 fails for a missing or mistyped path, the src stays a placeholder, hasPendingLocalMedia never clears, and kova --export hangs silently on macOS and Linux. The elements.tsx change has the same shape, a first render cancelled before it caches an SVG stops advancing the ready count. Please add a timeout to the export readiness path so a deck with a broken image or diagram still exports.

  3. Please revert the formatting changes in src/engine/export/exportPdfNative.ts and src/hooks/useResolvedSlides.ts. git diff -w shows no functional change in either file, it buries the real change and will clash with other branches. Keep the diff to cli.rs, App.tsx, SlideRenderer.tsx and elements.tsx.

  4. strip_verbatim_prefix duplicates the \\?\ handling already in commands/window.rs. Please lift it into one shared helper.

Thanks,
Ross

@florian-hubertSE

Copy link
Copy Markdown
Contributor Author

Hi @RDMillen,

thanks for your review feedback.

I have adapted the PR in accordance to your comments and hope everything fits to your needs. I have tested it locally on my windows system, and functional-wise it looks good.

Best,
Florian

The separating blank line and the safe_write_path doc comment were
collapsed onto the closing brace during the review edits.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@RDMillen

RDMillen commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Hi @florian-hubertSE,

All four points look well handled, thank you. I have pushed one tidy-up commit: the blank line and doc comment between safe_read_path and safe_write_path in file_io.rs had been collapsed onto the closing brace. Nothing functional.

Will squash-merge once CI is green so it makes the next release.

Thanks,
Ross

@RDMillen
RDMillen merged commit a281ff5 into KovaMD:main Sep 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants