Re: RFR: JDK-8321545: Override toString() for Format subclasses [v3]
On Fri, 12 Jan 2024 18:27:55 GMT, Justin Lu wrote:
>> Please review this PR which implements toString() for the `Format`
>> subclasses. Corresponding CSR:
>> [JDK-8323088](https://bugs.openjdk.org/browse/JDK-8323088)
>>
>> The general specification follows a template that provides the locale (if
>> the class is localized) and any relevant patterns. The specification was
>> intentionally kept minimal and deliberately worded as "for debugging".
>>
>> An example of all the classes has output such as
>>
>>
>> CompactNumberFormat [locale: "English (United States)", decimal pattern:
>> "foo#0.00#baz", compact patterns: "[, , , {one:0K other:0K}, {one:00K
>> other:00K}, {one:000K other:000K}, {one:0M other:0M}, {one:00M other:00M},
>> {one:000M other:000M}, {one:0B other:0B}, {one:00B other:00B}, {one:000B
>> other:000B}, {one:0T other:0T}, {one:00T other:00T}, {one:000T other:000T}]"]
>>
>> DecimalFormat [locale: "English (United States)", pattern: "foo#0.00#baz"]
>>
>> SimpleDateFormat [locale: "Chinese (China)", pattern: "EEE, MMM d, ''yy"]
>>
>> ListFormat [locale: "English (United States)", start: "{0}, {1}", middle:
>> "{0}, {1}", end: "{0}, and {1}", two: "{0} and {1}", three: "{0}, {1}, and
>> {2}"]
>>
>> MessageFormat [locale: "Chinese (China)", pattern: "foo {0}"]
>>
>> ChoiceFormat [pattern: "0#foo"]
>
> Justin Lu has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - swap placement of decimal pattern and compact patterns. Expand on tests
> - add unit tests
Added tests look good.
src/java.base/share/classes/java/text/MessageFormat.java line 1195:
> 1193: """
> 1194: MessageFormat [locale: "%s", pattern: "%s"]
> 1195: """.formatted(locale == null ? "None" :
> locale.getDisplayName(), toPattern());
I think printing `null` for `locale == null` is better here, as `None` is sort
of ambiguous. Applies to SDF (and related tests) as well.
-
PR Review: https://git.openjdk.org/jdk/pull/17355#pullrequestreview-1818845217
PR Review Comment: https://git.openjdk.org/jdk/pull/17355#discussion_r1450811275
Re: RFR: JDK-8321545: Override toString() for Format subclasses [v3]
> Please review this PR which implements toString() for the `Format`
> subclasses. Corresponding CSR:
> [JDK-8323088](https://bugs.openjdk.org/browse/JDK-8323088)
>
> The general specification follows a template that provides the locale (if the
> class is localized) and any relevant patterns. The specification was
> intentionally kept minimal and deliberately worded as "for debugging".
>
> An example of all the classes has output such as
>
>
> CompactNumberFormat [locale: "English (United States)", decimal pattern:
> "foo#0.00#baz", compact patterns: "[, , , {one:0K other:0K}, {one:00K
> other:00K}, {one:000K other:000K}, {one:0M other:0M}, {one:00M other:00M},
> {one:000M other:000M}, {one:0B other:0B}, {one:00B other:00B}, {one:000B
> other:000B}, {one:0T other:0T}, {one:00T other:00T}, {one:000T other:000T}]"]
>
> DecimalFormat [locale: "English (United States)", pattern: "foo#0.00#baz"]
>
> SimpleDateFormat [locale: "Chinese (China)", pattern: "EEE, MMM d, ''yy"]
>
> ListFormat [locale: "English (United States)", start: "{0}, {1}", middle:
> "{0}, {1}", end: "{0}, and {1}", two: "{0} and {1}", three: "{0}, {1}, and
> {2}"]
>
> MessageFormat [locale: "Chinese (China)", pattern: "foo {0}"]
>
> ChoiceFormat [pattern: "0#foo"]
Justin Lu has updated the pull request incrementally with two additional
commits since the last revision:
- swap placement of decimal pattern and compact patterns. Expand on tests
- add unit tests
-
Changes:
- all: https://git.openjdk.org/jdk/pull/17355/files
- new: https://git.openjdk.org/jdk/pull/17355/files/1dfc949d..3157ec8e
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk=17355=02
- incr: https://webrevs.openjdk.org/?repo=jdk=17355=01-02
Stats: 359 lines in 7 files changed: 357 ins; 0 del; 2 mod
Patch: https://git.openjdk.org/jdk/pull/17355.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17355/head:pull/17355
PR: https://git.openjdk.org/jdk/pull/17355
