On Thu, 12 Nov 2020 20:03:14 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> Hi, >> >> Please review the changes for the subject issue. This is to enhance the >> java.time package to support day periods, such as "in the morning", defined >> in CLDR. It will add a new pattern character 'B' and its supporting builder >> method. The motivation and its spec are in this CSR: >> >> https://bugs.openjdk.java.net/browse/JDK-8254629 >> >> Naoto > > Naoto Sato has updated the pull request incrementally with one additional > commit since the last revision: > > Addressed the following comments: > - https://github.com/openjdk/jdk/pull/938#discussion_r522185469 > - https://github.com/openjdk/jdk/pull/938#discussion_r522187931 > - https://github.com/openjdk/jdk/pull/938#discussion_r522203757 > - https://github.com/openjdk/jdk/pull/938#discussion_r522211444 > - https://github.com/openjdk/jdk/pull/938#discussion_r522244221 > - https://github.com/openjdk/jdk/pull/938#discussion_r522262379 > - https://github.com/openjdk/jdk/pull/938#discussion_r522266836 Approved with one overflow to fix. The spec could do with some rewording too. It might be better to explicitly mention the "resolving phase" with the three parts: > The day period is combined with other fields to make a `LocalTime` in the > resolving phase. If the `HOUR_OF_AMPM` field is present, it is combined with > the day period to make `HOUR_OF_DAY` taking into account any `MINUTE_OF_HOUR` > value. If `HOUR_OF_DAY` is present, it is validated against the day period > taking into account any `MINUTE_OF_HOUR` value. If a day period is present > without `HOUR_OF_DAY`, `MINUTE_OF_HOUR`, `SECOND_OF_MINUTE` and > `NANO_OF_SECOND` then the midpoint of the day period is set as the time. Note that the above is incomplete, and it doesn't describe STRICT/LENIENT, so the actual words will be more complex, src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5063: > 5061: } > 5062: Long moh = context.getValue(MINUTE_OF_HOUR); > 5063: long value = (hod * 60 + (moh != null ? moh : 0)) % 1_440; `long value = Math.floorMod(hod, 24) * 60 + (moh != null ? Math.floorMod(moh, 60) : 0);` and remove the next three lines ------------- Marked as reviewed by scolebourne (Author). PR: https://git.openjdk.java.net/jdk/pull/938
