On Thu, 5 Nov 2020 17:12:11 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: > > Fixed typo/grammatical error. test/jdk/java/time/tck/java/time/format/TCKDateTimeParseResolver.java line 858: > 856: return new Object[][]{ > 857: {STRICT, 0, LocalTime.of(6, 0), 0}, > 858: {STRICT, 1, LocalTime.of(18, 0), 1}, As mentioned in my other comment, this seems odd in STRICT mode. src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5055: > 5053: @Override > 5054: public boolean format(DateTimePrintContext context, > StringBuilder buf) { > 5055: Long value = context.getValue(MINUTE_OF_DAY); This does not match the spec: " During formatting, the day period is obtained from {@code HOUR_OF_DAY}, and optionally {@code MINUTE_OF_HOUR} if exist" It is possible and legal to create a Temporal that returns `HOUR_OF_DAY` and `MINUTE_OF_HOUR` but not `MINUTE_OF_DAY`. As such, this method must be changed to follow the spec. ----- In addition, it is possible for `HOUR_OF_DAY` and `MINUTE_OF_HOUR` to be outside their normal bounds. The right behaviour would be to combine the two fields within this method, and then use mod to get the value into the range 0 to 1440 before calling `dayPeriod.include`. (While the fall back behaviour below does cover this, it would be better to do what I propose here.) An example of this is a `TransportTime` class where the day runs from 03:00 to 27:00 each day (because trains run after midnight for no extra cost to the passenger, and it is more convenient for the operator to treat the date that way). A `TransportTime` of 26:30 should still resolve to "night1" rather than fall back to "am". ------------- PR: https://git.openjdk.java.net/jdk/pull/938
