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: Exact plural forms for basic MT translators #2454

Merged
merged 3 commits into from
Sep 4, 2024

Conversation

JanCizmar
Copy link
Contributor

No description provided.

@balk-sp
Copy link

balk-sp commented Sep 4, 2024

Looking at your current implementation of getPreparedSourceStrings()

Let's say the target locale is 'en-US' (with target-examples: 'one', 'other')
Let's say my ICU string contains 'zero' (as opposed to '=0').
If I interpret the logic correctly, it would remove the 'zero' case.

In my closed PR, I merge the cases quite carefully, to retain as many cases as possible.

Please also note that your unit-tests typically have target-locale 'cs', which happens to have an abundance of target-examples (one, few many, other), which might yield the desired output in your unit-tests, but would not produce the correct result for English as a target.

I think adding unit-tests for the translation from "cs" to "en" would be valuable.

@JanCizmar
Copy link
Contributor Author

Hey! Look into the test. This case is tested there with =1 and one

@JanCizmar JanCizmar marked this pull request as ready for review September 4, 2024 13:30
@JanCizmar JanCizmar merged commit 12b3fd6 into main Sep 4, 2024
29 checks passed
@JanCizmar JanCizmar deleted the jancizmar/fix-exact-forms-translation branch September 4, 2024 13:41
@balk-sp
Copy link

balk-sp commented Sep 4, 2024

I tested your branch, and just as I predicted, it removes the zero case.

@JanCizmar
Copy link
Contributor Author

JanCizmar commented Sep 4, 2024

Oh, I see. I believe this is correct. English doesn't require the zero case, because this can be usually covered by other. We don't want to keep unnecessary cases. So if you really want to force one case to be there, you can add your exact =1 case.

TolgeeMachine pushed a commit that referenced this pull request Sep 4, 2024
## [3.71.4](v3.71.3...v3.71.4) (2024-09-04)

### Bug Fixes

* Exact plural forms for basic MT translators ([#2454](#2454)) ([12b3fd6](12b3fd6))
@balk-sp
Copy link

balk-sp commented Sep 4, 2024

I'm not sure that when defining "zero/one/other" in your source plural-statement, it would be intuitive that most translations would drop the "zero" case, just because it happens not to be strictly required in the target-locale. This would be so tricky, that Tolgee would have to advise anybody to use "=0/=1/other" unless they are really sure what they are doing and have investigated all target-locale PluralRules (which is hidden in an ibm/icu-package).

You mentioned that "zero" can be grammatically handled by "other", but that seems theoretical. Sure you can say "You have 0 products in your shopping cart", but in my source ICU-message I could have "Your shopping cart is empty." Both are grammatically correct, but not translating the latter version is rather unfortunate.

Sure, using '=0' would be an effective workaround, but I prefer to adhere to the principle of least surprise.

@JanCizmar
Copy link
Contributor Author

"Your shopping cart is empty."

This is great example of the right case for exact=0 case.

zero might be incorrect. See this table
and Prussian example. By using zero form, you might unexpectedly get 10 rendered as "Your shopping cart is empty."

image

@balk-sp
Copy link

balk-sp commented Sep 4, 2024

Well I learned something... there even is a whole world of trickery behind 'one'.
So it would be "19 dogs", "20 no-dogs" and "21 dog" in Prussian. Who would have thought.

@JanCizmar
Copy link
Contributor Author

Yeah...

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.

3 participants