On Mon, 9 Nov 2020 01:37:39 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> 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. > > Thanks for the insights. I believe I fixed both of the issues with the new > commit. Added a test case with the latest commit: https://github.com/openjdk/jdk/commit/a960804ff4d3f7df18e51fe59dcdcaf04542e10a ------------- PR: https://git.openjdk.java.net/jdk/pull/938
