| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
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.
|
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.) |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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
Verification