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

Missing plural keys wrong locale displayed, does not respect ignore list #310

Closed
Gargron opened this issue Oct 26, 2018 · 7 comments
Closed

Comments

@Gargron
Copy link
Contributor

Gargron commented Oct 26, 2018

This is my mistake. For some reason, a missing plural in "it" shows up as missing in "zh-TW", and before that, missing plurals in "cs" also showed up as "zh-TW". I have no idea why, must be some key getting overwritten somewhere.

Also, I discovered I have a weird case where I have a structure like:

platforms:
  other: unknown platform

And unfortunately it shows up as missing "one" even though it's not a plural. Maybe when deciding based on only "other" being present, a condition should be that it contains the "count" variable as well?

@glebm
Copy link
Owner

glebm commented Oct 26, 2018

For some reason, a missing plural in "it" shows up as missing in "zh-TW", and before that, missing plurals in "cs" also showed up as "zh-TW". I have no idea why, must be some key getting overwritten somewhere.

If you send a PR with a failing test, I'll look into this one.

@glebm
Copy link
Owner

glebm commented Oct 26, 2018

And unfortunately it shows up as missing "one" even though it's not a plural. Maybe when deciding based on only "other" being present, a condition should be that it contains the "count" variable as well?

Looks like platforms has other children:

https://github.com/tootsuite/mastodon/blob/82e7988afcde5b19b99ad9ecf7973560a8a17f7f/config/locales/en.yml#L727-L739

It shouldn't be detected as a plural key at all. plural_forms? here will return false:

next if !plural_forms?(children) || present_keys.superset?(required_keys)

Not sure what's happening here but a failing test would help debug the issue.

@glebm
Copy link
Owner

glebm commented Oct 26, 2018

One issue I see is here:

node.value = children.to_hash

This changes the node, that shouldn't happen (we should create a new by calling derive). I'll try fixing this one, maybe it'll fix the other issues.

glebm added a commit that referenced this issue Oct 26, 2018
@glebm
Copy link
Owner

glebm commented Oct 26, 2018

@Gargron Does the commit above fix any issues?

@Gargron
Copy link
Contributor Author

Gargron commented Oct 26, 2018

Looks like platforms has other children:

In en it does, but because it's things like "firefox" they don't need to be translated into other languages, so in cy it would only have the other child

@glebm
Copy link
Owner

glebm commented Oct 26, 2018

Oh I see. Well, it's a sensible heuristic to add: if a key has only one child and that child does not have the count interpolation, then it's not a plural key.

@Gargron
Copy link
Contributor Author

Gargron commented Oct 26, 2018

This is a CI run from last night (without your fix yet): https://circleci.com/gh/tootsuite/mastodon/26761

Missing translations (1) | i18n-tasks v0.9.27
+--------+--------------------+----------------------------------+
| Locale | Key                | Value in other locales or source |
+--------+--------------------+----------------------------------+
| zh-TW  | sessions.platforms | one                              |
+--------+--------------------+----------------------------------+

If run with -f inspect, I can see it actually refers to the it translation, not zh-TW as it says. I checked all locale files and they all have the correct root key, so it's not a case of it.yml overwriting zh-TW or anything like that. Perhaps your patch already fixes it though, I'll need to test it

glebm added a commit that referenced this issue Oct 26, 2018
glebm added a commit that referenced this issue Oct 26, 2018
glebm added a commit that referenced this issue Oct 26, 2018
glebm added a commit that referenced this issue Oct 26, 2018
Gargron added a commit to Gargron/i18n-tasks that referenced this issue Oct 26, 2018
Otherwise the root key, which is the locale displayed in the report,
is set to the last processed locale

Fix glebm#310
Gargron added a commit to Gargron/i18n-tasks that referenced this issue Oct 26, 2018
glebm pushed a commit that referenced this issue Oct 27, 2018
Otherwise the root key, which is the locale displayed in the report,
is set to the last processed locale

Fix #310
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

No branches or pull requests

2 participants