Conversation
There was a problem hiding this comment.
Pull Request Overview
Adds Java 20+ support for time and datetime formatting in DefaultDateHelper and documents CLDR42 changes.
- Updates existing tests to require Java 20+ and adds new style cases for
getTimeFormat. - Introduces a dedicated Java 20+ test for
getDateTimeFormatwith multiple style combinations. - Adds a warning to the documentation about CLDR42’s switch to narrow non-breaking spaces and separator changes.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| grails-gsp/plugin/src/test/groovy/org/grails/plugins/web/DefaultDateHelperSpec.groovy | Refactored test requirements to isJava20Compatible, renamed tests, and added new style scenarios for time and datetime formatting. |
| grails-doc/src/en/ref/Tags - GSP/formatDate.adoc | Added a warning about Java 20+ CLDR42 behavior changes in formatted date/time output. |
Comments suppressed due to low confidence (3)
grails-gsp/plugin/src/test/groovy/org/grails/plugins/web/DefaultDateHelperSpec.groovy:101
- [nitpick] The test descriptions for
getTimeFormatare inconsistent: one is generic and the other includes “Full”. Consider unifying both to a standard pattern, e.g.,Java 20+ - getTimeFormat for style #style returns #expected.
void "Java 20+ - getTimeFormat for style #style returns #expected"(String style, String expected) {
grails-gsp/plugin/src/test/groovy/org/grails/plugins/web/DefaultDateHelperSpec.groovy:139
- [nitpick] This test name duplicates the first but adds “Full”; consider merging or renaming to avoid confusion and improve clarity.
void "Java 20+ - Full getTimeFormat for style #style returns #expected"(String style, String expected) {
grails-doc/src/en/ref/Tags - GSP/formatDate.adoc:60
- [nitpick] The abbreviation “IE.” is misleading; replace it with “i.e.,” or “e.g.,” as appropriate for clarification.
WARNING: In Java 20+, https://cldr.unicode.org/downloads/cldr-42[Unicode CLDR42] was implemented which changed the space character preceding the period (AM or PM) in formatted date/time text from a standard space (" ") to a narrow non-breaking space (NNBSP: "\u202F"). Additionally, when using the LONG or FULL timeStyle with dateStyle, the date and time separator has changed from ' at ' to ', '. IE. January 5, 1941, 8:00:00 AM UTC vs. January 5, 1941 at 8:00:00 AM UTC
jdaugherty
left a comment
There was a problem hiding this comment.
This change needs called out in the v6 to 7 upgrade notes too
|
I have updated to tests to use \u202F instead of NNBSP. IntelliJ makes is easy to see, but it is invisible in the diff. Also added details to Breaking changes section of v6 to 7 upgrade notes. |
In Java 20+, Unicode CLDR42 was implemented which changed the space character preceding the period (AM or PM) in formatted date/time text from a standard space (" ") to a narrow non-breaking space (NNBSP: "\u202F"). Additionally, when using the LONG or FULL timeStyle with dateStyle, the date and time separator has changed from ' at ' to ', '. IE. January 5, 1941, 8:00:00 AM UTC vs. January 5, 1941 at 8:00:00 AM UTC
We already had Java 8 and 17 specific tests. For each of the Java version specific scenarios covered by prior tests, a Java 20+ test was added.