ruby-i18n / i18n

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

Fix symbol resolving with pluralization #636

Closed movermeyer closed 1 year ago

movermeyer commented 2 years ago

What are you trying to accomplish?

Fixes #635. See #635 for discussion of the history of attempts to fix it.

What approach did you choose and why?

Instead of removing the count param within the Symbol resolving code of Backend::Base (as done in #480, then reverted), we remove the count parameter within the lookup code of Backend::Simple.

The impact of these changes

Resolution of Symbols that point to pluralization contexts will work for Backend::Pluralization (with a i18n.plural.rule entry).

Testing

Thankfully, each stage of the saga that lead to this PR, the tests for the failing cases were added:

So we can have confidence that at least all the known cases are covered.

I added a test that is similar to the one added in #480, but while using the Backend::Pluralization and not Backend::Base. This covers the case that was missing that would have caught the issue with the revert done in https://github.com/ruby-i18n/i18n/commit/1b5e34553003ca3b42b842769e86c98d5e3b71d4.

Anecdotally, I've tested it with our usage of the ruby-cldr exported data, (which contains a lot of aliases/Symbols from the CLDR data), and with this patch lookups using Backend::Pluralization no longer fail.

radar commented 1 year ago

Nice and simple. Thank you :)