Decimal #11
Merged
Decimal #11
Conversation
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
+ Coverage 97.25% 97.40% +0.14%
==========================================
Files 59 59
Lines 1312 1308 -4
==========================================
- Hits 1276 1274 -2
+ Misses 36 34 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR replaces the custom F64 wrapper with a new Decimal type backed by rust_decimal::Decimal, migrating all monetary and quantity models to use high-precision decimals via the dec! macro.
- Introduces
models/decimal.rsand addsdecto the prelude for easy decimal literals - Updates
UnitPrice,Quantity,Cost, exchange-rate logic, item builders, serialization, and tests to useDecimalanddec! - Renames the old
F64wrapper inf64_eq.rstoDecimal, which now conflicts with the newDecimaltype
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/core/src/models/unit_price.rs | Switched UnitPrice from F64 to new Decimal, updated tests |
| crates/core/src/models/quantity.rs | Switched Quantity to Decimal, added ONE constant |
| crates/core/src/models/cost.rs | Switched Cost to Decimal |
| crates/core/src/models/decimal.rs | New wrapper for rust_decimal::Decimal with serde + RON support |
| crates/core/src/models/f64_eq.rs | Renamed F64 to Decimal but now collides with new Decimal |
| crates/core/src/models/error.rs | Added InvalidDecimalConversion, updated parse error comments |
| crates/core/src/logic/serde_to_typst.rs | Enhanced RON→Typst conversion for decimal strings |
Comments suppressed due to low confidence (1)
crates/core/src/models/f64_eq.rs:26
- The old
F64wrapper was renamed toDecimal, but there is already a newDecimaltype inmodels/decimal.rs. Consider giving this wrapper a distinct name (e.g.,F64orF64Bits) or renaming the file to match.
pub struct Decimal(f64);
…l::Decimal which serializes into a number not a string
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.
Remove custom F64 type, replaced with
Decimalwhich is a wrapper aroundrust_decimal::Decimal- changing the serde to serialise into a f64, not a string, for simplest possible bridging into Typst dictionary. In the future if anyone needs more precision than f64 we can remove the wrapper and just use Decimal as is, and let it serialize into a string, and write a more advanced bridge to Typst than going via JSON.