GitHub PullRequestReviewCommentEvent at 2014-02-07 18:33:40
Repository: intl-messageformat
Actor: ericf
Payload: {"comment":{"url":"https://api.github.com/repos/yahoo/intl-messageformat/pulls/comments/9549392","id":9549392,"diff_hunk":"@@ -22,480 +23,406 @@\n         root.IntlMessageFormat = MessageFormat;\n     }\n \n-})(typeof global !== 'undefined' ? global : this, function() {\n-\n-    \"use strict\";\n+})(typeof global !== 'undefined' ? global : this, function (Intl) {\n \n-    var DEFAULT_LOCALE = null,\n+    'use strict';\n \n-        REGEX_WHITE_SPACE         = /\\s/g,\n-        REGEX_STRING_TO_PATTERN   = /\\$?\\{([^\\} ]*)\\}/g,\n+    // -- ES5 Built-ins --------------------------------------------------------\n \n-        DEFAULT_FORMATTERS = {\n-            // TYPE: number\n-            number_integer: function (val, locale) {\n-                // 20000 -> 20,000\n-                return (new Intl.NumberFormat(locale)).format(val);\n-            },\n-            number_currency: function (val, locale, options) {\n-                // 20000 -> $20,000.00\n-                var currencyFormat = new Intl.NumberFormat(locale, {\n-                    style: 'currency',\n-                    currency: options.currency || options.CURRENCY || 'USD'\n-                });\n-                return currencyFormat.format(val);\n-            },\n-            number_percent: function (val, locale) {\n-                // 20000 -> 200%\n-                return (new Intl.NumberFormat(locale, { style: 'percent'})).format(val);\n-            },\n+    // Purposely using the same implementation as the Intl.js `Intl` polyfill.\n+    // Copyright 2013 Andy Earnshaw, MIT License\n \n-            // TYPE: date\n-            // Date formats\n-            date_short: function (val, locale, options) {\n-                var dateFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    month: 'numeric',\n-                    day  : 'numeric',\n-                    year : '2-digit'\n-                });\n-\n-                return dateFormat.format(val);\n-            },\n+    // Used for proto-less objects which won't have this method.\n+    var hop = Object.prototype.hasOwnProperty;\n \n-            date_medium: function (val, locale, options) {\n-                var dateFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    month: 'short',\n-                    day  : 'numeric',\n-                    year : 'numeric'\n-                });\n+    var realDefineProp = (function () {\n+        try { return !!Object.defineProperty({}, 'a', {}); }\n+        catch (e) { return false; }\n+    })();\n \n-                return dateFormat.format(val);\n-            },\n+    var es3 = !realDefineProp && !Object.prototype.__defineGetter__;\n \n-            date_long: function (val, locale, options) {\n-                var dateFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    month: 'long',\n-                    day  : 'numeric',\n-                    year : 'numeric'\n-                });\n+    var defineProperty = realDefineProp ? Object.defineProperty :\n+            function (obj, name, desc) {\n \n-                return dateFormat.format(val);\n-            },\n+        if ('get' in desc && obj.__defineGetter__) {\n+            obj.__defineGetter__(name, desc.get);\n+        } else if (!hop.call(obj, name) || 'value' in desc) {\n+            obj[name] = desc.value;\n+        }\n+    };\n \n-            date_full: function (val, locale, options) {\n-                var dateFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    weekday: 'long',\n-                    month  : 'long',\n-                    day    : 'numeric',\n-                    year   : 'numeric'\n-                });\n+    var objCreate = Object.create || function (proto, props) {\n+        var obj, k;\n \n-                return dateFormat.format(val);\n-            },\n+        function F() {}\n+        F.prototype = proto;\n+        obj = new F();\n \n-            // TYPE: time\n-            time_short: function (val, locale, options) {\n-                var timeFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone: options.timeZone || null,\n-                    hour    : 'numeric',\n-                    minute  : 'numeric'\n-                });\n+        for (k in props) {\n+            if (hop.call(props, k)) {\n+                defineProperty(obj, k, props[k]);\n+            }\n+        }\n \n-                return timeFormat.format(val);\n-            },\n+        return obj;\n+    };\n \n-            time_medium: function (val, locale, options) {\n-                var timeFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone: options.timeZone || null,\n-                    hour    : 'numeric',\n-                    minute  : 'numeric',\n-                    second  : 'numeric'\n-                });\n+    var fnBind = Function.prototype.bind || function (thisObj) {\n+        var fn   = this,\n+            args = [].slice.call(arguments, 1);\n \n-                return timeFormat.format(val);\n-            },\n+        return function () {\n+            fn.apply(thisObj, args.concat([].slice.call(arguments)));\n+        };\n+    };\n \n-            time_long: function (val, locale, options) {\n-                var timeFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    hour        : 'numeric',\n-                    minute      : 'numeric',\n-                    second      : 'numeric',\n-                    timeZoneName: 'short'\n-                });\n+    // -- MessageFormat --------------------------------------------------------\n \n-                return timeFormat.format(val);\n-            },\n+    function MessageFormat(pattern, locales, formats) {\n+        // Parse string messages into a tokenized JSON structure for traversal.\n+        if (typeof pattern === 'string') {\n+            pattern = parse(pattern);\n+        }\n \n-            time_full: function (val, locale, options) {\n-                var timeFormat = new Intl.DateTimeFormat(locale, {\n-                    timeZone    : options.timeZone || null,\n-                    hour        : 'numeric',\n-                    minute      : 'numeric',\n-                    second      : 'numeric',\n-                    timeZoneName: 'short'\n-                });\n+        // Creates a new object with the default formats as its prototype, then\n+        // it's extended with the `formats` provided to the constructor.\n+        formats = extend(objCreate(MessageFormat.FORMATS), formats);\n \n-                return timeFormat.format(val);\n-            }\n-        },\n+        // Define first because it's used to build the format pattern.\n+        defineProperty(this, '_locale',  {value: this._resolveLocale(locales)});\n \n-        // localeData registered by __addLocaleData()\n-        localeData = {};\n+        // Define the format pattern which is highly optimized for repeated\n+        // `format()` invocations. **Note:** This passes the `locales` set\n+        // provided to the constructor instead of just the resolved locale.\n+        pattern = this._buildFormatPattern(pattern, locales, formats);\n+        defineProperty(this, '_pattern', {value: pattern});\n \n-    /**\n-     Creates MessageFormat object from a pattern, locale and field formatters.\n-     String patterns are broken down Arrays. Objects should match the\n-     following pattern:\n-\n-     ```\n-     {\n-        type: 'plural|gender|select',\n-        valueName: 'string',\n-        offset: 1, // consistent offsets for plurals\n-        options: {}, // keys match options for plurals, gender and selects\n-        formatter: 'string|function' // strings are matched to internal formatters\n-     }\n-     ```\n-\n-     @constructor\n-\n-     @param {Array|String} pattern Array or string that serves as formatting pattern.\n-         Use array for plural and select messages, otherwise use string form.\n-     @param {LocaleList|String} locale Locale for string formatting.\n-     @param {Object} optFieldFormatters Holds user defined formatters for each field (Dojo like).\n-     */\n-    function MessageFormat (pattern, locale, optFieldFormatters) {\n-        var chunks,\n-            matches,\n-            len,\n-            i,\n-            p;\n-\n-        // default locale to null\n-        /*jshint expr:true */\n-        locale || (locale = null);\n-\n-        if (locale) {\n-            // strict value checking for locale when provided\n-            if (\n-                typeof locale !== 'string' || // make sure we have a string\n-                locale.replace(REGEX_WHITE_SPACE,'').length < 2 // it's at least two characters\n-            ) {\n-                throw new RangeError('Invalid language tag.');\n-            }\n-        }\n+        // Bind `format()` method to `this` so it can be passed by reference\n+        // like the other `Intl` APIs.\n+        this.format = fnBind.call(this.format, this);\n+    }\n \n-        // store locale\n-        this.locale = locale;\n+    // Default format options used as the prototype of the `formats` provided to\n+    // the constructor. These are used when constructing the internal\n+    // Intl.NumberFormat and Intl.DateTimeFormat instances.\n+    defineProperty(MessageFormat, 'FORMATS', {\n+        enumerable: true,\n \n+        value: {\n+            number_currency: {\n+                style: 'currency'\n+            },\n \n-        // We calculate the pluralization function used for the specific locale.\n-        // Since this is a bit expensive (if repeated too much) and since the\n-        // locale can change on us without notice, we need to keep track of\n-        // which locale was used in choosing the pluralization function.\n-        // (It's expected that the locale will change very infrequently for\n-        // each MessageFormat object.)\n-        this._pluralLocale = undefined;\n-        this._pluralFunc = undefined;\n+            number_percent: {\n+                style: 'percent'\n+            },\n \n-        // Assume the string passed in is a simple pattern for replacement.\n-        if (typeof pattern === 'string') {\n-            pattern = MessageFormat.parse(pattern);\n-        }\n+            date_short: {\n+                month: 'numeric',\n+                day  : 'numeric',\n+                year : '2-digit'\n+            },\n \n-        // save the pattern internally\n-        this.pattern = pattern;\n+            date_medium: {\n+                month: 'short',\n+                day  : 'numeric',\n+                year : 'numeric'\n+            },\n \n-        // store formatters\n-        this.formatters = optFieldFormatters || {};\n-        /*jshint proto:true*/\n-        this.formatters.__proto__ = DEFAULT_FORMATTERS;\n-    }\n+            date_long: {\n+                month: 'long',\n+                day  : 'numeric',\n+                year : 'numeric'\n+            },\n \n-    /**\n-     Formats pattern with supplied parameters.\n-     Dates, times and numbers are formatted in locale sensitive way.\n-     @param {Array|Object} data\n-     @return {String}\n-     */\n-    MessageFormat.prototype.format = function (data) {\n+            date_full: {\n+                weekday : 'long',\n+                month   : 'long',\n+                day     : 'numeric',\n+                year    : 'numeric'\n+            },\n \n-        var pattern = this.pattern,\n-            tokens,\n-            key,\n-            len,\n-            i;\n+            time_short: {\n+                hour  : 'numeric',\n+                minute: 'numeric'\n+            },\n \n-        if (typeof data !== 'object') {\n-            throw new ReferenceError('`format` expects the first argument to be an Object. ' + typeof data + ' was found.');\n-        }\n+            time_medium:  {\n+                hour  : 'numeric',\n+                minute: 'numeric',\n+                second: 'numeric'\n+            },\n \n-        // the pattern we have is an array, we need to stitch it together\n-        // before moving forward\n-        if (Object.prototype.toString.call(pattern) === '[object Array]') {\n-            // let's not destroy the local pattern\n-            pattern = pattern.concat();\n+            time_long: {\n+                hour        : 'numeric',\n+                minute      : 'numeric',\n+                second      : 'numeric',\n+                timeZoneName: 'short'\n+            },\n \n-            // turn the array into a string\n-            pattern = this._processTokens.call(this, pattern, data);\n+            time_full: {\n+                hour        : 'numeric',\n+                minute      : 'numeric',\n+                second      : 'numeric',\n+                timeZoneName: 'short'\n+            }\n+        }\n+    });\n+\n+    // Define internal private properties for dealing with locale data.\n+    defineProperty(MessageFormat, '__availableLocales__', {value: []});\n+    defineProperty(MessageFormat, '__localeData__', {value: objCreate(null)});\n+    defineProperty(MessageFormat, '__addLocaleData', {value: function (data) {\n+        if (!(data && data.locale)) {\n+            throw new Error('Object passed does not identify itself with a valid language tag');\n         }\n \n-        // make sure we have a string\n-        pattern += '';\n+        if (!data.messageformat) {\n+            throw new Error('Object passed does not contain locale data for IntlMessageFormat');\n+        }\n \n-        // find tokens and replace with the object\n-        tokens = pattern.match(REGEX_STRING_TO_PATTERN);\n+        var availableLocales = MessageFormat.__availableLocales__,\n+            localeData       = MessageFormat.__localeData__;\n \n-        // if there were any tokens found, we need to replace them with the\n-        if (tokens) {\n-            for (i = 0, len = tokens.length; i < len; i++) {\n-                // extract key out of ${key} or {key}\n-                key = tokens[i].charAt(0) === '$' ?\n-                        tokens[i].substr(2) :\n-                        tokens[i].substr(1);\n+        // Message format locale data only requires the first part of the tag.\n+        var locale = data.locale.toLowerCase().split('-')[0];\n \n-                // remove trailing }\n-                key = key.substr(0, key.length - 1);\n+        availableLocales.push(locale);\n+        localeData[locale] = data.messageformat;\n \n-                // replace the token with data[key]\n-                if (data.hasOwnProperty(key)) {\n-                    pattern = pattern.replace(tokens[i], data[key]);\n-                }\n-            }\n+        if (MessageFormat.defaultLocale === undefined) {\n+            MessageFormat.defaultLocale = locale;\n         }\n+    }});\n \n-        return pattern;\n+    // Define public `defaultLocale` property which is set when the first bundle\n+    // of locale data is added.\n+    defineProperty(MessageFormat, 'defaultLocale', {\n+        enumerable: true,\n+        writable  : true\n+    });\n+\n+    MessageFormat.prototype.format = function (values) {\n+        return this._format(this._pattern, values);\n     };\n \n-    /**\n-     Returns resolved options, in this case supported locale.\n-     @return {Object}\n-     */\n     MessageFormat.prototype.resolvedOptions = function () {\n-        // TODO: Figure out what options should be returned for messages\n-        return {};\n+        // TODO: Provide anything else?\n+        return {\n+            locale: this._locale\n+        };\n     };\n \n-    /**\n-     Normalizes the number to option values for plural identification\n-     @param {Number} count Number to normalize\n-     @return {String}\n-     */\n-    MessageFormat.prototype._normalizeCount = function (count) {\n-        var locale = this.locale || DEFAULT_LOCALE,\n-            data,\n-            fn,\n-            parts;\n-\n-        // if the locale isn't set, and there is no default locale set, throw\n-        if (\n-            typeof locale !== 'string' || // make sure we have a string\n-            locale.replace(/\\s/g,'').length < 2 // it's at least two characters\n-        ) {\n-            throw new ReferenceError('No locale data has been provided for this object yet.');\n-        }\n+    MessageFormat.prototype._format = function (pattern, values) {\n+        var result = '',\n+            i, len, part, valueName, value, options;\n \n-        // cache the choice of pluralization function\n-        if (this._pluralLocale !== locale) {\n-            if (locale !== DEFAULT_LOCALE) {\n-                parts = this.locale.toLowerCase().split('-');\n-                while (parts.length) {\n-                    data = localeData[parts.join('_')];\n-                    if (data && data.pluralFunction) {\n-                        fn = data.pluralFunction;\n-                        break;\n-                    }\n-                    parts.pop();\n-                }\n+        for (i = 0, len = pattern.length; i < len; i += 1) {\n+            part = pattern[i];\n+\n+            // Exist early for string parts.\n+            if (typeof part === 'string') {\n+                result += part;\n+                continue;\n             }\n-            if (!fn) {\n-                // While this seems excessive, it's possible the user has a\n-                // complex default locale (such as \"zh-hans-CN\") since the\n-                // default locale can come from a browser setting.\n-                parts = DEFAULT_LOCALE.toLowerCase().split('-');\n-                while (parts.length) {\n-                    data = localeData[parts.join('_')];\n-                    if (data && data.pluralFunction) {\n-                        fn = data.pluralFunction;\n-                        break;\n-                    }\n-                    parts.pop();\n-                }\n+\n+            valueName = part.valueName;\n+\n+            // Enforce that all required values are provided by the caller.\n+            if (!(values && hop.call(values, valueName))) {\n+                throw new Error('A value must be provided for: ' + valueName);\n             }\n-            if (!fn) {\n-                data = localeData[DEFAULT_LOCALE];\n-                fn = (data && data.pluralFunction) || function() {\n-                    return 'other';\n-                };\n+\n+            value   = values[valueName];\n+            options = part.options;\n+\n+            // Recursively format plural and select parts' option — which can be\n+            // a nested pattern structure. The choosing of the option to use is\n+            // abstracted-by and delegated-to the part helper object.\n+            if (options) {\n+                result += this._format(part.getOption(value), values);\n+            } else {\n+                result += part.format(value);","path":"index.js","position":501,"original_position":501,"commit_id":"9cb342495cddab18b3c8ee37aabfda1357c601fe","original_commit_id":"9cb342495cddab18b3c8ee37aabfda1357c601fe","user":{"login":"ericf","id":29096,"avatar_url":"https://gravatar.com/avatar/cf33841918d90fe941f522dde5ee4ff0?d=https%3A%2F%2Fidenticons.github.com%2F19317b848dbb3dcbfb5684c662fd25ff.png&r=x","gravatar_id":"cf33841918d90fe941f522dde5ee4ff0","url":"https://api.github.com/users/ericf","html_url":"https://github.com/ericf","followers_url":"https://api.github.com/users/ericf/followers","following_url":"https://api.github.com/users/ericf/following{/other_user}","gists_url":"https://api.github.com/users/ericf/gists{/gist_id}","starred_url":"https://api.github.com/users/ericf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ericf/subscriptions","organizations_url":"https://api.github.com/users/ericf/orgs","repos_url":"https://api.github.com/users/ericf/repos","events_url":"https://api.github.com/users/ericf/events{/privacy}","received_events_url":"https://api.github.com/users/ericf/received_events","type":"User","site_admin":false},"body":"> Should we always expect the result to have a format function?\r\n\r\nYes because the pattern is pre-processed, there's no need for the extra checks.\r\n\r\n> My thinking would be: \r\n> result += part.format ? part.format(value) : values[valueName]\r\n> \r\n> This would prevent an error such as \"TypeError: Property 'format' of object # is not a function\" from being thrown.\r\n\r\nI disagree, we want this error to be thrown.","created_at":"2014-02-07T18:33:40Z","updated_at":"2014-02-07T18:33:40Z","html_url":"https://github.com/yahoo/intl-messageformat/pull/47#discussion_r9549392","pull_request_url":"https://api.github.com/repos/yahoo/intl-messageformat/pulls/47","_links":{"self":{"href":"https://api.github.com/repos/yahoo/intl-messageformat/pulls/comments/9549392"},"html":{"href":"https://github.com/yahoo/intl-messageformat/pull/47#discussion_r9549392"},"pull_request":{"href":"https://api.github.com/repos/yahoo/intl-messageformat/pulls/47"}}}}