RFR: 8341923: java.util.Locale class specification improvements [v2]

[Justin Lu]

On Mon, 18 Nov 2024 20:37:27 GMT, Chen Liang <liach at openjdk.org> wrote:

>> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   address Chen's review; make other similar changes
>
> src/java.base/share/classes/java/util/Locale.java line 94:
> 
>> 92:  * <p> A {@code Locale} is composed of the bolded fields described below; note that a
>> 93:  * {@code Locale} need not have all such fields. For example, {@link
>> 94:  * Locale#ENGLISH Locale.ENGLISH} is only comprised of the <em>language</em> field.
> 
> Side remark: in the `Locale` class itself, you can just use `{@link #ENGLISH Locale.ENGLISH}` to generate the link.

Thanks for taking a look Chen.

https://github.com/openjdk/jdk/pull/22192/commits/6b5b5cd067a42dbb98033314851255841e0d91fb should address your comments, and includes other changes that are in line with your suggestions. The apidiff and javadoc links have also been refreshed.

> Update: Didn't notice you posted apidiff and javadoc beforehand:
> 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

(Sorry, I should have linked these directly on this PR).

> src/java.base/share/classes/java/util/Locale.java line 120:
> 
>> 118:  *
>> 119:  *   <dd> <em>Syntax:</em> Well-formed {@code language} values have the form {@code [a-zA-Z]{2,8}}.
>> 120:  *   BCP 47 deviation: this is not the full BCP 47 language production, since it excludes
> 
> Should we add `<br>` before these deviations for more straightforward rendering?

Good point. Went ahead and just prefixed with a `<dd>` instead (for consistency).

> src/java.base/share/classes/java/util/Locale.java line 500:
> 
>> 498:  * <p>Because of these issues, it is recommended that apps migrate
>> 499:  * away from constructing non-conforming locales and use the
>> 500:  * {@link #forLanguageTag} and {@link Locale.Builder} APIs instead.
> 
> Same remarks for links, add a override render text to prevent javadoc from generating long parameter lists.

I gave the `Locale.of` links an explicit render. I think the `forLanguageTag` links were fine w/o an explicit render, as long as `{@link #forLanguageTag(String)}` was used, not `{@link #forLanguageTag}` (so that the fully qualified class name is omitted).

On a side note, the default render was pretty nasty for the `Locale.LanguageRange` see tags.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847824296
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847826300
PR Review Comment: https://git.openjdk.org/jdk/pull/22192#discussion_r1847831453