On Mon, 25 Jul 2022 16:10:02 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> This PR is to propose supporting the `T` extension to the BCP 47 to which
>> `java.util.Locale` class conforms. There are two extensions to the BCP 47,
>> one is `Unicode Locale Extension` which has been supported since JDK7, the
>> other is this `Transformed Content` extension. A CSR has also been drafted.
>
> Naoto Sato has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Removed unnecessary `contains()` check
Looks good to me. Some minor comments below.
src/java.base/share/classes/java/util/Locale.java line 236:
> 234: * particular Unicode locale attributes or key/type pairs.
> 235: *
> 236: * <h2><a id="t_extension">Transformed Content extension</a></h2>
Both the full name "Transformed Content extension" and the abbreviated forms "T
Extension" are used in this document. I see Unicode and the RFC referred to it
as "T Extension", " 't' Extension", and "Extension T". If we describe the
extension as "T Extension", maybe we can make it clear in the title, e.g.:
Transformed Content (T) Extension
could thus in the doc thereafter use "T Extension". The existing methods look
like used full extension name, but for T Extension, the short form, e.g.
getTExtensionFields also matches its javadoc ("Returns the map of fields in the
T extension"). Your call.
src/java.base/share/classes/java/util/Locale.java line 265:
> 263: * field separator (one alpha + one digit), followed by one or more
> subtags of the length 3 to 8,
> 264: * each delimited by a hyphen.
> 265: * <p>The transformed content information {@code source} language tag
> and {@code fields} are returned
"transformed content information" seems not needed as "The {@code source}
language tag and {@code fields}" are known from the previous paragraph.
src/java.base/share/classes/sun/util/locale/TransformedContentExtension.java
line 40:
> 38: public final class TransformedContentExtension extends Extension {
> 39:
> 40: public static final char SINGLETON = 't';
"singleton" as in "The singleton identifier" in the doc, seems to mean the key
consists of a single character. But it can be confused with the Singleton
pattern. T_EXTENSION_KEY (as methods such as isTransformedContentxxx expect a
key) or Locale.TRANSFORMED_CONTENT_EXTENSION might be better.
-------------
PR: https://git.openjdk.org/jdk/pull/9620
