RFR: 8341923: java.util.Locale class specification improvements [v2]
Naoto Sato
naoto at openjdk.org
Tue Nov 19 21:03:07 UTC 2024
On Tue, 19 Nov 2024 07:58:22 GMT, Justin Lu <jlu at openjdk.org> wrote:
>> Please review this PR and corresponding CSR which includes a wide range of specification improvements for java.util.Locale. See the CSR for further detail. Other changes/suggestions are welcomed to be included as part of this change.
>>
>> APIDiff: https://cr.openjdk.org/~jlu/8341923_apidiff/java.base/java/util/Locale.html
>> Javadoc: https://cr.openjdk.org/~jlu/api/java.base/java/util/Locale.html
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
>
> address Chen's review; make other similar changes
Looks good. Some comments that I missed in the initial (internal) review.
src/java.base/share/classes/java/util/Locale.java line 89:
> 87: * exchange. Each {@code Locale} is associated with locale data which is retrieved
> 88: * by the installed {@link java.util.spi.LocaleServiceProvider LocaleServiceProvider}
> 89: * implementations. Depending on the implementation, such data may vary by release.
This reads like locale data are only obtained with the SPI implementations that are provided by the users. The locale data also (and mainly) come from within Java runtime itself. (BTW, "installed" should be removed, as it is no longer correct. Same applies to getAvailableLocales()/availableLocales() methods)
src/java.base/share/classes/java/util/Locale.java line 98:
> 96: * Locale.forLanguageTag("en-Latn-US-POSIX-u-nu-latn")} would be comprised of all
> 97: * the fields below. This particular {@code Locale} would represent English in
> 98: * the United States using the Latin alphabet and numerics for use in POSIX
"script" is more precise than "alphabet", as the latter suggests only letters [A-Z]
src/java.base/share/classes/java/util/Locale.java line 144:
> 142: * each indicating its own semantics, these values should be ordered
> 143: * by importance, with most important first, separated by
> 144: * underscore('_'). The variant field is case sensitive.</dd>
This part "separated by underscore('_')" is missing in the revised doc
src/java.base/share/classes/java/util/Locale.java line 216:
> 214: * overloads do not make any syntactic checks on the input.
> 215: *
> 216: * <h3><a id="def_locale_extension">Unicode locale/language extension</a></h3>
Probably using the exact name in the external spec is better [Unicode BCP 47 U Extension](https://unicode.org/reports/tr35/#u_Extension)
src/java.base/share/classes/java/util/Locale.java line 218:
> 216: * <h3><a id="def_locale_extension">Unicode locale/language extension</a></h3>
> 217: *
> 218: * <p>UTS#35, "Unicode Locale Data Markup Language" defines optional
UTS35 (https://unicode.org/reports/tr35/) should be listed with @spec link
src/java.base/share/classes/java/util/Locale.java line 344:
> 342: * to BCP 47 syntax. Use a builder to enforce syntactic restrictions on the input.</dd>
> 343: * </dl>
> 344: * <p>The following are all equivalent:
"all equivalent" seems a bit vague, as the actual implementation is not equivalent (unlike other examples below). Something like "the following all produce equivalent locale objects" or along the lines?
-------------
PR Review: https://git.openjdk.org/jdk/pull/22192#pullrequestreview-2446512394
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1848969909
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849007375
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849018322
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849026657
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849027660
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1849032564
More information about the core-libs-dev
mailing list