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

Fix symbol resolving with pluralization #636

Conversation

movermeyer
Copy link
Contributor

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 1b5e345.

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.

@movermeyer movermeyer marked this pull request as ready for review August 23, 2022 17:53
@radar
Copy link
Collaborator

radar commented Oct 6, 2022

Nice and simple. Thank you :)

@radar radar merged commit c5ccd28 into ruby-i18n:master Oct 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Following Symbols doesn't work with Pluralization lookup
2 participants