To add to Roger's comments, the tests should cover parsing as well as
formatting. It is the adjacent parsing behaviour that will be most
important to test and get right.
For example,
Pattern "dMyyyy" cannot be parsed
(is "1122018" the 1st Dec or the 11th Feb?)
But this one should be parsed "ppdppMyyyy"
(" 1122018" has fixed width day = 1 and fixed width month = 12)
("11 22018" has fixed width day =11 and fixed width month = 2)
And as Roger says, this should be tested using
DateTimrFormatterBuilder::padNext as well as the pattern letters.
thanks
Stephen
On 18 April 2018 at 19:34, Roger Riggs <roger.ri...@oracle.com> wrote:
> Hi Pallavi,
>
> The fix here seems to make padding to a fixed width incompatible with
> adjacent value parsing.
> That's not intuitive, since adjacent value parsing is intended to allow more
> flexible parsing
> of a combination leading fixed-width fields and subsequent variable length
> fields.
> The specification of the behavior of padding vs adjacent value parsing
> should be investigated
> and clarified if necessary.
>
> The implementation would be better expressed as checking whether the active
> PrinterParser
> *is* a NumberPrinterParser as a precondition for entering the adjacent
> parsing mode block
> (and the necessary cast).
> And otherwise, fall into the existing code in which the new Parser becomes
> the new active parser.
>
> The tests should be included in the existing test classes for padding, and
> be written using
> the direct DateTimeFormatterBuilder methods (padNext(), instead of the
> patterns) since the patterns
> are just a front end for the builder methods.
> See test/java/time/format/TestPadPrinterDecorator.java
>
> TestDateTimeFormatter.java:
>
> line 96: please keep the static imports for testng together
>
> Line 662: The odd formatting and incorrect indentation should no longer be a
> problem
> because the indentation will not need to change.
>
> Regards, Roger
>
>
>
> On 4/18/18 8:41 AM, Pallavi Sonal wrote:
>>
>> Hi,
>>
>> Please review the changes to the following issue:
>>
>> Bug :https://bugs.openjdk.java.net/browse/JDK-8193877
>>
>> The proposed fix is located at:
>>
>> Webrev :http://cr.openjdk.java.net/~rpatil/8193877/webrev.00/
>>
>>
>> When padding is used in a pattern where there are unpadded values adjacent
>> to padded ones (like "pdQ") , the previous PrinterParser (used for parsing
>> 'd' in the example ) is fetched to use its settings for parsing the next
>> value('Q' in the example). But , in cases like this , it is a
>> PadPrinterDecoratorParser instead of an assumed NumberPrinterParser and a
>> ClassCastException is thrown.
>>
>> The fix has been done to check such cases where the previous parserprinter
>> is PadPrinterDecoratorParser and use the new NumberPrinterParser instead for
>> parsing the next value.
>>
>>
>> All the tier1 and tier2 Mach 5 tests have passed.
>>
>>
>> Thanks,
>>
>> Pallavi Sonal
>>
>>
>
>
