Split out invoice specific logic and models from core and render.#24
Split out invoice specific logic and models from core and render.#24
Conversation
…rom core/render - preparing for reuse for other documents than invoices.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the codebase to separate invoice-specific logic from generic PDF rendering capabilities, enabling future reuse for other document types beyond invoices.
- Split
klirr-coreintoklirr-core-invoice(invoice domain logic) andklirr-core-pdf(generic PDF primitives) - Split
klirr-renderintoklirr-render-invoice(invoice rendering) andklirr-render-pdf(generic Typst-based PDF rendering) - Introduced
DocumentPlanandInlineModuleabstractions to replace invoice-specific rendering context
Reviewed Changes
Copilot reviewed 87 out of 171 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/render-pdf/Cargo.toml | New crate for generic PDF rendering with Typst dependencies |
| crates/render-pdf/src/lib.rs | Exports generic document rendering API |
| crates/render-pdf/src/module.rs | Defines DocumentPlan and InlineModule for flexible document composition |
| crates/render-pdf/src/render.rs | Generic render_document function replacing invoice-specific implementation |
| crates/render-pdf/src/typst_context/* | Refactored Typst context to work with generic modules instead of fixed invoice sources |
| crates/core-pdf/Cargo.toml | New crate for PDF-focused primitives |
| crates/core-pdf/src/* | Extracted font handling, Typst conversion traits, and PDF type from core |
| crates/render-invoice/* | Updated to use new generic rendering layer via DocumentPlan |
| crates/core-invoice/* | Renamed from klirr-core with updated import paths |
| crates/cli/* | Updated imports to reference renamed crates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if let Some(font) = self.environment().fonts().get(index).cloned() { | ||
| Some(font) | ||
| } else { | ||
| self.environment().fonts().get(index).cloned().or_else(|| { |
There was a problem hiding this comment.
The or_else closure will always panic, making the cloned() operation pointless. The panic should occur when get(index) returns None, not inside or_else. Consider using unwrap_or_else or restructuring to: self.environment().fonts().get(index).cloned().unwrap_or_else(|| panic!(\"Font not found at index: {}\", index))
| self.environment().fonts().get(index).cloned().or_else(|| { | |
| self.environment().fonts().get(index).cloned().unwrap_or_else(|| { |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
==========================================
- Coverage 95.74% 94.41% -1.33%
==========================================
Files 92 100 +8
Lines 2113 2186 +73
==========================================
+ Hits 2023 2064 +41
- Misses 90 122 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fa49646 to
d93e8b1
Compare
fd16b99 to
e9e1395
Compare
e9e1395 to
a4daab7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 91 out of 176 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Note
ChatGPT authored the commits of this PR.
Preparing for reuse for other documents than invoices.