ruby-i18n / i18n

Internationalization (i18n) library for Ruby
MIT License
976 stars 408 forks source link

Fix I18n.t when locale contains separator #656

Closed tubaxenor closed 1 year ago

tubaxenor commented 1 year ago

v1.13.0 broke a backward compatibility when locale contains separator, example:

# v1.12.0
require "i18n"

I18n.enforce_available_locales = false
I18n.backend.store_translations(:"en-api", v1: {foo: 'Foo in API'})
I18n.locale = :"en-api.v1"
I18n.t("foo")
# => "Foo in API"

# v1.13.0
require "i18n"

I18n.enforce_available_locales = false
I18n.backend.store_translations(:"en-api", v1: {foo: 'Foo in API'})
I18n.locale = :"en-api.v1"
I18n.t("foo")
# => "translation missing: en-api.v1.foo"

This PR brings back the separator separation for locale in normalize_keys.

ngan commented 1 year ago

👋 @radar -- this bug is prevent us from updating to i18n 1.13.0. Anything we can do to help move this along? 🙇

radar commented 1 year ago

Apologies for the delay on this one, it's been a super-busy time of year for me.

@tubaxenor @ngan the build is failing here -- it appears the fix isn't going to work. Could you please investigate a fix that has all the tests passing?

You can run them with bundle exec rake test locally.

ngan commented 1 year ago

Hi @radar, thanks for the response. main looks like it's failing a lot too: https://github.com/ruby-i18n/i18n/actions/runs/4807180573/jobs/8555607559

Just so we can debug this, should we be fixing the errors on main too? Or are they known/expected failures?

radar commented 1 year ago

Thank you. I've fixed up some of those in the #661 PR. I would suggest using that as a base for your work. If I pull your commit in there, it will break what is currently a passing build. Could you please investigate how your change breaks that build?

tubaxenor commented 1 year ago

@radar I've fixed the tests by 4aec070f00a73ed26555e76741fb94656ebda758 🙇

tubaxenor commented 1 year ago

@radar Looks like most of the remaining errors are the same as https://github.com/ruby-i18n/i18n/pull/661, but let me know if there are any remains from my PR 🙇

radar commented 1 year ago

Thank you @tubaxenor. The changes look good to me. I am not sure why the Rails main build is failing. I am guessing that is a temporary situation

radar commented 1 year ago

@fatkodima Just an FYI to you: this PR reverts part of your performance work from #651. Would you like to re-attempt that work for the next release?

fatkodima commented 1 year ago

@radar I retested again - the reverted part does not make much difference, if any. The part that gave most performance improvement stays in the codebase. So I do not think this reverted part is worth reintroducing.