Hi Naoto,
Looks very good, and a few comments...
CompactNumberFormat.java:
144: "each singular and plural patterns" -> "each singular and plural
pattern"
148: "Should the pattern include" -> "If the pattern includes"
336: Add a "." at the end of the sentence
407/417: If the plural rules is not syntatically correct, how is the
error reported? IllegalArgumentException?
587-588: Can be simplified a bit:
The pattern appears multiple times and could be a utility method.
private String getPrefix(boolean isNegative, int compactDataIndex,
int iPart) {
return (isNegative ? negativePrefixPatterns :
positivePrefixPatterns)
.get(compactDataIndex).get(iPart);
}
1667: Pattern DIGITS; is there a pattern syntax to surround the inf and
nan parts
to ensure they are treated as literals?
Can Pattern.compile throw? The parse(text,pos) method does not declare
IllegalArgumentException.
It might be clearer to check for inf/nan outside the digits parse pattern.
2378: Using == on doubles seems like a risky choice but may be ok given
that the input is integers, not floating point.
2350-2353 Looks ok like a lot of pattern matching on every parse.
If performance turns out to be an issue, it should be possible to parse
and generate a tree of lambdas with bindings to the literal values.
The lambda expressions could be saved and re-used.
TestEquality.java:
51: please break the line into reasonable line lengths.
Generated PluralRules.java: Each plural entry has an extra space at the
end of the string;
CLDRConverter.java:
1215: Please trim strings to avoid looking like they are significant.
Regards, Roger
ResourceBundleGenerator.java does not have any changes and may not need
to be in the webrev.
On 11/26/19 4:35 PM, naoto.s...@oracle.com wrote:
I modified CompactNumberFormat.java to simplify the syntax parsing:
https://cr.openjdk.java.net/~naoto/8222756/webrev.01/
Please review this webrev instead.
Naoto
On 11/25/19 1:16 PM, naoto.s...@oracle.com wrote:
Hello,
CompactNumberFormat has been added since JDK 12 to support compact
number formatting, such as 10_000 being formatted as "10K." [1] It
works fine for English, however, not for other languages that take
plural forms in formatted number prefixes/suffixes. In order to fix
this, I filed the following CSR to extend the current
CompactNumberFormat spec:
https://bugs.openjdk.java.net/browse/JDK-8232633
It basically accommodates the plural prefix/suffix forms into the
existing compact patterns array, so that the existing compact number
format works compatibly. The plural rules are solely based on the
CLDR's plural language rules [2]
Here is the implementation of the CSR:
https://cr.openjdk.java.net/~naoto/8222756/webrev.00/
Please review the CSR as well as its implementation.
Naoto
[1] https://bugs.openjdk.java.net/browse/JDK-8177552
[2]
https://unicode.org/reports/tr35/tr35-numbers.html#Language_Plural_Rules
