CLDR-16279 fix random spacing in the Numbers report for compact currency #3729
Conversation
This file contains 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
pedberg-icu
requested review from
srl295,
macchiati,
richgillam and
DraganBesevic
macchiati
approved these changes
May 17, 2024
|
After merging, verified in smoketest that this fixes the spacing issue in the Compact Numbers report. |
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.
CLDR-16279
The Numbers report in Survey Tool (and Charts) is generated by util/VerifyCompactNumbers.java, which calls test/BuildIcuCompactDecimalFormat.java to assemble the formats. BuildIcuCompactDecimalFormat.buildCustomData() was iterating over the paths for compact number formats without paying attention to which ones were alt='alphaNextToNumber' (with spacing) and non-alt (normally without spacing), leading to random use of space in compact currency formats in the report (see the Numbers report for en_CA, for example). This PR fixes BuildIcuCompactDecimalFormat.buildCustomData() to explicitly choose the alt='alphaNextToNumber' format when appropriate (currency symbol has letter on side adjacent to value), and to skip the alt format (and use the standard format) otherwise.
It is possible that a similar change will also be needed in util/ICUServiceBuilder.java
ALLOW_MANY_COMMITS=true