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

Fraction Formatting #2254

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Fraction Formatting #2254

merged 3 commits into from
Aug 18, 2021

Conversation

oleibman
Copy link
Collaborator

@oleibman oleibman commented Aug 6, 2021

See issue #2253. User's analysis was correct - leading zeros in the decimal portion were being stripped out, so 0.0625 and 0.625 were being treated the same. As it turns out, integers also aren't handled well (0 0/1 anyone?). The latter problem had been hidden because caller tested for integer first and skipped call if true; but FractionFormatter::format is public and should work correctly regardless. All Phpstan baseline entries for FractionFormatter and NumberFormatter are eliminated. New test data is added; no need for changes to test code.

This is:

- [x] a bugfix
- [ ] a new feature

Checklist:

Why this change is needed?

See issue PHPOffice#2253. User's analysis was correct - leading zeros in the decimal portion were being stripped out, so 0.0625 and 0.625 were being treated the same. As it turns out, integers also aren't handled well (`0 0/1` anyone?). The latter problem had been hidden because caller tested for integer first and skipped call if true; but FractionFormatter::format is public and should work correctly regardless. All Phpstan baseline entries for FractionFormatter and NumberFormatter are eliminated. New test data is added; no need for changes to test code.
Ensure result is string.
@oleibman oleibman merged commit 710f9f1 into PHPOffice:master Aug 18, 2021
@oleibman oleibman deleted the fixfrac branch August 27, 2021 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant