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

ICU-22303 Support parsing infinity/NaN when decimal pattern match is required #2427

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stevedlawrence
Copy link

If a DecimalFormat pattern contains a decimal point and setDecimalPatternMatchRequired is true, then DecimalFormat parse() fails to parse infinity/NaN representations. This is because infinfity/NAn parsing does not set the HAS_DECIMAL_SEPARATOR_FLAG and so the RequireDecimalSeparatorValidator fails.

This modifies the RequireDecimalSeparatorValidator so that it does not fail if the INFINITY or NAN flags are set, making it so decimal separators are not required if the infinity/NaN representations are parsed.

Checklist
  • Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22303
  • Required: The PR title must be prefixed with a JIRA Issue number.
  • Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
  • Required: Each commit message must be prefixed with a JIRA Issue number.
  • Issue accepted (done by Technical Committee after discussion)
  • Tests included, if applicable
  • API docs and/or User Guide docs changed or added, if applicable

@CLAassistant
Copy link

CLAassistant commented Apr 13, 2023

CLA assistant check
All committers have signed the CLA.

sffc
sffc previously approved these changes Apr 20, 2023
Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Praise: Clean solution consistent with the architecture of NumberParser

richgillam
richgillam previously approved these changes Apr 20, 2023
Copy link
Contributor

@richgillam richgillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me too. I'll merge in a day or so, to give Markus time to check for CLAs.

macchiati
macchiati previously approved these changes May 25, 2023
@stevedlawrence stevedlawrence dismissed stale reviews from macchiati, richgillam, and sffc via 93a09e9 October 27, 2023 21:23
@stevedlawrence stevedlawrence force-pushed the icu-22303-decimal-inf-nan branch from 1bdfa13 to 93a09e9 Compare October 27, 2023 21:23
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/test/intltest/numfmtst.cpp is different
  • icu4j/main/classes/core/src/com/ibm/icu/impl/number/parse/RequireDecimalSeparatorValidator.java is no longer changed in the branch
  • icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberFormatTest.java is now changed in the branch
  • icu4j/main/core/src/main/java/com/ibm/icu/impl/number/parse/RequireDecimalSeparatorValidator.java is now changed in the branch
  • icu4j/main/tests/core/src/com/ibm/icu/dev/test/format/NumberFormatTest.java is no longer changed in the branch

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@stevedlawrence
Copy link
Author

FYI, this PR had some merge conflicts due to the mavenization of the repo. I rebased onto the latest main branch with no changes needed and force pushed to resolve the conflicts.

Copy link
Contributor

@richgillam richgillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix still looks good.

…required

If a DecimalFormat pattern contains a decimal point and
setDecimalPatternMatchRequired is true, then DecimalFormat parse() fails
to parse infinity/NaN representations. This is because infinity/NaN
parsing does not set the HAS_DECIMAL_SEPARATOR_FLAG and so the
RequireDecimalSeparatorValidator fails.

This modifies the RequireDecimalSeparatorValidator so that it does not
fail if the INFINITY or NAN flags are set, making it so decimal
separators are not required if the infinity/NaN representations are
parsed.
@stevedlawrence stevedlawrence force-pushed the icu-22303-decimal-inf-nan branch from 93a09e9 to e7ca1a2 Compare May 20, 2024 12:10
@jira-pull-request-webhook
Copy link

Notice: the branch changed across the force-push!

  • icu4c/source/test/intltest/numfmtst.cpp is different
  • icu4j/main/common_tests/src/test/java/com/ibm/icu/dev/test/format/NumberFormatTest.java is different

View Diff Across Force-Push

~ Your Friendly Jira-GitHub PR Checker Bot

@stevedlawrence
Copy link
Author

FYI, this PR was quite a bit behind main so I rebased it onto the latest master.

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.

5 participants