jeresig / i18n-node-2

Lightweight simple translation module for node.js / express.js with dynamic json storage. Uses common __('...') syntax in app and templates.
MIT License
507 stars 79 forks source link

preferredLocale method is incorrect result. #63

Closed kanbekotori closed 9 years ago

kanbekotori commented 9 years ago

I think this method has bug, my accept-language header is "ja,en-US;q=0.8,en;q=0.6,zh;q=0.4,hu;q=0.2" and locales is "['en', 'zh-CN']",console output "prefLocale" all is undefined. isPreferredLocale() always return false.

preferredLocale: function(req) {
        req = req || this.request;

        if (!req || !req.headers) {
            return;
        }

        var accept = req.headers["accept-language"] || "",
            regExp = /(^|,\s*)([a-z-]+)/gi,
            self = this,
            prefLocale;

        while ((match = regExp.exec(accept))){
            var locale = match[2];
            var parts = locale.split("-");

            if (!prefLocale) {
                if (self.locales[locale]) {
                    prefLocale = locale;
                } else if (parts.length > 1 && self.locales[parts[0]]) {
                    prefLocale = parts[0];
                }
            }
        }

        return prefLocale || this.defaultLocale;
    }
gjuchault commented 9 years ago

Could you also paste the code where you instanciate i18n, and how you bind it with express ?

kanbekotori commented 9 years ago

@gjuchault Sorry! That's my mistake. But how prefLocale work? I want to use "Accept-Language" header to determine i18n infomation, i can't found any method to do it.

This is my middleware for koa.

module.exports = {
  i18n: function() {
    return new I18N(config());
  },
  middleware: function() {
    return function *(next) {
      var cfg = config(this.request);
      this['i18n'] = new I18N(cfg);

      var lang = this.acceptsLanguages();
      for(var l of lang) {
        if(localeSet.has(l)) {
          this.i18n.setLocale(l);
          break;
        }
      }

      yield next;
    }
  }
};

I hope i18n-2 has a method to set locale from "accept-language" header, then i just set a config property to true like {query: true} or {subdomain: true}. Thanks!

gjuchault commented 9 years ago

Ok. I did executed the code below to check if preferredLocale was working :

var accept = "ja,en-US;q=0.8,en;q=0.6,zh;q=0.4,hu;q=0.2";
var regExp = /(^|,\s*)([a-z-]+)/gi;
var self = { locales : { en: [], 'zh-CN': [] } };
var prefLocale;

while ((match = regExp.exec(accept))){
    var locale = match[2];
    var parts = locale.split("-");

    if (!prefLocale) {
        if (self.locales[locale]) {
            prefLocale = locale;
        } else if (parts.length > 1 && self.locales[parts[0]]) {
            prefLocale = parts[0];
        }
    }
}

And the code do set prefLocale to 'en'. So i18n-node2 is not the problem here.

Looking at your code, I don't really get how you use it.

i18n: function() {
    return new I18N(config());
}

What is ̀config() ?

this['i18n'] = new I18N(cfg);

Why do you override the previous method with an object ?

var lang = this.acceptsLanguages();
for(var l of lang) {
  if(localeSet.has(l)) {
    this.i18n.setLocale(l);
    break;
  }
}

Why do you try to emulate preferredLocale ? What is localeSet ?