On Tue, 8 Feb 2022 19:08:45 GMT, Naoto Sato <na...@openjdk.org> wrote:
>> 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: > > Fixed LocalizedPrinterParser.toString() to reflect requestedTemplate src/java.base/share/classes/java/time/format/DateTimeFormatter.java line 742: > 740: * All pattern symbols are optional, and each pattern symbol > represents the field it is in, > 741: * e.g., 'M' represents the Month field. The number of the pattern > symbol letters follows the > 742: * same presentation, such as "number" or "text" as in the <a > href="#patterns">Patterns for I would reword as: * All pattern symbols are optional, and each pattern symbol represents a field, * for example, 'M' represents the Month field. The number of the pattern symbol letters follows the src/java.base/share/classes/java/time/format/DateTimeFormatter.java line 753: > 751: * <p> > 752: * The locale is determined from the formatter. The formatter > returned directly by > 753: * this method will use the {@link Locale#getDefault() default > FORMAT locale}. Editorial suggestion: * this method uses the {https://github.com/link Locale#getDefault() default FORMAT locale}. src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 231: > 229: > 230: /** > 231: * Gets the formatting pattern for the requested template for a > locale and chronology. "Returns" is more conventional (than "Gets; though it is consistent within the file) src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 246: > 244: * @param locale the locale, non-null > 245: * @return the locale and Chronology specific formatting pattern > 246: * @throws IllegalArgumentException if {@code requestedTemplate} is > invalid The meaning "Invalid" should be clearly defined; repeating or refering to the template regex. src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 1485: > 1483: * <li>the {@code requestedTemplate} specified to this method > 1484: * <li>the {@code Locale} of the {@code DateTimeFormatter} > 1485: * <li>the {@code Chronology}, selecting the best available Perhaps "best available" should be well defined, or just drop "best". or ... "of the {@code DateTimeFormatter} unless overridden" src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 1513: > 1511: * } > 1512: * All pattern symbols are optional, and each pattern symbol > represents the field it is in, > 1513: * e.g., 'M' represents the Month field. The number of the pattern > symbol letters follows the Ditto above: * All pattern symbols are optional, and each pattern symbol represents the field, * for example, 'M' represents the Month field. The number of the pattern symbol letters follows the src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 5148: > 5146: var useRequestedTemplate = requestedTemplate != null; > 5147: String key = chrono.getId() + '|' + locale.toString() + '|' > + > 5148: (useRequestedTemplate ? requestedTemplate : > Objects.toString(dateStyle) + timeStyle); The boolean `useRequestedTemplate` isn't necessary, replace with `requestedTemplate != null`. src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java line 64: > 62: > 63: /** > 64: * Gets the formatting pattern for the requested template, > calendarType, and locale. "Returns" is more conventional. src/java.base/share/classes/sun/util/locale/provider/JavaTimeDateTimePatternImpl.java line 69: > 67: public String getJavaTimeDateTimePattern(int timeStyle, int > dateStyle, String calType, Locale locale) { > 68: LocaleResources lr = > LocaleProviderAdapter.getResourceBundleBased().getLocaleResources(locale); > 69: return lr.getJavaTimeDateTimePattern( Fold the parameters onto a single line. src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java line 690: > 688: private String resolveInputSkeleton(String type) { > 689: var regionToSkeletonMap = inputSkeletons.get(type); > 690: return regionToSkeletonMap.getOrDefault(locale.getLanguage() + > "-" + locale.getCountry(), This structure computes all the defaults even if the value isn't needed (because the value has to be passed to the `getOrDefault` method. Perhaps performance isn't an issue. ------------- PR: https://git.openjdk.java.net/jdk/pull/7340