Generalize render function, move typst functions to new create founda…#25
Merged
Generalize render function, move typst functions to new create founda…#25
Conversation
…tion, preparing for reuse of them.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Klirr’s Typst/PDF rendering pipeline by extracting shared primitives into a new klirr-foundation crate, generalizing the Typst render entrypoint for reuse, and updating the CLI + docs to the new API and naming (L18n → L10n, render_invoice → render).
Changes:
- Introduce
klirr-foundation(PDF bytes model, font models, Typst conversion traits, shared Typst foundation.typhelpers). - Replace the old render-invoice integration with a generalized
klirr-render-typst::render(...)that accepts generic i18n/data/layout and an error-mapping callback. - Update CLI, core-invoice, docs, and dev tooling (Makefile → Justfile) to use the new crates/types and L10n naming.
Reviewed changes
Copilot reviewed 56 out of 65 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| makefile | Removed Make-based dev commands (replaced by justfile). |
| justfile | Adds Just recipes for generating/opening PDFs and running help. |
| crates/render-typst/src/render_test_helpers.rs | Updates test helpers to use L10n + new render signature. |
| crates/render-typst/src/render.rs | Generalizes Typst rendering and inlines shared foundation.typ module. |
| crates/render-typst/src/lib.rs | Adjusts crate prelude exports after refactor. |
| crates/render-typst/fixtures/expected_services.png | Adds/updates expected rendered output fixture (services). |
| crates/render-typst/fixtures/expected_expenses.png | Adds/updates expected rendered output fixture (expenses). |
| crates/render-typst/Cargo.toml | Renames crate + updates dependencies to klirr-foundation and dev deps. |
| crates/render-pdf/src/typst_context/environment.rs | Switches to prelude-based imports after moving types to foundation. |
| crates/render-pdf/src/typst_context/context.rs | Updates tests to use foundation font types. |
| crates/render-pdf/src/render.rs | Uses foundation Pdf and prelude types. |
| crates/render-pdf/src/module.rs | Refactors imports toward crate prelude. |
| crates/render-pdf/src/lib.rs | Expands prelude to re-export foundation + IndexSet. |
| crates/render-pdf/Cargo.toml | Replaces klirr-core-pdf dependency with klirr-foundation. |
| crates/foundation/src/traits/typst.rs | Moves/extends Typst conversion traits and adds used_fonts() helper. |
| crates/foundation/src/traits/mod.rs | Exposes foundation traits module. |
| crates/foundation/src/traits/font_requiring.rs | Adds FontRequiring trait used by render pipeline. |
| crates/foundation/src/models/pdf.rs | Introduces shared Pdf type + save_pdf helper. |
| crates/foundation/src/models/named_pdf.rs | Introduces generic AbstractNamedPdf<D> model. |
| crates/foundation/src/models/mod.rs | Exposes foundation models module. |
| crates/foundation/src/models/font_weight.rs | Moves FontWeight into foundation. |
| crates/foundation/src/models/font_identifier.rs | Moves FontIdentifier into foundation and fixes asset include paths. |
| crates/foundation/src/lib.rs | Adds foundation crate entry + prelude exports. |
| crates/foundation/layout/test.typ | Renames Typst entry function to render(data, l10n). |
| crates/foundation/layout/foundation.typ | Adds shared Typst helper functions for layouts. |
| crates/foundation/Cargo.toml | Renames klirr-core-pdf → klirr-foundation and broadens deps. |
| crates/core-pdf/src/lib.rs | Removes old core-pdf crate root module. |
| crates/core-invoice/tests/typst_conversion.rs | Updates snapshot test to L10n + foundation ToTypstFn. |
| crates/core-invoice/tests/snapshots/typst_conversion__l10n_english_to_typst.snap | Adds new snapshot for renamed test. |
| crates/core-invoice/src/models/pdf.rs | Switches Pdf to foundation type and keeps sample impl. |
| crates/core-invoice/src/models/named_pdf.rs | Replaces concrete NamedPdf with foundation generic alias. |
| crates/core-invoice/src/models/mod.rs | Removes old l18n/font/pdf modules and exports new l10n module. |
| crates/core-invoice/src/models/layout.rs | Updates layout test layout path + render signature + FontRequiring. |
| crates/core-invoice/src/models/l10n/vendor_info.rs | Renames vendor info model L18n → L10n. |
| crates/core-invoice/src/models/l10n/swedish.rs | Updates Swedish localization constructors to L10n names. |
| crates/core-invoice/src/models/l10n/snapshots/klirr_core_invoice__models__l10n__localization__tests__l10n_swedish.snap | Updates snapshot to L10n paths/type names. |
| crates/core-invoice/src/models/l10n/snapshots/klirr_core_invoice__models__l10n__localization__tests__l10n_english.snap | Updates snapshot to L10n paths/type names. |
| crates/core-invoice/src/models/l10n/mod.rs | Adds new l10n module root + exports. |
| crates/core-invoice/src/models/l10n/localization.rs | Renames L18n → L10n, adds ToTypstFn impl, updates map name. |
| crates/core-invoice/src/models/l10n/line_items.rs | Renames line item localization model L18n → L10n. |
| crates/core-invoice/src/models/l10n/language.rs | Adds Language enum under l10n module (EN/SV). |
| crates/core-invoice/src/models/l10n/invoice_info.rs | Renames invoice info localization model L18n → L10n. |
| crates/core-invoice/src/models/l10n/content.rs | Renames localization content model L18n → L10n and ToTypst marker. |
| crates/core-invoice/src/models/l10n/client_info.rs | Renames client localization model L18n → L10n. |
| crates/core-invoice/src/models/font_weight.rs | Removes old re-export (moved to foundation). |
| crates/core-invoice/src/models/font_identifier.rs | Removes old re-export (moved to foundation). |
| crates/core-invoice/src/models/error.rs | Renames error variant L18nNotFound → L10nNotFound. |
| crates/core-invoice/src/models/data/submodels/email/lettre_bridge.rs | Adjusts attachment conversion to new NamedPdf/Pdf types. |
| crates/core-invoice/src/models/data/data_from_disk_with_items_of_kind.rs | Imports foundation ToTypst and keeps PreparedData Typst marker. |
| crates/core-invoice/src/logic/read_write_data/get_localization.rs | Updates return type and internals to L10n. |
| crates/core-invoice/src/logic/mod.rs | Renames create_pdf module export to create_invoice_pdf. |
| crates/core-invoice/src/logic/create_invoice_pdf.rs | Renames create functions and switches to L10n + foundation save_pdf/Pdf. |
| crates/core-invoice/src/lib.rs | Removes klirr_core_pdf prelude export and adjusts derive_more imports. |
| crates/core-invoice/layouts/aioo.typ | Imports shared Typst foundation helpers and renames render function/l10n usage. |
| crates/core-invoice/Cargo.toml | Switches deps to klirr-foundation (+ render crates) and updates formatting/description. |
| crates/cli/src/run.rs | Updates CLI dispatch to renamed render_invoice_sample functions. |
| crates/cli/src/input/get_input/get_input.rs | Uses explicit InvoiceLayout alias to avoid type confusion. |
| crates/cli/src/dispatch_command.rs | Adds invoice-specific render wrapper mapping render-pdf errors → core-invoice errors. |
| crates/cli/Cargo.toml | Switches from klirr-render-invoice to klirr-render-typst + adds foundation/render-pdf deps. |
| _typos.toml | Updates excluded paths from l18n → l10n locations. |
| HOW_IT_WORKS.md | Updates documentation to L10n + new render function name + justfile mention. |
| Cargo.toml | Updates workspace members/deps (remove core-pdf/render-invoice; add foundation/render-typst). |
| Cargo.lock | Locks dependency graph updates for new crates and removed ones. |
Comments suppressed due to low confidence (1)
crates/render-pdf/src/module.rs:5
crate::prelude::*re-exportsInlineModuleandDocumentPlan(from this module). Importing the prelude here will bring those names into scope and then this module defines them again, which typically causes a duplicate-definition import error (E0255). Prefer importing only what this file needs (e.g.IndexSet,FontIdentifier) instead of the crate prelude.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
5
| use crate::prelude::*; | ||
| use chrono::{DateTime, Local}; | ||
| use getset::Getters; | ||
| use indexmap::IndexSet; | ||
| use klirr_core_pdf::FontIdentifier; | ||
|
|
There was a problem hiding this comment.
IndexSet is imported twice: via use crate::prelude::*; (which re-exports IndexSet) and again via use indexmap::IndexSet;. This will fail to compile with a duplicate import error. Remove the explicit use indexmap::IndexSet; here (or stop re-exporting it in the prelude).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
+ Coverage 94.41% 94.48% +0.07%
==========================================
Files 100 100
Lines 2186 2196 +10
==========================================
+ Hits 2064 2075 +11
+ Misses 122 121 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your 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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…tion, preparing for reuse of them.