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::Tasks::PluralKeys#non_plural_other? inadvertently missing plural keys #461

Closed
fschwahn opened this issue Jun 14, 2022 · 0 comments · Fixed by #462
Closed

I18n::Tasks::PluralKeys#non_plural_other? inadvertently missing plural keys #461

fschwahn opened this issue Jun 14, 2022 · 0 comments · Fixed by #462
Assignees

Comments

@fschwahn
Copy link

fschwahn commented Jun 14, 2022

This code was in #312 added to not falsely report "lonely" other keys:

def non_plural_other?(s)
s.size == 1 && s.first.leaf? && (!s.first.value.is_a?(String) || !s.first.value.include?('%{count}'))
end

However, this caused an issue for us. We did the following:

  1. Add foo.one in base locale, use translate-missing to add it to all locales
  2. Add foo.other in base locale, use translate-missing to add it to all locales. This did not work.
  3. We have a sanity check on CI which runs i18n-tasks missing -t diff,plural - this also did not catch the missing key.

The reason being that there is just one node, and it does not contain %{count} - which is to be expected if the key is one. This was originally just meant to cover only other keys, but was later amended to cover all possible keys in review.

It seems this was added solely to cover the special case mentioned in #310. In the meantime it became possible to ignore keys when checking plurals (see b04a9cf). I suggest removing this hardcoded special case (ie. the entire non_plural_other?-method), and instead use the ignore config for this case.

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 a pull request may close this issue.

2 participants