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

[Naoto Sato]

On Wed, 31 Jan 2024 22:24:14 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"

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

> 368:  *
> 369:  * MessageFormat provides patterns that support both the {@link java.time} package
> 370:  * and the {@link Date java.util.Date} type. Consider the following three examples,

Might read better with "patterns that support the date/time formatters in the java.time.format and java.text packages"

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

> 373:  * <p>1) a <i>date</i> {@code FormatType} with a <i>full</i> {@code FormatStyle},
> 374:  * {@snippet lang=java :
> 375:  * Object[] arg = {new Date(2023, 11, 16)};

This is not correct (year and month are wrong), and actually should not be used in an example as the 3-arg constructor is deprecated. Use `Calendar.getTime()` to obtain a `Date` instead.

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

> 674:      * java.time.format.DateTimeFormatter}. In addition, {@code DateTimeFormatter}
> 675:      * does not implement {@code equals()}, and thus cannot be represented as a
> 676:      * pattern. Any "default"/"medium" styles are omitted according to the specification.

Since `DateTimeFormatter` eventually are converted to j.t.Format with `toFormat()` method, wouldn't it be possible to implement this method for those dtf(s)?

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

> 1866:             throw new IllegalArgumentException();
> 1867:         }
> 1868:     }

Can this be replaced with `Enum.valueOf(String)`, after input is normalized?

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

> 1902:             throw new IllegalArgumentException();
> 1903:         }
> 1904:     }

Same as above.

test/jdk/java/text/Format/MessageFormat/CompactSubFormats.java line 42:

> 40: 
> 41: import static org.junit.jupiter.api.Assertions.assertEquals;
> 42: import static org.junit.jupiter.api.Assertions.assertThrows;

Does not seem to be used in this test

test/jdk/java/text/Format/MessageFormat/CompactSubFormats.java line 62:

> 60:     @Test
> 61:     public void badApplyPatternTest() {
> 62:         // An exception won't be thrown since 'compact_regular' will be interpreted as a

If the test is to ensure the given type is a subformat, the comment and the method name do not seem to represent it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475065503
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475076366
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475100517
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475152563
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475152764
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475153695
PR Review Comment: https://git.openjdk.org/jdk/pull/17663#discussion_r1475159578