Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lossless Value::Number and allow +unsigned #479

Merged
merged 10 commits into from
Aug 20, 2023

Conversation

juntyr
Copy link
Member

@juntyr juntyr commented Aug 19, 2023

This PR

  • cleans up the value module a bit
  • expands the value::Number enum to cover all possible number types losslessly
  • merges the private parser::AnyNum type with value::Number
  • fixes plus sign on unsigned integers #445 by allowing +unsigned to parse as an unsigned integer

I see this as a small improvement towards a better Value. While it does touch a lot of tests, since Number is more explicit now and always chooses the smallest variant that fits (and serde's Deserialize impls do the rest by upcasting again if necessary), more control could be given to the user with #241, if someone wants to implement that.

  • I've included my change in CHANGELOG.md

@codecov-commenter
Copy link

codecov-commenter commented Aug 19, 2023

Codecov Report

Patch coverage: 75.78% and project coverage change: -2.36% ⚠️

Comparison is base (dea68fe) 85.28% compared to head (c3af695) 82.93%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #479      +/-   ##
==========================================
- Coverage   85.28%   82.93%   -2.36%     
==========================================
  Files          72       74       +2     
  Lines        8850     9257     +407     
==========================================
+ Hits         7548     7677     +129     
- Misses       1302     1580     +278     
Files Changed Coverage Δ
src/value/map.rs 46.26% <46.26%> (ø)
src/de/value.rs 74.06% <63.07%> (-14.46%) ⬇️
src/value/mod.rs 32.36% <66.66%> (-14.90%) ⬇️
src/value/number.rs 71.27% <71.27%> (ø)
src/parse.rs 75.31% <72.88%> (-3.87%) ⬇️
src/de/mod.rs 74.90% <100.00%> (-1.14%) ⬇️
src/de/tests.rs 88.35% <100.00%> (-11.65%) ⬇️
src/ser/tests.rs 86.30% <100.00%> (-13.70%) ⬇️
src/ser/value.rs 30.76% <100.00%> (-19.24%) ⬇️
tests/115_minimal_flattening.rs 100.00% <100.00%> (ø)
... and 6 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@juntyr juntyr changed the title Lossless Value::Number Lossless Value::Number and allow +unsigned Aug 19, 2023
@juntyr juntyr marked this pull request as ready for review August 19, 2023 21:36
@juntyr juntyr self-assigned this Aug 19, 2023
src/parse.rs Show resolved Hide resolved
@juntyr juntyr merged commit b7acecb into ron-rs:master Aug 20, 2023
8 checks passed
@juntyr juntyr deleted the lossless-value-number branch August 20, 2023 22:22
@juntyr juntyr mentioned this pull request Aug 21, 2023
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.

plus sign on unsigned integers
2 participants