On Mon, 2 Nov 2020 17:05:40 GMT, Stephen Colebourne <scolebou...@openjdk.org>
wrote:
>> 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.
CLDR allows rules that involve MINUTE_OF_HOUR. The validation I meant was
within the `updateCheckDayPeriodConflict`:
long hod = fieldValues.get(HOUR_OF_DAY);
long moh = fieldValues.containsKey(MINUTE_OF_HOUR) ?
fieldValues.get(MINUTE_OF_HOUR) : 0;
long mod = hod * 60 + moh;
if (!dayPeriod.includes(mod)) {
throw new DateTimeException("Conflict found: Resolved time
%02d:%02d".formatted(hod, moh) +
" conflicts with " + dayPeriod);
}
which checks both hod/moh. moh is assumed zero if `MINUTE_OF_HOUR` does not
exist. Are you suggesting `HOUR_OF_DAY` should also be assumed zero if it does
not exist?
>> 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
I implemented what you suggested here in the latest PR, but that would be a
behavioral change which requires a CSR, as "AM" would be resolved to 06:00
which was not before. Do you think it would be acceptable? If so, I will reopen
the CSR and describe the change. (In fact some TCK failed with this impl)
-------------
PR: https://git.openjdk.java.net/jdk/pull/938
