On Tue, 23 Jan 2024 22:15:40 GMT, Justin Lu <j...@openjdk.org> wrote:
>> Archie Cobbs has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add @implNote to Javadoc for toPattern().
>
> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line
> 90:
>
>> 88: // A few more test cases from the PR#17416 discussion
>> 89: testRoundTrip(new MessageFormat("Test: {0,number,foo'{'#.00}"));
>> 90: testRoundTrip(new MessageFormat("Test: {0,number,foo'}'#.00}"));
>
> Can we add some more concrete test cases, for example, escaped quotes within
> a MessageFormat pattern (represented by doubled single quotes).
>
> Another funny case is something like a MessageFormat created by `"{0,number,'
> abc }'' ' 0.00}"` which becomes `"{0,number, abc '}''' #0.00}"` after
> created from the value returned by toPattern() from the first MessageFormat.
> The Strings appear really different but format equivalently, it would be nice
> to have suspicious cases like these explicitly defined.
Sure thing - added in 58e8cc68f45.
> test/jdk/java/text/Format/MessageFormat/MessageFormatToPatternTest.java line
> 154:
>
>> 152: // Generate a "random" MessageFormat. We do this by creating a
>> MessageFormat with "{0}" placeholders
>> 153: // and then substituting in random DecimalFormat, DateFormat, and
>> ChoiceFormat subformats. The goal here
>> 154: // is to avoid using pattern strings to construct formats, because
>> they're what we're trying to check.
>
> Can you add a general example of a randomly generated MessageFormat in the
> comments, (I know they vary by quite a lot), but it would be hard for someone
> to piece if together without digging into the code. And unfortunately the
> current toString() of MessageFormat is not implemented, so although JUnit
> prints the MessageFormat out, it's just gibberish.
Added in 58e8cc68f45 as a test case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1464143414
PR Review Comment: https://git.openjdk.org/jdk/pull/17416#discussion_r1464143622
