http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.h File src/extensions/experimental/datetime-format.h (right):
http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.h#newcode37 src/extensions/experimental/datetime-format.h:37: } U_USING_ICU_NAMESPACE is set to 0 for all the projects that rely on ICU in the latest version of icu.gyp. So, I guess you don't need this any more. http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/datetime-format.h#newcode60 src/extensions/experimental/datetime-format.h:60: // Get list of months - narrow or wide format. how about formatting vs stand-alone? For use in menus, stand-alone names (as opposed to "formatting" names) are needed. We need to review the spec and add this distinction if that's not yet covered. http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n-utils.cc File src/extensions/experimental/i18n-utils.cc (right): http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n-utils.cc#newcode63 src/extensions/experimental/i18n-utils.cc:63: result->setTo(tmp); *result = icu::UnicodeString::fromUTF8(*ascii_value) would work, too. However, what you did is as good. http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n-utils.h File src/extensions/experimental/i18n-utils.h (right): http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n-utils.h#newcode38 src/extensions/experimental/i18n-utils.h:38: } this is not necessary any more with the latest icu.gyp in the chrome tree. http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n.js File src/extensions/experimental/i18n.js (right): http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n.js#newcode155 src/extensions/experimental/i18n.js:155: var shortDate = 'yMMdd'; Hmm... this is problematic in some locales. In Korean, SHORT date format does not use zero-padding, but there is a pattern for MMdd in case zero-padded month number and day number are desired. If the skeleton has 'MMdd', the pattern picked up will do zero-padding, which is not actual 'Short' format for Korean in CLDR. Perhaps, 'yMd' works better for some locales, but it may break other locales in the opposite direction. I guess the best is to use real SHORT date style directly. It's more involved, but I don't see any work around that. http://codereview.chromium.org/7014019/diff/8001/src/extensions/experimental/i18n.js#newcode163 src/extensions/experimental/i18n.js:163: settings.hasOwnProperty('timeType')) { After our discussion, this point became unapplicable because you're gonna change datetype/timetype handling per our discussion/the comment above. Just for the record, I'm leaving it alone (in case we change our mind again :-)) How about this? var skeleton = ""; if (has skeleton) { skeleton = settings... } else { if (has ('dateType')) { var dt = settings['dateType']; if (dt === 'short') skeleton = shortDate; else if (dt === 'medium') skeleton = mediumDate; } if (has timeType) { var tt = .... if (tt is short) skeleton += shortTime; .... } if (skeleton == "") skeleton = shortTime + shortDate } http://codereview.chromium.org/7014019/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
