Re: RFR: 8176706: Additional Date-Time Formats [v2]
On Mon, 7 Feb 2022 21:22:12 GMT, Roger Riggs wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Modified per suggestions on the PR
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 5162:
>
>> 5160:
>> 5161: formatter = new
>> DateTimeFormatterBuilder().appendPattern(pattern).toFormatter(locale);
>> 5162: DateTimeFormatter old =
>> FORMATTER_CACHE.putIfAbsent(key, formatter);
>
> .computeIfAbsent(key, () -> {}) might be a bit cleaner/clearer here and
> avoid the race a few lines of code below. (a slight improvement in old code).
Good point. Changed to `computeIfAbsent()`.
-
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v2]
On Tue, 8 Feb 2022 00:39:04 GMT, Joe Wang wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Modified per suggestions on the PR
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 254:
>
>> 252: public static String getLocalizedDateTimePattern(String
>> requestedTemplate,
>> 253: Chronology chrono,
>> Locale locale) {
>> 254: Objects.requireNonNull(locale, "requestedTemplate");
>
> Typo, "locale" should have been requestedTemplate.
Good catch!
> src/java.base/share/classes/sun/util/locale/provider/JavaTimeDateTimePatternImpl.java
> line 77:
>
>> 75: LocaleProviderAdapter lpa =
>> LocaleProviderAdapter.getResourceBundleBased();
>> 76: // CLDR's 'u'/'U' are not supported in the JDK. Replace them
>> with 'y' instead
>> 77: final var modifiedSkeleton =
>> requestedTemplate.replaceAll("[uU]", "y");
>
> Seems to me requestedTemplate needs to be validated when it gets passed to
> getLocalizedDateTimePattern, similar as to appendLocalized
This was a left over which should be removed. In fact, CLDR specific pattern
symbols should not be accepted as the requested template. Removed the
substitutions. As to the validation, it will be performed in the following
`LocaleResources.getLocalizedPattern()` method.
-
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v2]
> Following the prior discussion [1], here is the PR for the subject > enhancement. CSR has also been updated according to the suggestion. > > [1] > https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Modified per suggestions on the PR - Changes: - all: https://git.openjdk.java.net/jdk/pull/7340/files - new: https://git.openjdk.java.net/jdk/pull/7340/files/f9311dce..059132f5 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=00-01 Stats: 80 lines in 4 files changed: 23 ins; 37 del; 20 mod Patch: https://git.openjdk.java.net/jdk/pull/7340.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7340/head:pull/7340 PR: https://git.openjdk.java.net/jdk/pull/7340
