DateFormat should respect value of hc in locale

Description

Per Markus and Mark Davis, DateFormat should use different hour cycle to format if the local contains hc extension.

So I try the following

diff --git a/icu4c/source/test/intltest/dtfmttst.cpp b/icu4c/source/test/intltest/dtfmttst.cpp
index 97bca2f00c..5908336f8c 100644
— a/icu4c/source/test/intltest/dtfmttst.cpp
+++ b/icu4c/source/test/intltest/dtfmttst.cpp
@@ -122,6 +122,7 @@ void DateFormatTest::runIndexedTest( int32_t index, UBool exec, const char* &nam
TESTCASE_AUTO(TestMinuteSecondFieldsInOddPlaces);
TESTCASE_AUTO(TestDayPeriodParsing);
TESTCASE_AUTO(TestParseRegression13744);
+ TESTCASE_AUTO(TestHourCycleLocale);

TESTCASE_AUTO_END;
}
@@ -5557,6 +5558,24 @@ void DateFormatTest::TestParseRegression13744() {
assertEquals("Error index", inDate.length(), pos.getErrorIndex());
}

+void DateFormatTest::TestHourCycleLocale() {
+ LocalPointer<DateFormat> dfmt1(DateFormat::createDateTimeInstance(
+ DateFormat::SHORT, DateFormat::SHORT, Locale("fr-u-hc-h11")));
+ SimpleDateFormat* sdtfmt1 = dynamic_cast<SimpleDateFormat*>(dfmt1.getAlias());
+ UnicodeString pattern1;
+ sdtfmt1->toPattern(pattern1);
+
+ LocalPointer<DateFormat> dfmt2(DateFormat::createDateTimeInstance(
+ DateFormat::SHORT, DateFormat::SHORT, Locale("fr-u-hc-h23")));
+ SimpleDateFormat* sdtfmt2 = dynamic_cast<SimpleDateFormat*>(dfmt2.getAlias());
+ UnicodeString pattern2;
+ sdtfmt2->toPattern(pattern2);
+ if (pattern1 == pattern2) {
+ std::string p1;
+ pattern1.toUTF8String(p1);
+ errln("FAILURE! fr-u-hc-h11 and fr-u-hc-h23 get different pattern but both got %s", p1.c_str());
+ }
+}
#endif /* #if !UCONFIG_NO_FORMATTING */

//eof
diff --git a/icu4c/source/test/intltest/dtfmttst.h b/icu4c/source/test/intltest/dtfmttst.h
index efc5673249..f317a3a6ad 100644
— a/icu4c/source/test/intltest/dtfmttst.h
+++ b/icu4c/source/test/intltest/dtfmttst.h
@@ -262,6 +262,7 @@ class DateFormatTest: public CalendarTimeZoneTest {
void TestMinuteSecondFieldsInOddPlaces();
void TestDayPeriodParsing();
void TestParseRegression13744();
+ void TestHourCycleLocale();

private:
UBool showParse(DateFormat &format, const UnicodeString &formattedString);

But I got the following:

$ LD_LIBRARY_PATH=lib:stubdata:tools/ctestfw:$LD_LIBRARY_PATH test/intltest/intltest format/DateFormatTest/TestHourCycleLocale
-----------------------------------------------
IntlTest (C++) Test Suite for
International Components for Unicode 63.1
Bits: 64, Byte order: Little endian, Chars: ASCII
-----------------------------------------------
Options:
all (a) : Off
Verbose (v) : Off
No error messages : Off
Exhaustive (e) : Off
Leaks (l) : Off
utf-8 (u) : Off
notime (T) : Off
noknownissues (K) : Off
Warn on missing data (w) : Off
Threads : 12
-----------------------------------------------

=== Handling test: format/DateFormatTest/TestHourCycleLocale: ===
format {
DateFormatTest {
TestHourCycleLocale {
FAILURE! fr-u-hc-h11 and fr-u-hc-h23 get different pattern but both got dd/MM/y HH:mm

} ERRORS (1) in TestHourCycleLocale (28ms)

} ERRORS (1) in DateFormatTest (29ms)

} ERRORS (1) in format (29ms)

--------------------------------------
Errors in total: 1.
TestHourCycleLocale
DateFormatTest
format

--------------------------------------
Elapsed Time: 00:00:00.029

so regardless which hc we use in the locale, both "fr-u-hc-h11" and "fr-u-hc-h23" use pattern "dd/MM/y HH:mm"

Activity

Show:
Shane Carr
January 9, 2020, 5:40 PM

To-do: the test case Frank posted in the bug description should be added and ensure that it passes.

Caio Lima
February 14, 2020, 1:46 PM

I tested this test case and it doesn’t pass after PR #944. However I’m a bit confused if this should be expected to pass. IIUC, our changes on PR #944 affects cases where we use getBestPattern, however I don’t see it being executed when we get DateFormat::createDateTimeInstance. From DateFormat::create execution, we return SimpleDateFormat(timeStyle, dateStyle, locale, status); or SimpleDateFormat(locale, status); and can’t see those constructors calling into getBestPattern. I’m not sure what should be the next step in this issue.

Shane Carr
February 14, 2020, 11:22 PM

Ok. This is the dateStyle/timeStyle code path. Since those go directly to a pattern, we would have to modify the pattern in order to change the hour cycle. That is less elegant than getBestPattern, where we modify the skeleton rather than the pattern.

thoughts on this? Should we wait for ?

Peter Edberg
March 18, 2020, 6:41 AM

Should we wait for CLDR-13425: Make datetime styles (lengths) map to skeletons instead of patterns?

Yes, I think that would be cleaner. The current PR-944 for this was specifically about DateTimePatternGenerator, not DateFormat. Maybe we can add 68.1 to the versions for this ticket and then do the rest of it in ICU 68 when (hopefully) we will have data for CLDR-13425

Shane Carr
March 18, 2020, 9:49 PM

OK cool. I filed blocked by .

Assignee

Caio Lima

Reporter

Frank Yung-Fong Tang

Components

Reviewer

Peter Edberg

Priority

major

Time Needed

Days

Fix versions

Configure