You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/05/12 11:04:06 UTC
[GitHub] [commons-lang] stevebosman-oc opened a new pull request, #892: LANG-1680 Add support for standalone month formats
stevebosman-oc opened a new pull request, #892:
URL: https://github.com/apache/commons-lang/pull/892
Contributed on behalf of the @opencastsoftware open source team
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r873211413
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -1694,5 +1694,43 @@ public void testWeekIterator() {
now.add(Calendar.DATE, 1);
}
}
+
+ @Test
+ public void testGetStandaloneLongMonthNames() {
+ Locale testLocale = Locale.GERMAN;
+ String[] standaloneShortMonths = DateUtils.getStandaloneLongMonths(testLocale);
Review Comment:
Copy pasta typo: standaloneShortMonths -> standaloneLongMonths?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r873211504
##########
src/test/java/org/apache/commons/lang3/time/DateUtilsTest.java:
##########
@@ -1694,5 +1694,43 @@ public void testWeekIterator() {
now.add(Calendar.DATE, 1);
}
}
+
+ @Test
+ public void testGetStandaloneLongMonthNames() {
+ Locale testLocale = Locale.GERMAN;
Review Comment:
We don't need a local variable for the locale IMO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124961000
Is it safe to that only locales with 12 months are supported? In theory some locales can have thirteen months, although I am not sure there are any supported by Locales in the standard Java library.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124941791
Why not use code like `java.time.Month.APRIL.getDisplayName(TextStyle.FULL, Locale.GERMAN)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory merged pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory merged PR #892:
URL: https://github.com/apache/commons-lang/pull/892
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] arturobernalg commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r873082991
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1831,4 +1832,40 @@ public void remove() {
}
}
+ /**
+ * Gets full standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Long names of months
+ */
+ static String[] getStandaloneLongMonths(final Locale locale) {
+ return getMonthNames(locale, Calendar.LONG_STANDALONE);
+ }
+
+ /**
+ * Gets short standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Short names of months
+ */
+ static String[] getStandaloneShortMonths(final Locale locale) {
+ return getMonthNames(locale, Calendar.SHORT_STANDALONE);
+ }
+
+ /**
+ * Gets month names in the requested style.
+ * @param locale Locale
+ * @param style Must be a valid {@link Calendar#getDisplayNames(int, int, Locale)} month style.
+ * @return Styled names of months
+ */
+ private static String[] getMonthNames(final Locale locale, final int style) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ Map<String, Integer> displayNames = calendar.getDisplayNames(Calendar.MONTH, style, locale);
Review Comment:
add final
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1831,4 +1832,40 @@ public void remove() {
}
}
+ /**
+ * Gets full standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Long names of months
+ */
+ static String[] getStandaloneLongMonths(final Locale locale) {
+ return getMonthNames(locale, Calendar.LONG_STANDALONE);
+ }
+
+ /**
+ * Gets short standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Short names of months
+ */
+ static String[] getStandaloneShortMonths(final Locale locale) {
+ return getMonthNames(locale, Calendar.SHORT_STANDALONE);
+ }
+
+ /**
+ * Gets month names in the requested style.
+ * @param locale Locale
+ * @param style Must be a valid {@link Calendar#getDisplayNames(int, int, Locale)} month style.
+ * @return Styled names of months
+ */
+ private static String[] getMonthNames(final Locale locale, final int style) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ Map<String, Integer> displayNames = calendar.getDisplayNames(Calendar.MONTH, style, locale);
+ final String[] monthNames = new String[displayNames.size()];
+ for (Map.Entry<String, Integer> entry: displayNames.entrySet()) {
Review Comment:
`final Map.Entry<String, Integer> entry`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124909433
After investigating I have determined the Java 8, 11, and 17 all return different standalone short months for September in German which makes my tests unstable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1125763691
Odd, I seem to have encountered a version of https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8146356 but that claims to have been fixed in an earlier version of the 1.8 JDK than the one I'm running
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r873077554
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1831,4 +1831,40 @@ public void remove() {
}
}
+ /**
+ * Obtain full standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Long names of months
+ */
+ static String[] getStandaloneLongMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.LONG_STANDALONE, locale);
+ }
+ return monthNames;
+ }
+
+ /**
+ * Obtain short standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Short names of months
+ */
+ static String[] getStandaloneShortMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.SHORT_STANDALONE, locale);
+ }
+ return monthNames;
+ }
Review Comment:
sorry, I really should have noticed that before submitting. I'll refactor
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r872979099
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1831,4 +1831,40 @@ public void remove() {
}
}
+ /**
+ * Obtain full standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Long names of months
+ */
+ static String[] getStandaloneLongMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.LONG_STANDALONE, locale);
+ }
+ return monthNames;
+ }
+
+ /**
+ * Obtain short standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Short names of months
+ */
+ static String[] getStandaloneShortMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.SHORT_STANDALONE, locale);
+ }
+ return monthNames;
+ }
Review Comment:
These two methods are identical except for Calendar.SHORT_STANDALONE vs. Calendar.LONG_STANDALONE? Why duplicate? Javadoc: "Obtain..." -> "Gets..."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124901550
Interesting. two unicode issues - both easy to fix, and one issue that seems to work on Java 11, but 8
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124983610
I wasn't using safe as in security, I was using safe as in an invalid assumption which might break. However I have just checked and all the locales in Locale.getAvailableLocales() do only have 12 values. I'll swap to the enum as I agree they are generally safer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on a diff in pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on code in PR #892:
URL: https://github.com/apache/commons-lang/pull/892#discussion_r873082132
##########
src/main/java/org/apache/commons/lang3/time/DateUtils.java:
##########
@@ -1831,4 +1831,40 @@ public void remove() {
}
}
+ /**
+ * Obtain full standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Long names of months
+ */
+ static String[] getStandaloneLongMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.LONG_STANDALONE, locale);
+ }
+ return monthNames;
+ }
+
+ /**
+ * Obtain short standalone month names as used in "LLLL" date formatting.
+ * @param locale Locale
+ * @return Short names of months
+ */
+ static String[] getStandaloneShortMonths(final Locale locale) {
+ // Unfortunately standalone month names are not available in DateFormatSymbols,
+ // so we have to extract them.
+ final Calendar calendar = Calendar.getInstance(locale);
+ final int monthCount = calendar.getMaximum(Calendar.MONTH) + 1;
+ final String[] monthNames = new String[monthCount];
+ for (int i = 0; i < monthCount; i++) {
+ calendar.set(Calendar.MONTH, i);
+ monthNames[i] = calendar.getDisplayName(Calendar.MONTH, Calendar.SHORT_STANDALONE, locale);
+ }
+ return monthNames;
+ }
Review Comment:
refactored into one method and changed javadoc. While doing this I looked at the Calendar source and Calendar#getDisplayNames seems to be more efficient than using the Calendar#getDisplayName method multiple times, so I am now using that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] stevebosman-oc commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
stevebosman-oc commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1125057255
Unfortunately there appears to be a bug in Java 8 (I have 1.8_0332) around getDisplayName of standalone values
This
Month.MARCH.getDisplayName(TextStyle.FULL_STANDALONE, Locale.GERMAN)
returns "3" using Java 8, but using Java 11 it returns "März"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-lang] garydgregory commented on pull request #892: LANG-1680 Add support for standalone month formats
Posted by GitBox <gi...@apache.org>.
garydgregory commented on PR #892:
URL: https://github.com/apache/commons-lang/pull/892#issuecomment-1124969397
Safe? I'm not sure how this is a security issue but an enum seems "safer" than open ended code...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org