On Thu, 5 Nov 2020 23:24:19 GMT, Stephen Colebourne <scolebou...@openjdk.org> wrote:
>> 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) > > I find the whole "half way between the start and end" behaviour of day > periods odd anyway, but if it is to be supported then it should be consistent > as you've implemented. > > Another option I should have thought of earlier would be to simply not > support the "half way between the start and end" behaviour of LDML in either > dayPeriod or AM/PM. But since LDML defines it, you've implemented it, and it > isn't overly harmful I think its OK to leave it in. > > Would it be possible for STRICT mode to not have the "half way between the > start and end" behaviour? I've had a look tonight, but found two more problems! The comments at the start of `resolveTimeLenient()` indicate that setting the midpoint in `resolveTimeFields()` is a bad idea - it needs to be done inside `resolveTimeLenient()`. (This ensures user-defined fields can resolve to ChronoFields IIRC). Thus, the day period resolving would have to be split between the two methods. How important is the midpoint logic? Could it just be dropped? Secondly, we need to ensure that "24:00 midnight" (using HOUR_OF_DAY only) correctly parses to the end of day midnight, not the start of day or an exception. This is related to the problem above. I _think_ (but haven't confirmed everything yet) that the only thing that should be in `resolveTimeFields()` is the resolution of HOUR_OF_AMPM + dayPeriod to HOUR_OF_DAY (with `dayPeriod` then being set to `null`). Everything else should go in `resolveTimeLenient()` - the midpoint logic in the first if block (where time fields are defaulted), and the validation logic at the end of the method. ------------- PR: https://git.openjdk.java.net/jdk/pull/938
