8000 Falling back to first translation if no choice available by lstrojny · Pull Request #6619 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

Falling back to first translation if no choice available #6619

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

Closed
wants to merge 2 commits into from
Closed

Falling back to first translation if no choice available #6619

wants to merge 2 commits into from

Conversation

lstrojny
Copy link
Contributor
@lstrojny lstrojny commented Jan 8, 2013

Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #4228
License of the code: MIT

@stof
Copy link
Member
stof commented Jan 8, 2013

This makes the fallback better, but we should probably log a warning if possible as the translation is still invalid (and so should be fixed to have the proper pluralization in your UI)

@lstrojny
Copy link
Contributor Author
lstrojny commented Jan 8, 2013

@stof as we don’t have a logger in the MessageSelector, whose job would it be to log a message?

@@ -74,7 +74,7 @@ public function choose($message, $number, $locale)

$position = PluralizationRules::get($number, $locale);
if (!isset($standardRules[$position])) {
throw new \InvalidArgumentException(sprintf('Unable to choose a translation for "%s" with locale "%s". Double check that this translation has the correct plural options (e.g. "There is one apple|There are %%count%% apples").', $message, $locale));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to remove the phpdoc exception too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thanks!

@fabpot
Copy link
Member
fabpot commented Jan 8, 2013

There is a PR for adding logging in the Translator component (see #5537 -- this should probably be updated to use the new PSR-3 logger interface)

@lstrojny
Copy link
Contributor Author
lstrojny commented Jan 8, 2013

@fabpot I don’t think this PR should wait for the logging support as it possibly breaks existing code bases.

@lstrojny
Copy link
Contributor Author

ping

@beberlei
Copy link
Contributor

Why break BC on this, if you can make use of inheritance? I see value in both approaches

@jeremylivingston
Copy link
Contributor

We're also encountering this issue.

If a user doesn't pass the MaxLength validation constraint, we are receiving the error:

Twig_Error_Runtime: An exception has been thrown during the rendering of a template ("Unable to choose a translation for "MYMESSAGE" with locale "en". Double check that this translation has the correct plural options (e.g. "There is one apple|There are %count% apples").")

I've provided the exact message in the validator, so I wouldn't think that the translation would be necessary.
Any information on this fix?

@stof
Copy link
Member
stof commented Feb 19, 2013

@jeremylivingston the MaxLength constraint is providing a pluralization, so the translation is done with transChoice, not with trans. this means you still have to provide the pluralization in the translation strings

@jeremylivingston
Copy link
Contributor

@stof What if the message doesn't have separate plural and singular versions? It seems odd to force this even if it may not apply.

I think that there should be a way to specify a single message in cases where you don't require a plural version. It should then know that if there are two messages that the latter is the plural version.

Would this be a better implementation for a future version? I can add the same message for now, but it seems that this solution would be more flexible.

@stof
Copy link
Member
stof commented Feb 22, 2013

@jeremylivingston The alternatives don't need to be in the key. they have to be in the translated string. So the renderer cannot use the key to decide whether the message is translated or no.

@fabpot
Copy link
Member
fabpot commented Mar 6, 2013

related to #6988

@kriswallsmith
Copy link
Contributor

I'm leaning toward 👎 on this. Removing the exception could lead to some confusion when users think their code is doing one thing but is actually doing another. My fix to the validator makes sense because the user is not explicitly calling the translator so we need flexibility there. Otherwise an exception is appropriate.

@fabpot
Copy link
Member
fabpot commented Mar 8, 2013

I totally agree with @kriswallsmith. Closing.

@fabpot fabpot closed this Mar 8, 2013
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.

7 participants
0