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

Inability to Ignore Specific Pluralization Keys in Arabic Translations #600

Open
tagliala opened this issue Oct 18, 2024 · 7 comments
Open

Comments

@tagliala
Copy link
Contributor

tagliala commented Oct 18, 2024

Hello,

I am opening this issue because I did not find an existing solution or workaround. If this turns out to be a duplicate or addressed elsewhere (e.g., in the documentation), I apologize in advance. This is not a duplicate of #582 as far as I can tell

Problem Description

We are using Weblate for our translations, and due to Weblate issue #4484, we only utilize the one and other pluralization keys for our strings. However, when running i18n-tasks health, we receive warnings about missing pluralization keys (such as zero, two, few, and many) specifically for Arabic.

For example:

+--------+---------------------------------+----------------------------------+
| Locale | Key                             | Value in other locales or source |
+--------+---------------------------------+----------------------------------+
|   ar   | document_search.documents_found | zero, two, few, many             |
+--------+---------------------------------+----------------------------------+

Currently, we are using ignore_missing to suppress these warnings, but this results in ignoring the entire document_search.documents_found key, which is not ideal. We would like to be able to ignore only specific pluralization keys (e.g., zero, two, few, many), while still maintaining checks for one and other.

Request

Is there a way to configure i18n-tasks to ignore only specific pluralization keys (such as zero, two, few, and many), without having to exclude the entire key?

Thank you for your attention, and I look forward to any guidance you can provide.

@davidwessman
Copy link
Collaborator

I do not think this is supported today, and adding ignores for each full key seems hard to maintain.
Seems like we need to allow some locale specific config for this?

Now

base_locale: en
locales: [en, ar]

to something like:

locales:
  en:
    base: true
    plural_keys:
      - one
      - other
  ar:
    plural_keys:
      - zero
      - two
      - few
      - many

Could this be useful for your changes @markedmondson?

What do you think @glebm ?

@glebm
Copy link
Owner

glebm commented Oct 20, 2024

Sounds good to me, and perhaps we use the defaults from here https://github.com/svenfuchs/rails-i18n/blob/master/lib/rails_i18n/pluralization.rb and https://github.com/svenfuchs/rails-i18n/tree/master/lib/rails_i18n/common_pluralizations (per locale definitions are here: https://github.com/svenfuchs/rails-i18n/tree/master/rails/pluralization)

@markedmondson
Copy link
Contributor

we only utilize the one and other pluralization keys for our strings

Is this purely for a workaround for the bug noted?

Can't you configure this using wildcards and per locale settings, eg:

ignore_missing:
  ar:
    - *.zero
    - *.two
    - *.few
    - *.many  

I don't think this impacts the translator pluralization changes I made, however.

tagliala added a commit to diowa/ruby3-rails7-bootstrap-heroku that referenced this issue Oct 22, 2024
@tagliala
Copy link
Contributor Author

Can't you configure this using wildcards and per locale settings, eg:

Apparently not, or I did not get the suggestion

117 ignore_missing:
118   ar:
119     - *.zero
120     - *.two
121     - *.few
122     - *.many
 (<unknown>): did not find expected alphabetic or numeric character while scanning an alias at line 118 column 7 (Psych::SyntaxError)

I've provided a reproducible test case here:

https://github.com/diowa/ruby3-rails7-bootstrap-heroku/tree/tmp/i18n-tasks-600

Before posting, I've also tried

ignore_missing:
  - zero

but after (briefly) reading the code, it seems that pluralization follows different rules, and I still get:

$ i18n-tasks health
#StandWithUkraine
Forest (en, ar) has 10 keys across 2 locales. On average, values are 28 characters long, keys have 2.6 segments, a locale has 5 keys.
Missing translations (1) | i18n-tasks v1.0.14
+--------+-------------------------+----------------------------------+
| Locale | Key                     | Value in other locales or source |
+--------+-------------------------+----------------------------------+
|   ar   | activemodel.models.user | zero, two, few, many             |
+--------+-------------------------+----------------------------------+

tagliala added a commit to diowa/ruby3-rails7-bootstrap-heroku that referenced this issue Oct 22, 2024
@markedmondson
Copy link
Contributor

Can't you configure this using wildcards and per locale settings, eg:

Apparently not, or I did not get the suggestion

117 ignore_missing:
118   ar:
119     - *.zero
120     - *.two
121     - *.few
122     - *.many
 (<unknown>): did not find expected alphabetic or numeric character while scanning an alias at line 118 column 7 (Psych::SyntaxError)

I've provided a reproducible test case here:

https://github.com/diowa/ruby3-rails7-bootstrap-heroku/tree/tmp/i18n-tasks-600

Before posting, I've also tried

ignore_missing:
  - zero

but after (briefly) reading the code, it seems that pluralization follows different rules, and I still get:

$ i18n-tasks health
#StandWithUkraine
Forest (en, ar) has 10 keys across 2 locales. On average, values are 28 characters long, keys have 2.6 segments, a locale has 5 keys.
Missing translations (1) | i18n-tasks v1.0.14
+--------+-------------------------+----------------------------------+
| Locale | Key                     | Value in other locales or source |
+--------+-------------------------+----------------------------------+
|   ar   | activemodel.models.user | zero, two, few, many             |
+--------+-------------------------+----------------------------------+

Apologies, those values need stringifying:

ignore_missing:
  ar:
    - '*.zero'
    - '*.two'
    - '*.few'
    - '*.many'

@tagliala
Copy link
Contributor Author

Apologies, those values need stringifying:

Apologies on my behalf too, I've also tried that, but I forgot to report it. There are no syntax errors, but the result does not change

Here it is:

diff --git a/config/i18n-tasks.yml b/config/i18n-tasks.yml
index b48d6a6..ba966f4 100644
--- a/config/i18n-tasks.yml
+++ b/config/i18n-tasks.yml
@@ -137,7 +137,12 @@ search:
 #   #   Keep in mind the context of all the strings for a more accurate translation.
 
 ## Do not consider these keys missing:
-# ignore_missing:
+ignore_missing:
+  ar:
+    - '*.zero'
+    - '*.two'
+    - '*.few'
+    - '*.many'
 # - 'errors.messages.{accepted,blank,invalid,too_short,too_long}'
 # - '{devise,simple_form}.*'
$ i18n-tasks health
#StandWithUkraine
Forest (en, ar) has 10 keys across 2 locales. On average, values are 28 characters long, keys have 2.6 segments, a locale has 5 keys.
Missing translations (1) | i18n-tasks v1.0.14
+--------+-------------------------+----------------------------------+
| Locale | Key                     | Value in other locales or source |
+--------+-------------------------+----------------------------------+
|   ar   | activemodel.models.user | zero, two, few, many             |
+--------+-------------------------+----------------------------------+
✓ Well done! Every translation is in use.
✓ Good job! No inconsistent interpolations found.
✓ Well done! All data is normalized

@markedmondson
Copy link
Contributor

markedmondson commented Oct 22, 2024

Ah, this is supported in my PR in https://github.com/glebm/i18n-tasks/pull/596/files#diff-3a91d9f9d36a17731fb2e6a43d064bbd3d16867bd2bab2c1627eaf30defd0267R120
On main, ignore_key? is not checked in missing.

Although I noticed I should be passing the locale in - will add that change. (Edit: Done in e804c6a)

HOWEVER, that said, it only checks the base key and not the plural key, which I guess makes sense. We could add support for checking the plural key in my PR relatively simply though if that's acceptable...

next if present_keys.merge(required_keys).any? do |plural_key|
  ignore_key?("#{node.full_key(root: false)}.#{plural_key}", :missing, locale)
end

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

4 participants