FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Scope temp Typst cleanup to compile call, fix #730 by vishnujayvel · Pull Request #760 · rendercv/rendercv · GitHub

Scope temp Typst cleanup to compile call, fix #730 - #760

Open
vishnujayvel wants to merge 1 commit into
rendercv:mainfrom
vishnujayvel:fix/730-scope-typst-cleanup
Open

Scope temp Typst cleanup to compile call, fix #730#760
vishnujayvel wants to merge 1 commit into
rendercv:mainfrom
vishnujayvel:fix/730-scope-typst-cleanup

Conversation

Copy link
Copy Markdown

Problem

Closes #730 (reported by @AndDeluiz). When settings.render_command.dont_generate_typst is True and PDF/PNG generation is enabled, generate_pdf/generate_png silently skip compilation instead of producing output. The Typst source is required to compile the PDF/PNG, but the code path that should provide it when the standalone .typ file isn't being persisted was never wired up.

As @sinaatalay noted on the issue, the right fix is to generate the Typst source into a temporary location whenever it's needed for compilation, regardless of whether the user wants the standalone file kept around.

Fix

ensure_typst_source_for_compilation is now a @contextlib.contextmanager backed by tempfile.TemporaryDirectory(). generate_pdf/generate_png use it with a with block, so the temporary Typst source directory is created, used for compilation, and removed within the scope of that single compile call — on both success and error — rather than relying on a longer-lived temp dir.

This directly addresses the concern @sinaatalay raised on #577 ("Are we sure those temp dirs get removed at the end of the run?"), which along with the related #605 was closed over uncertainty about whether mkdtemp + atexit-based cleanup actually ran. Scoping cleanup to a context manager means it happens deterministically at the end of the with block, not deferred to process exit.

Also corrected the dont_generate_typst field description and CLI help text, which described it as controlling whether Typst (and therefore PDF/PNG) generation happens at all — it only controls whether the standalone .typ file is persisted to disk. Regenerated schema.json via just update-schema to match.

Changes

  • src/rendercv/renderer/pdf_png.py: ensure_typst_source_for_compilation converted to a context manager over a TemporaryDirectory; generate_pdf/generate_png updated to use it via with.
  • src/rendercv/schema/models/settings/render_command.py and src/rendercv/cli/render_command/render_command.py: corrected dont_generate_typst description/help text.
  • schema.json: regenerated from the corrected model.
  • tests/renderer/test_pdf_png.py: added coverage for temp dir cleanup on both success and error, confirmed a persisted typst_path is left untouched, and an end-to-end check that generate_pdf leaves no temp dir behind.

Verification

  • uv sync --frozen --all-extras — installs cleanly (submodules, including typst_fontawesome, need git submodule update --init --recursive first).
  • uv run --frozen --all-extras pytest tests/renderer/test_pdf_png.py tests/cli/render_command/test_render_command.py — 56 passed, 0 failed.
  • uv run --frozen --all-extras ruff check on all files touched by this patch — all checks passed.

Address review must-fixes on the rendercv#730 fix:

- ensure_typst_source_for_compilation is now a context manager backed by
  tempfile.TemporaryDirectory instead of mkdtemp + atexit. The temp Typst
  source is created, compiled, and removed within generate_pdf/generate_png's
  `with` block, on both success and error, rather than lingering until
  process exit (the exact concern @sinaatalay raised on rendercv#577).
- Corrected the dont_generate_typst field description and CLI help text:
  it only controls whether the standalone .typ file is persisted, not
  whether PDF/PNG compile. Regenerated schema.json via `just update-schema`.
- Added unit tests covering temp dir cleanup on success/error, that a
  persisted typst_path is left untouched, and an end-to-end check that
  generate_pdf leaves no temp dir behind.

Copy link
Copy Markdown
Author

The failing Run pre-commit checks job looks unrelated to this PR: the prek hook installs floating ty>=0.0.24 (currently resolving to 0.0.61) while uv.lock pins 0.0.24, and the newer ty's 36 diagnostics (unused-ignore-comment, stricter invalid-type-form/invalid-argument-type) reproduce identically on unmodified main — none of them are in files this PR touches. Happy to open a separate, scoped PR upgrading ty and clearing those diagnostics if that's useful; keeping it out of this one so the diff stays focused on #730. (The review job failure is org-side — empty ANTHROPIC_API_KEY in the workflow.)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDF file isn't generated if dont_generate_typst is set to true

1 participant


Back | FazBrowse Home | New Git URL