Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

I18n::Backend::Chain does not support pluralization split between different backends #493

Open
QQism opened this issue Oct 13, 2019 · 5 comments

Comments

@QQism
Copy link

QQism commented Oct 13, 2019

To make it simple, I am using part of the test file tests/backend/chain_test.rb to demonstrate the issue.

# From /i18n/blob/master/test/backend/chain_test.rb
def backend(translations)  
  backend = I18n::Backend::Simple.new 
  translations.each { |locale, data| backend.store_translations(locale, data) }
  backend
end

What I tried to do

I18n::Backend::Chain is used for two different backends (regardless of types). Two backends (main, and fallback) contain the same translation key but with non-overlapping pluralization (i.e. one and other).

main_backend = backend({ en: { plural: { one: "1" } })
fallback_backend = backend({ en: { plural: { other: "%{count}" } } })

I18n.backend = I18n::Backend::Chain.new(main_backend, fallback_backend)

What I expected to happen

I18n.t(:plural, count: 2)

=> "2"

What actually happened

I18n.t(:plural, count: 2)

I18n::InvalidPluralizationData: translation data {:one=>"1"} can not be used with :count => 2. key 'other' is missing. 
From i18n/lib/i18n/backend/base.rb:169:in `pluralize'   

Versions of i18n, rails, and anything else you think is necessary

1.7.0 (current master)

Why it does happen

It seems that I18n::Backend::Chain#translate and I18n::Backend::Chain#translate rely on throw and catch mechanism while I18n::Backend::Base#pluralize uses raise.

I18n::Backend::Chain#translate tries to loop over available backends and uses catch(:exception) to control the flow if a translation is missing (i.e. move to the next available backend). It does miss out on a case when the pluralization has not been available for one backend, InvalidPluralizationData exception is raised, but not throw.

I've been thinking about using begin and rescue wrapped around the catch(:exception) somehow similar to what we have in I18n::Backend::Fallbacks#translate

Please let me know if this is a good idea. Happy to send a PR for review and further discussion 😁.

@papa-cool
Copy link

papa-cool commented Dec 2, 2019

We are facing the same issue using only I18n::Backend::Simple, I18n::Backend::Fallbacks and I18n::Backend::Pluralization. When a translation is missing for some translation rules, the error InvalidPluralizationData is raised and the fallback does not happen.

The strategy to raise an error in I18n::Backend::Pluralization does not seems compatible with the throw catch strategy used in I18n::Backend::Fallbacks.

Would it be possible to throw(:exception, InvalidPluralizationData.new(entry, count, key)) instead of execute raise InvalidPluralizationData.new(entry, count, key)?

I propose the following changes #502.

@radar
Copy link
Collaborator

radar commented Jan 13, 2020

I am able to reproduce this issue with this small Ruby script:

require 'bundler'
Bundler.setup

require 'i18n'
require 'pry'

def backend(translations)
  backend = I18n::Backend::Simple.new
  translations.each { |locale, data| backend.store_translations(locale, data) }
  backend
end

main_backend = backend({ en: { plural: { one: "1" } }})
fallback_backend = backend({ en: { plural: { other: "%{count}" } } })

I18n.backend = I18n::Backend::Chain.new(main_backend, fallback_backend)

p I18n.t(:plural, count: 2)

I will take a look into this a little later on.

@radar
Copy link
Collaborator

radar commented Jan 13, 2020

A PR to fix this issue would be welcome :)

@papa-cool
Copy link

Thank you @radar. What about this PR #502?

@papa-cool
Copy link

papa-cool commented May 25, 2021

Hi @radar.
I have updated my PR #502.
It throw an exception instead of raising an error in Backend::Base and Backend::Pluralization. Furthermore, it avoid the exception caught to be returned in Backend::Base#resolve which ensure fallback to default to work.
Is it fine?

Another approach would be to add I18n::Config#invalid_pluralization_handler like for missing_interpolation_argument_handler, with the same default behavior.
Adding a handler which do nothing and return nil would fix this issue.
Do you prefer this approach?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants