I don't think this is quite right.
if ((length > position + 3) && (text.charAt(position + 3) == ':')) {
parseType = 10;
}
This code will *always* select type 10 (colons) if a colon is found at
position+3. Whereas the spec now says that it should only do this if
the pattern is "HH". For other patterns, the colon/no-colon choice is
defined to be based on the pattern.
That said, I'm thinking it is better to make the spec more lenient to
match the behaviour as implemented:
When parsing in lenient mode, only the hours are mandatory - minutes
and seconds are optional. If the character after the hour digits is a colon
then the parser will parse using the pattern "HH:mm:ss", otherwise the
parser will parse using the pattern "HHmmss".
Additional TCKDateTimeFormatterBuilder tests will be needed to
demonstrate the above. There should also be a test for data following
the lenient parse. The following should all succeed:
DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendZoneId();
"+01:00Europe/London"
"+0100Europe/London"
DateTimeFormatterBuilder().parseLenient().appendOffset("HH:MM").appendLiteral(":").appendZoneId();
"+01:Europe/London"
Note this special case, where the colon affects the parse type, but is
not ultimately part of the offset, thus it is left to match the
appendLiteral(":")
You may want to think of some additional nasty edge cases!
Stephen
On 25 February 2016 at 15:44, nadeesh tv <nadeesh...@oracle.com> wrote:
>
> Hi all,
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8032051/webrev.02/
>
> Thanks and Regards,
> Nadeesh
>
> On 2/23/2016 5:17 PM, Stephen Colebourne wrote:
>>
>> Thanks for the changes.
>>
>> In `DateTimeFormatter`, the code should be
>>
>> .parseLenient()
>> .appendOffsetId()
>> .parseStrict()
>>
>> and the same in the other case. This ensures that existing callers who
>> then embed the formatter in another formatter (like the
>> ZONED_DATE_TIME constant) are unaffected.
>>
>>
>> The logic for lenient parsing does not look right as it only handles
>> types 5 and 6. This table shows the mappings needed:
>>
>> "+HH", -> "+HHmmss" or "+HH:mm:ss"
>> "+HHmm", -> "+HHmmss",
>> "+HH:mm", -> "+HH:mm:ss",
>> "+HHMM", -> "+HHmmss",
>> "+HH:MM", -> "+HH:mm:ss",
>> "+HHMMss", -> "+HHmmss",
>> "+HH:MM:ss", -> "+HH:mm:ss",
>> "+HHMMSS", -> "+HHmmss",
>> "+HH:MM:SS", -> "+HH:mm:ss",
>> "+HHmmss",
>> "+HH:mm:ss",
>>
>> Note that the "+HH" pattern is a special case, as we don't know
>> whether to use the colon or non-colon pattern. Whether to require
>> colon or not is based on whether the next character after the HH is a
>> colon or not.
>>
>> Proposed appendOffsetId() Javadoc:
>>
>> * Appends the zone offset, such as '+01:00', to the formatter.
>> * <p>
>> * This appends an instruction to format/parse the offset ID to the
>> builder.
>> * This is equivalent to calling {@code appendOffset("+HH:MM:ss", "Z")}.
>> * See {@link #appendOffset(String, String)} for details on formatting
>> and parsing.
>>
>> Proposed appendOffset(String, String) Javadoc:
>>
>> * During parsing, the offset...
>>
>> changed to:
>>
>> * When parsing in strict mode, the input must contain the mandatory
>> and optional elements are defined by the specified pattern.
>> * If the offset cannot be parsed then an exception is thrown unless
>> the section of the formatter is optional.
>> * <p>
>> * When parsing in lenient mode, only the hours are mandatory - minutes
>> and seconds are optional.
>> * The colons are required if the specified pattern contains a colon.
>> * If the specified pattern is "+HH", the presence of colons is
>> determined by whether the character after the hour digits is a colon
>> or not.
>> * If the offset cannot be parsed then an exception is thrown unless
>> the section of the formatter is optional.
>>
>> thanks and sorry for delay
>> Stephen
>>
>>
>>
>> On 11 February 2016 at 20:22, nadeesh tv <nadeesh...@oracle.com> wrote:
>>>
>>> Hi all,
>>>
>>> Please review a fix for
>>>
>>> Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051
>>>
>>> webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/
>>>
>>> --
>>> Thanks and Regards,
>>> Nadeesh TV
>>>
>
> --
> Thanks and Regards,
> Nadeesh TV
>
