Reviewers: Jungshik Shin,

Description:
Change timeType and dateType in i18n date format API into timeStyle and
dateStyle to match the proposal.
I've kept old keys too, until ChromeOS user switches to time/dateStyle.

TEST=i18n.kaziprst.org/datetimeformat.html should show proper results for both
timeType and timeStyle.

Please review this at http://codereview.chromium.org/7244008/

SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/

Affected files:
  M     src/extensions/experimental/datetime-format.cc
  M     src/extensions/experimental/i18n.js


Index: src/extensions/experimental/datetime-format.cc
===================================================================
--- src/extensions/experimental/datetime-format.cc      (revision 8401)
+++ src/extensions/experimental/datetime-format.cc      (working copy)
@@ -293,34 +293,34 @@
     }
   }

-  // Extract date type and time type from settings.
-  icu::UnicodeString date_type;
-  icu::DateFormat::EStyle date_style = icu::DateFormat::kNone;
-  if (I18NUtils::ExtractStringSetting(settings, "dateType", &date_type)) {
-    date_style = GetDateTimeStyle(date_type);
+  // Extract date style and time style from settings.
+  icu::UnicodeString date_style;
+  icu::DateFormat::EStyle icu_date_style = icu::DateFormat::kNone;
+ if (I18NUtils::ExtractStringSetting(settings, "dateStyle", &date_style)) {
+    icu_date_style = GetDateTimeStyle(date_style);
   }

-  icu::UnicodeString time_type;
-  icu::DateFormat::EStyle time_style = icu::DateFormat::kNone;
-  if (I18NUtils::ExtractStringSetting(settings, "timeType", &time_type)) {
-    time_style = GetDateTimeStyle(time_type);
+  icu::UnicodeString time_style;
+  icu::DateFormat::EStyle icu_time_style = icu::DateFormat::kNone;
+ if (I18NUtils::ExtractStringSetting(settings, "timeStyle", &time_style)) {
+    icu_time_style = GetDateTimeStyle(time_style);
   }

-  // Try all combinations of date/time types.
-  if (date_style == icu::DateFormat::kNone &&
-      time_style == icu::DateFormat::kNone) {
+  // Try all combinations of date/time styles.
+  if (icu_date_style == icu::DateFormat::kNone &&
+      icu_time_style == icu::DateFormat::kNone) {
     // Return default short date, short
     return icu::DateFormat::createDateTimeInstance(
         icu::DateFormat::kShort, icu::DateFormat::kShort, icu_locale);
-  } else if (date_style != icu::DateFormat::kNone &&
-             time_style != icu::DateFormat::kNone) {
+  } else if (icu_date_style != icu::DateFormat::kNone &&
+             icu_time_style != icu::DateFormat::kNone) {
     return icu::DateFormat::createDateTimeInstance(
-        date_style, time_style, icu_locale);
-  } else if (date_style != icu::DateFormat::kNone) {
-    return icu::DateFormat::createDateInstance(date_style, icu_locale);
+        icu_date_style, icu_time_style, icu_locale);
+  } else if (icu_date_style != icu::DateFormat::kNone) {
+    return icu::DateFormat::createDateInstance(icu_date_style, icu_locale);
   } else {
-    // time_style != icu::DateFormat::kNone
-    return icu::DateFormat::createTimeInstance(time_style, icu_locale);
+    // icu_time_style != icu::DateFormat::kNone
+    return icu::DateFormat::createTimeInstance(icu_time_style, icu_locale);
   }
 }

Index: src/extensions/experimental/i18n.js
===================================================================
--- src/extensions/experimental/i18n.js (revision 8401)
+++ src/extensions/experimental/i18n.js (working copy)
@@ -145,9 +145,8 @@
  * @param {Object} locale - locale object to pass to formatter.
  * @param {Object} settings - formatting flags:
  *   - skeleton
- *   - dateType
- *   - timeType
- *   - calendar
+ *   - dateStyle
+ *   - timeStyle
  * @private
  * @constructor
  */
@@ -161,25 +160,37 @@
     cleanSettings['skeleton'] = settings['skeleton'];
   } else {
     cleanSettings = {};
-    if (settings.hasOwnProperty('dateType')) {
+    if (settings.hasOwnProperty('dateStyle')) {
+      var ds = settings['dateStyle'];
+      if (!/^short|medium|long|full$/.test(dt)) ds = 'short';
+      cleanSettings['dateStyle'] = ds;
+    } else if (settings.hasOwnProperty('dateType')) {
+      // Obsolete. New spec requires dateStyle, but we'll keep this around
+      // for current users.
+      // TODO(cira): Remove when all internal users switch to dateStyle.
       var dt = settings['dateType'];
       if (!/^short|medium|long|full$/.test(dt)) dt = 'short';
-      cleanSettings['dateType'] = dt;
+      cleanSettings['dateStyle'] = dt;
     }

-    if (settings.hasOwnProperty('timeType')) {
+    if (settings.hasOwnProperty('timeStyle')) {
+      var ts = settings['timeStyle'];
+      if (!/^short|medium|long|full$/.test(tt)) ts = 'short';
+      cleanSettings['timeStyle'] = ts;
+    } else if (settings.hasOwnProperty('timeType')) {
+      // TODO(cira): Remove when all internal users switch to timeStyle.
       var tt = settings['timeType'];
       if (!/^short|medium|long|full$/.test(tt)) tt = 'short';
-      cleanSettings['timeType'] = tt;
+      cleanSettings['timeStyle'] = tt;
     }
   }

   // Default is to show short date and time.
   if (!cleanSettings.hasOwnProperty('skeleton') &&
-      !cleanSettings.hasOwnProperty('dateType') &&
-      !cleanSettings.hasOwnProperty('timeType')) {
-    cleanSettings = {'dateType': 'short',
-                     'timeType': 'short'};
+      !cleanSettings.hasOwnProperty('dateStyle') &&
+      !cleanSettings.hasOwnProperty('timeStyle')) {
+    cleanSettings = {'dateStyle': 'short',
+                     'timeStyle': 'short'};
   }

   locale = v8Locale.__createLocaleOrDefault(locale);


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to