Skip to content

[Form] NumberType: Fix parsing of numbers in exponential notation and negative exponent #57860

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

Closed
wants to merge 1 commit into from

Conversation

jbtronics
Copy link
Contributor

Q A
Branch? 5.4, 6.4, 7.0, and 7.1 for bug fixes
Bug fix? yes
New feature? no
Deprecations? no
Issues See below
License MIT

Currently, when inputting a string like "1E-3" into a NumberType leads incorrectly to a result of "0". The correct result would be "0.001", which is also the result, when inputting "1.0E-3".

This is caused by NumberToLocalizedStringTransformer, which assumes that the result must be a integer, if the string does not contain the decimal separator. However, this behavior is incorrect, if the string is in exponential notation with negative exponent.
The NumberFormatter can correctly parse this format, but it needs to be told that the result should be a double (otherwise its gets incorrectly rounded to 0)

To fix this behavior, I added a check for the negative exponential format, which then results in a double result.

A value like 1E-3 results in a float value, even if it not contains the decimal separator. NumberFormatter can parse this correctly, but we need to pass TYPE_DOUBLE. Before it were only used when the string contained the decimal separator, therefore the TYPE_INT was used before (which resulted in the inproper result of 0).
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.2" but it seems your PR description refers to branch "5.4, 6.4, 7.0, and 7.1 for bug fixes".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 7.2 milestone Jul 28, 2024
@jbtronics jbtronics changed the base branch from 7.2 to 7.1 July 28, 2024 20:08
@jbtronics jbtronics changed the base branch from 7.1 to 7.2 July 28, 2024 20:08
@jbtronics jbtronics closed this Jul 28, 2024
@jbtronics
Copy link
Contributor Author

Replaced by #57861

As this PR is against the wrong branch (7.2 instead of 7.1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants