<i18n dev> RFR: 8323699: MessageFormat.toPattern() generates non-equivalent MessageFormat pattern [v7]

Archie Cobbs acobbs at openjdk.org
Fri Feb 2 01:57:19 UTC 2024


On Thu, 1 Feb 2024 23:02:59 GMT, Justin Lu <jlu at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix misspelling in comment.
>
> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line 26:
> 
>> 24: /*
>> 25:  * @test
>> 26:  * @summary Verify MessageFormat.toPattern() is equivalent to original pattern
> 
> Could make the summary a little more specific for future readers

Thanks, should be fixed.

> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line 70:
> 
>> 68:     @BeforeAll
>> 69:     public static void setup() {
>> 70:         savedLocale = Locale.getDefault();
> 
> I'm not sure we need to save the default locale and restore it, unless I'm missing something.

We are verifying output that includes floating point numbers, and the current locale affects that:

jshell> Locale.setDefault(Locale.US);

jshell> new MessageFormat("{0}").format(new Object[] { 1.23 });
$9 ==> "1.23"

jshell> Locale.setDefault(Locale.FRENCH);

jshell> new MessageFormat("{0}").format(new Object[] { 1.23 });
$11 ==> "1,23"

> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line 104:
> 
>> 102:             Arguments.of("{0,choice,0.0#option A: {0}|1.0#option B: {0}'}'}", "option B: 1.23}"),
>> 103:             Arguments.of("{0,choice,0.0#option A: {0}|2.0#option B: {0}'}'}", "option A: 1.23"),
>> 104: 
> 
> Suggestion:
> 
> // Absurd double quote examples
> Arguments.of("Foo '}''''''''}' {0,number,bar'}' '}' } baz ", "Foo }''''} bar} } 1 baz "),
> Arguments.of("'''}''{'''}''''}'"), "'}'{'}''}"),

Thanks, should be fixed.

> test/jdk/java/text/Format/MessageFormat/MessageFormatsByArgumentIndex.java line 35:
> 
>> 33: import java.text.NumberFormat;
>> 34: 
>> 35: public class MessageFormatsByArgumentIndex {
> 
> Can you bump the latter copyright year for this file

Thanks, should be fixed.

> test/jdk/java/text/Format/MessageFormat/MessageRegression.java line 114:
> 
>> 112:      * MessageFormat.toPattern has weird rounding behavior.
>> 113:      */
>> 114:     @Test
> 
> This file as well

Thanks, should be fixed.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1475420027
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1475420069
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1475419965
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1475420147
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1475420112


More information about the i18n-dev mailing list