<i18n dev> RFR: JDK-8318761: MessageFormat pattern support for CompactNumberFormat, ListFormat, and DateTimeFormatter [v6]

[Naoto Sato]

On Thu, 8 Feb 2024 20:37:18 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Please review this PR and [CSR](https://bugs.openjdk.org/browse/JDK-8319344) which adds MessageFormat pattern support for the following subformats: ListFormat, CompactNumberFormat, and DateTimeFormatter. This change is intended to provide pattern support for the more recently added JDK Format subclasses, as well as improving java.time formatting within i18n. The draft javadoc can be viewed here: https://cr.openjdk.org/~jlu/docs/api/java.base/java/text/MessageFormat.html. Please see the CSR for more in-depth behavioral changes, as well as limitations.
>> 
>> The `FormatTypes`:  dtf_date, dtf_time, dtf_datetime, pre-defined DateTimeFormatter(s), and list are added.
>> The `FormatStyles`: compact_short, compact_long, or, and unit are added.
>> 
>> For example, previously,
>> 
>> 
>> Object[] args = {LocalDate.of(2023, 11, 16), LocalDate.of(2023, 11, 27)};
>> MessageFormat.format("It was {0,date,full}, now it is {1,date,full}", args);
>> 
>> 
>> would throw `Exception java.lang.IllegalArgumentException: Cannot format given Object as a Date`
>> 
>> Now, a user can call
>> 
>> 
>> MessageFormat.format("It was {0,dtf_date,full}, now it is {1,dtf_date,full}", args);
>> 
>> 
>> which returns "It was Thursday, November 16, 2023, now it is Friday, November 17, 2023"
>
> Justin Lu has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Move the if-else nFmt checking to a package-private method in NumberFormat

Looks much better. Left some nits.

src/java.base/share/classes/java/text/MessageFormat.java line 745:

> 743:             String nStyle = NumberFormat.matchToStyle(nFmt, locale);
> 744:             if (nStyle != null) {
> 745:                 return ",number" + (nStyle.equals("") ? nStyle : "," + nStyle);

`nStyle.isEmpty()` may read better

src/java.base/share/classes/java/text/MessageFormat.java line 780:

> 778:             }
> 779:         }
> 780:         else if (fmt != null) {

This `else if` does not seem necessary

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

PR Review: https://git.openjdk.org/jdk/pull/17663#pullrequestreview-1871266747
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1483596204
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1483599292