Integrated: 8176706: Additional Date-Time Formats
On Thu, 3 Feb 2022 23:29:54 GMT, Naoto Sato 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 This pull request has now been integrated. Changeset: 9b74c3f2 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/9b74c3f2e74a4efdec1c1488e96ab5939a408df0 Stats: 821 lines in 12 files changed: 725 ins; 12 del; 84 mod 8176706: Additional Date-Time Formats Reviewed-by: joehw, rriggs - PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v4]
On Fri, 11 Feb 2022 22:26:03 GMT, Roger Riggs wrote:
>> Naoto Sato has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Addressing review comments
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java
> line 5103:
>
>> 5101: * @param timeStyle the time style to use, may be null
>> 5102: */
>> 5103: LocalizedPrinterParser(FormatStyle dateStyle, FormatStyle
>> timeStyle) {
>
> Can the constructors be `private`.
> The combination of package protected and the style of caller doing the
> validation makes me a bit nervous.
> There should not be any callers outside of DateTimeFormatterBuilder.
Changed to `private` so that it can only be instantiated within
`DateTimeFormatterBuilder`. Same modifications are applied to other
`DateTimePrinterParser` implementations.
> src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java
> line 79:
>
>> 77: public String getJavaTimeDateTimePattern(String requestedTemplate,
>> String calType, Locale locale) {
>> 78: // default implementation throws exception
>> 79: throw new DateTimeException("Formatter is not available for the
>> requested template: " + requestedTemplate);
>
> "Formatting **pattern** is not available"...
Modified, as well as other minor corrections in the javadoc.
-
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v5]
> 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: Addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7340/files - new: https://git.openjdk.java.net/jdk/pull/7340/files/af3c0d62..399257d4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=03-04 Stats: 31 lines in 2 files changed: 3 ins; 0 del; 28 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
Re: RFR: 8176706: Additional Date-Time Formats [v4]
On Fri, 11 Feb 2022 00:03:50 GMT, Naoto Sato 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:
>
> Addressing review comments
Looks good.
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
5103:
> 5101: * @param timeStyle the time style to use, may be null
> 5102: */
> 5103: LocalizedPrinterParser(FormatStyle dateStyle, FormatStyle
> timeStyle) {
Can the constructors be `private`.
The combination of package protected and the style of caller doing the
validation makes me a bit nervous.
There should not be any callers outside of DateTimeFormatterBuilder.
src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java
line 66:
> 64: * Returns the formatting pattern for the requested template,
> calendarType, and locale.
> 65: * Concrete implementation of this method will retrieve
> 66: * a java.time specific pattern from selected Locale Provider.
editorial: "from **the** selected Locale"...
src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java
line 79:
> 77: public String getJavaTimeDateTimePattern(String requestedTemplate,
> String calType, Locale locale) {
> 78: // default implementation throws exception
> 79: throw new DateTimeException("Formatter is not available for the
> requested template: " + requestedTemplate);
"Formatting **pattern** is not available"...
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v3]
On Thu, 10 Feb 2022 22:20:48 GMT, Roger Riggs wrote:
>> 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/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.
Indeed, yes. I thought this was simple enough, and as you said, not
performance-critical.
-
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v4]
> 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: Addressing review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7340/files - new: https://git.openjdk.java.net/jdk/pull/7340/files/41408ff3..af3c0d62 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=02-03 Stats: 33 lines in 5 files changed: 1 ins; 2 del; 30 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
Re: RFR: 8176706: Additional Date-Time Formats [v3]
On Tue, 8 Feb 2022 19:08:45 GMT, Naoto Sato 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 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: *
> 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: * the {@code requestedTemplate} specified to this method
> 1484: * the {@code Locale} of the {@code DateTimeFormatter}
> 1485: * 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
Re: RFR: 8176706: Additional Date-Time Formats [v3]
On Tue, 8 Feb 2022 19:08:45 GMT, Naoto Sato 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 make/jdk/src/classes/build/tools/cldrconverter/Bundle.java line 113: > 111: // DateFormatItem prefix > 112: final static String DATEFORMATITEM_KEY_PREFIX = "DateFormatItem."; > 113: final static String DATEFORMATITEM_INPUT_REGIONS_PREFIX = > "DateFormatItemInputRegions."; The canonical order of modifiers is "static final". - PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v3]
On Tue, 8 Feb 2022 19:08:45 GMT, Naoto Sato 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 Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats [v3]
> 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 - Changes: - all: https://git.openjdk.java.net/jdk/pull/7340/files - new: https://git.openjdk.java.net/jdk/pull/7340/files/059132f5..41408ff3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=01-02 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 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
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
Re: RFR: 8176706: Additional Date-Time Formats
On Thu, 3 Feb 2022 23:29:54 GMT, Naoto Sato 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
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.
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
-
PR: https://git.openjdk.java.net/jdk/pull/7340
Re: RFR: 8176706: Additional Date-Time Formats
On Thu, 3 Feb 2022 23:29:54 GMT, Naoto Sato 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
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
5115:
> 5113:
> 5114: private LocalizedPrinterParser(FormatStyle dateStyle,
> FormatStyle timeStyle, String requestedTemplate) {
> 5115: this.dateStyle = dateStyle;
Drop this constructor; it opens up the possibility of ambiguity between the
modes.
Keep only the two constructors with disjoint checking and initialization.
src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line
5151:
> 5149: */
> 5150: private DateTimeFormatter formatter(Locale locale, Chronology
> chrono) {
> 5151: var dtStyle = dateStyle !=null || timeStyle != null;
Switch to using requestedTemplate == null or != null as the discriminator.
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).
src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java line
606:
> 604: var matcher = VALID_SKELETON_PATTERN.matcher(skeleton);
> 605: if (!matcher.matches()) {
> 606: throw new IllegalArgumentException("Requested template is
> invalid: " + requestedTemplate);
The substitutions are not going to be visible in the exception message.
That might make it harder to identify and fix the cause of the exception.
Perhaps the message can make it clear that the matching was done after
substitutions and show the 'skeleton'.
-
PR: https://git.openjdk.java.net/jdk/pull/7340
RFR: 8176706: Additional Date-Time Formats
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 - Commit messages: - Removed trailing space - Merge branch 'master' into skeleton - Tidying up - Some more tests - Brought back the part that was mistakenly removed - Merge branch 'master' into skeleton - Reverted IllegalArgumentException back - Further cleanups - Removed exceptions from ofLocalizedPattern() method, as they are lazily thrown on format() - Cleanup - ... and 19 more: https://git.openjdk.java.net/jdk/compare/cda9c301...f9311dce Changes: https://git.openjdk.java.net/jdk/pull/7340/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7340=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8176706 Stats: 777 lines in 12 files changed: 733 ins; 9 del; 35 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
Re: Additional Date-Time Formats
Updated the CSR based on your inputs. On 1/27/22 2:34 PM, Stephen Colebourne wrote: On Thu, 27 Jan 2022 at 19:23, Naoto Sato wrote: Now come to think of it, I came up with the draft based on `ofPattern()` methods. One of them is a overload method that takes a Locale argument. Why is it so? There is a case for the Locale on the static method (as a convenience, and to remind callers that the locale matters). But there is no case for it on the builder. Eliminated the overload with locale. Instead, added a static method DateTimeFormatterBuilder.getLocalizedDateTimePattern() that take "requestedTemplate". Also, renamed `appendLocalizedPattern` to `appendLocalized` to make it an overload method to the existing one. The spec Javadoc doesn't explain what repeating the pattern letter actually does/means. Is "M" the same as ""? That depends on the locale and the availability of the formats. For example, 'M' translates into these patterns in each locale with Gregorian calendar: https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f As things stand, the Javadoc isn't a standalone spec. I don't know how much that matters, but I think there should be some indication in Javadoc as to why the letter should be repeated. Added a description that explains the number of pattern letters. They follow the same presentation rule as in the pattern chart. Naoto
Re: Additional Date-Time Formats
On Thu, 27 Jan 2022 at 19:23, Naoto Sato wrote: > Now come to think of it, I came up with the draft based on `ofPattern()` > methods. One of them is a overload method that takes a Locale argument. > Why is it so? There is a case for the Locale on the static method (as a convenience, and to remind callers that the locale matters). But there is no case for it on the builder. > > The spec Javadoc doesn't explain what repeating the pattern letter > > actually does/means. Is "M" the same as ""? > > That depends on the locale and the availability of the formats. For > example, 'M' translates into these patterns in each locale with > Gregorian calendar: > > https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f As things stand, the Javadoc isn't a standalone spec. I don't know how much that matters, but I think there should be some indication in Javadoc as to why the letter should be repeated. Stephen
Re: Additional Date-Time Formats
Hi Stephen,
On 1/27/22 1:00 AM, Stephen Colebourne wrote:
Hi,
This would be a useful addition. Some comments:
There is no need for the method overload that takes Locale. The other
similar methods all operate using the locale of the formatter, and
have this Javadoc:
* The locale is determined from the formatter. The formatter
returned directly by
* this method will use the {@link Locale#getDefault() default
FORMAT locale}.
* The locale can be controlled using {@link
DateTimeFormatter#withLocale(Locale) withLocale(Locale)}
* on the result of this method.
And `appendLocalizedPattern` should not take a Locale either. Again ,
it would use the locale of the formatter instance, calculating the
actual pattern on-demand when the formatter is run.
That makes sense. Will revise the spec.
Now come to think of it, I came up with the draft based on `ofPattern()`
methods. One of them is a overload method that takes a Locale argument.
Why is it so?
The spec Javadoc doesn't explain what repeating the pattern letter
actually does/means. Is "M" the same as ""?
That depends on the locale and the availability of the formats. For
example, 'M' translates into these patterns in each locale with
Gregorian calendar:
https://unicode-org.github.io/cldr-staging/charts/40/by_type/date_&_time.gregorian.html#959cbb42bb2962f
Naoto
Re: Additional Date-Time Formats
Hi,
This would be a useful addition. Some comments:
There is no need for the method overload that takes Locale. The other
similar methods all operate using the locale of the formatter, and
have this Javadoc:
* The locale is determined from the formatter. The formatter
returned directly by
* this method will use the {@link Locale#getDefault() default
FORMAT locale}.
* The locale can be controlled using {@link
DateTimeFormatter#withLocale(Locale) withLocale(Locale)}
* on the result of this method.
And `appendLocalizedPattern` should not take a Locale either. Again ,
it would use the locale of the formatter instance, calculating the
actual pattern on-demand when the formatter is run.
The spec Javadoc doesn't explain what repeating the pattern letter
actually does/means. Is "M" the same as ""?
thanks
Stephen
On Thu, 20 Jan 2022 at 21:47, Naoto Sato wrote:
>
> Hello,
>
> I am proposing a couple of new factory methods in
> java.time.format.DateTimeFormatter that produce flexible localized
> date/time formats, other than the existing pre-defined
> (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year
> and month only string, such as the title for the calendar, currently
> they would have to use DateTimeFormatter.ofPattern() with explicit
> pattern argument, such as "MMM y". This is problematic because it is
> correct in locales such as US, but not correct in other locales.
>
> So, I propose methods that are parallel to ofPattern(), which take
> pattern template. This is based on the CLDR's skeleton:
> https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems
>
> Detailed design can be found in the draft CSR:
> https://bugs.openjdk.java.net/browse/JDK-8243445
>
> Comments are welcome.
>
> Naoto
>
Re: Additional Date-Time Formats
Hi Naoto,
Looks good to me, and thanks for the explanation. I agree, AIOOBE would
be a separate bug with ofPattern.
Thanks,
Joe
On 1/25/22 2:30 PM, Naoto Sato wrote:
Hi Joe,
On 1/24/22 5:50 PM, Joe Wang wrote:
The 2nd and 3rd statements defined the requestedTemplate,
does it imply the characters listed in the snippet are the only ones
that are valid, in other words, can other characters under the
Patterns section be used? It may be helpful to elaborate on the
snippet a bit more.
Those symbols represent each field, so other symbols are considered
illegal as a template symbol. Added some explanation there.
Also, the range implies a valid range for a particular
symbol, if that's the case, y and w feel like they are unbound. If I
do that with ofPettern, I get ArrayIndexOutOfBoundsException.
The spec of 'year' and 'number' presentations do not have any upper
limit number of letters, thus I added the '*' quantifier. Not exactly
sure why AIOOBE is thrown with ofPattern(), could be a separate bug?
It should be zero-padded or sign-padded.
For the sample code, it might be helpful to put them in a code
snippet and with the actual java code. If "yMMM" formats to 'Jun
2020', that might require some explanation too since that would be
the same as ofPattern("MMM y") for the default(US) locale, or was it
a typo?. (I'm not familiar with the use of DTF, just printed out
date.format(DTF.ofPattern("yMMM" and "MMM y") :-))
Well, it is not a typo and `ofLocalizedPattern("yMMM", Locale.US)` and
`ofPattern("MMM y", Locale.US)` both generating the same result is
exactly what this API is aiming at. Users don't need to pay attention
to locale specific format pattern with this API.
HTH,
Naoto
Re: Additional Date-Time Formats
Hi Joe,
On 1/24/22 5:50 PM, Joe Wang wrote:
The 2nd and 3rd statements defined the requestedTemplate, does
it imply the characters listed in the snippet are the only ones that are
valid, in other words, can other characters under the Patterns section
be used? It may be helpful to elaborate on the snippet a bit more.
Those symbols represent each field, so other symbols are considered
illegal as a template symbol. Added some explanation there.
Also, the range implies a valid range for a particular symbol,
if that's the case, y and w feel like they are unbound. If I do that
with ofPettern, I get ArrayIndexOutOfBoundsException.
The spec of 'year' and 'number' presentations do not have any upper
limit number of letters, thus I added the '*' quantifier. Not exactly
sure why AIOOBE is thrown with ofPattern(), could be a separate bug? It
should be zero-padded or sign-padded.
For the sample code, it might be helpful to put them in a code snippet
and with the actual java code. If "yMMM" formats to 'Jun 2020', that
might require some explanation too since that would be the same as
ofPattern("MMM y") for the default(US) locale, or was it a typo?. (I'm
not familiar with the use of DTF, just printed out
date.format(DTF.ofPattern("yMMM" and "MMM y") :-))
Well, it is not a typo and `ofLocalizedPattern("yMMM", Locale.US)` and
`ofPattern("MMM y", Locale.US)` both generating the same result is
exactly what this API is aiming at. Users don't need to pay attention to
locale specific format pattern with this API.
HTH,
Naoto
Re: Additional Date-Time Formats
Hi Naoto,
Nice use of regular expression! Saves a lot of description if I would
just follow the regex pattern.
Question:
The 2nd and 3rd statements defined the requestedTemplate, does
it imply the characters listed in the snippet are the only ones that are
valid, in other words, can other characters under the Patterns section
be used? It may be helpful to elaborate on the snippet a bit more.
Also, the range implies a valid range for a particular symbol,
if that's the case, y and w feel like they are unbound. If I do that
with ofPettern, I get ArrayIndexOutOfBoundsException.
For the sample code, it might be helpful to put them in a code snippet
and with the actual java code. If "yMMM" formats to 'Jun 2020', that
might require some explanation too since that would be the same as
ofPattern("MMM y") for the default(US) locale, or was it a typo?. (I'm
not familiar with the use of DTF, just printed out
date.format(DTF.ofPattern("yMMM" and "MMM y") :-))
Joe
On 1/24/22 4:13 PM, Naoto Sato wrote:
Updated the CSR (https://bugs.openjdk.java.net/browse/JDK-8243445), by
adding a regular expression for the requested template. This way, it
is less depending on the LDML specification.
Naoto
On 1/21/22 2:39 PM, Naoto Sato wrote:
Thanks, Joe.
Good point. I will elaborate the pattern template part more, less
depending on the LDML spec. Would have been better if we could
introduce our own, such as ofLocalizedPattern(Set
template), but not exactly suffices the need.
Naoto
On 1/20/22 9:52 PM, Joe Wang wrote:
Hi Naoto,
The javadoc points to LDML, it seems to me though it might be useful
to add more information similar to that for the ofPattern methods,
what's under the "Patterns for Formatting and Parsing" section, so
that for at least the common use cases we could rely on the javadoc
without having to consult the LDML specification. Some comparison
with the ofPattern methods might also be helpful.
Just my 2 cents.
Thanks,
Joe
On 1/20/22 1:46 PM, Naoto Sato wrote:
Hello,
I am proposing a couple of new factory methods in
java.time.format.DateTimeFormatter that produce flexible localized
date/time formats, other than the existing pre-defined
(FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a
year and month only string, such as the title for the calendar,
currently they would have to use DateTimeFormatter.ofPattern() with
explicit pattern argument, such as "MMM y". This is problematic
because it is correct in locales such as US, but not correct in
other locales.
So, I propose methods that are parallel to ofPattern(), which take
pattern template. This is based on the CLDR's skeleton:
https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems
Detailed design can be found in the draft CSR:
https://bugs.openjdk.java.net/browse/JDK-8243445
Comments are welcome.
Naoto
Re: Additional Date-Time Formats
Updated the CSR (https://bugs.openjdk.java.net/browse/JDK-8243445), by adding a regular expression for the requested template. This way, it is less depending on the LDML specification. Naoto On 1/21/22 2:39 PM, Naoto Sato wrote: Thanks, Joe. Good point. I will elaborate the pattern template part more, less depending on the LDML spec. Would have been better if we could introduce our own, such as ofLocalizedPattern(Set template), but not exactly suffices the need. Naoto On 1/20/22 9:52 PM, Joe Wang wrote: Hi Naoto, The javadoc points to LDML, it seems to me though it might be useful to add more information similar to that for the ofPattern methods, what's under the "Patterns for Formatting and Parsing" section, so that for at least the common use cases we could rely on the javadoc without having to consult the LDML specification. Some comparison with the ofPattern methods might also be helpful. Just my 2 cents. Thanks, Joe On 1/20/22 1:46 PM, Naoto Sato wrote: Hello, I am proposing a couple of new factory methods in java.time.format.DateTimeFormatter that produce flexible localized date/time formats, other than the existing pre-defined (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year and month only string, such as the title for the calendar, currently they would have to use DateTimeFormatter.ofPattern() with explicit pattern argument, such as "MMM y". This is problematic because it is correct in locales such as US, but not correct in other locales. So, I propose methods that are parallel to ofPattern(), which take pattern template. This is based on the CLDR's skeleton: https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems Detailed design can be found in the draft CSR: https://bugs.openjdk.java.net/browse/JDK-8243445 Comments are welcome. Naoto
Re: Additional Date-Time Formats
Thanks, Joe. Good point. I will elaborate the pattern template part more, less depending on the LDML spec. Would have been better if we could introduce our own, such as ofLocalizedPattern(Set template), but not exactly suffices the need. Naoto On 1/20/22 9:52 PM, Joe Wang wrote: Hi Naoto, The javadoc points to LDML, it seems to me though it might be useful to add more information similar to that for the ofPattern methods, what's under the "Patterns for Formatting and Parsing" section, so that for at least the common use cases we could rely on the javadoc without having to consult the LDML specification. Some comparison with the ofPattern methods might also be helpful. Just my 2 cents. Thanks, Joe On 1/20/22 1:46 PM, Naoto Sato wrote: Hello, I am proposing a couple of new factory methods in java.time.format.DateTimeFormatter that produce flexible localized date/time formats, other than the existing pre-defined (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year and month only string, such as the title for the calendar, currently they would have to use DateTimeFormatter.ofPattern() with explicit pattern argument, such as "MMM y". This is problematic because it is correct in locales such as US, but not correct in other locales. So, I propose methods that are parallel to ofPattern(), which take pattern template. This is based on the CLDR's skeleton: https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems Detailed design can be found in the draft CSR: https://bugs.openjdk.java.net/browse/JDK-8243445 Comments are welcome. Naoto
Re: Additional Date-Time Formats
Hi Naoto, The javadoc points to LDML, it seems to me though it might be useful to add more information similar to that for the ofPattern methods, what's under the "Patterns for Formatting and Parsing" section, so that for at least the common use cases we could rely on the javadoc without having to consult the LDML specification. Some comparison with the ofPattern methods might also be helpful. Just my 2 cents. Thanks, Joe On 1/20/22 1:46 PM, Naoto Sato wrote: Hello, I am proposing a couple of new factory methods in java.time.format.DateTimeFormatter that produce flexible localized date/time formats, other than the existing pre-defined (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year and month only string, such as the title for the calendar, currently they would have to use DateTimeFormatter.ofPattern() with explicit pattern argument, such as "MMM y". This is problematic because it is correct in locales such as US, but not correct in other locales. So, I propose methods that are parallel to ofPattern(), which take pattern template. This is based on the CLDR's skeleton: https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems Detailed design can be found in the draft CSR: https://bugs.openjdk.java.net/browse/JDK-8243445 Comments are welcome. Naoto
Additional Date-Time Formats
Hello, I am proposing a couple of new factory methods in java.time.format.DateTimeFormatter that produce flexible localized date/time formats, other than the existing pre-defined (FULL/LONG/MEDIUM/SHORT) styles. For example, if the user wants a year and month only string, such as the title for the calendar, currently they would have to use DateTimeFormatter.ofPattern() with explicit pattern argument, such as "MMM y". This is problematic because it is correct in locales such as US, but not correct in other locales. So, I propose methods that are parallel to ofPattern(), which take pattern template. This is based on the CLDR's skeleton: https://www.unicode.org/reports/tr35/tr35-dates.html#availableFormats_appendItems Detailed design can be found in the draft CSR: https://bugs.openjdk.java.net/browse/JDK-8243445 Comments are welcome. Naoto
