On Fri, 30 Oct 2020 21:30:50 GMT, Naoto Sato <[email protected]> wrote:
>> src/java.base/share/classes/java/time/format/Parsed.java line 472:
>>
>>> 470: }
>>> 471: if (dayPeriod != null) {
>>> 472: if (fieldValues.containsKey(HOUR_OF_DAY)) {
>>
>> Are we certain that the CLDR data does not contain day periods based on
>> minutes as well as hours? This logic does not check against MINUTE_OF_HOUR
>> for example. The logic also conflicts with the spec Javadoc that says
>> MINUTE_OF_HOUR is validated.
>
> MINUTE_OF_HOUR without HOUR_OF_DAY/HOUR_OF_AMPM may not make any sense, so it
> is only validated in updateCheckDayPeriodConflict.
But can you get a CLDR rule that says "at night" before 05:30 and "In the
morning" from 05:30 onwards? If you can then I don't think it is handled,
because HOUR_OF_DAY and MINUTE_OF_HOUR are not used together when checking
against DayPeriod.
>> src/java.base/share/classes/java/time/format/Parsed.java line 500:
>>
>>> 498: }
>>> 499: }
>>> 500: }
>>
>> Looking at the existing logic, the `AMPM_OF_DAY` field is completely ignored
>> if there is no `HOUR_OF_AMPM` field. Thus, there is no validation to check
>> `AMPM_OF_DAY` against `HOUR_OF_DAY`. This seems odd. (AMPM_OF_DAY = 0 and
>> HOUR_OF_DAY=18 does not look like it throws an exception, when it probably
>> should).
>>
>> On solution would be for `AMPM_OF_DAY` to be resolved to a day period at
>> line 427, checking for conflicts with any parsed day period. (a small bug
>> fix behavioural change)
>
> There are cases where a period crosses midnight, e.g., 23:00-04:00 so it
> cannot validate am/pm, so I decided to just override ampm with dayperiod
> without validating.
Pulling on this a little more.
As the PR stands, it seems that if the user passes in text with just a
day-period of "AM" they get a `LocalTime` of 06:00 but if they pass in
`AMPM_OF_DAY` of "AM" the get no `LocalTime` in the result. Is that right? If
so, I don't think this is sustainable.
Thats why I think `AMPM_OF_DAY` will have to be resolved to a dayPeriod of "am"
or "pm". If dayPeriod is more precise than `AMPM_OF_DAY`, then dayPeriod can
silently take precedence
-------------
PR: https://git.openjdk.java.net/jdk/pull/938