Skip to content

Implemented tagging support for translation. #5547

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 1 commit into from
Closed

Implemented tagging support for translation. #5547

wants to merge 1 commit into from

Conversation

guilhermeblanco
Copy link
Contributor

Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes

Introduced tagging support for translation entries.
The idea behind this patch is to start support for gender specific translations without interfering with current support of Symfony.

Internals: Instead of only adding elements to translation choices without indexing the key, I added a new entry to these choices by considering the key. So, whenever I add a new translation, my transchoice now works not only with integers, but also strings.
Example:

male: His ideas|female: Her ideas|couple: Their ideas

Then I could simply do:

$gender = 'female';

$t = $this->get('translator')->transChoice(
    'male: His ideas|female: Her ideas|couple: Their ideas',
    $gender
);

@asm89
Copy link
Contributor

asm89 commented Sep 19, 2012

How would this work if you want to have: his/her apple(s)?

@merk
Copy link
Contributor

merk commented Sep 19, 2012

I asked guilherme on irc about that, apparently it is either or, not both

@pborreli
Copy link
Contributor

could you add some test to reflect this new feature (gender) ?

@Koc
Copy link
Contributor

Koc commented Sep 19, 2012

Looks like duplicate of #4884

@@ -38,7 +38,7 @@ public function trans($id, array $parameters = array(), $domain = null, $locale
* Translates the given choice message by choosing a translation according to a number.
*
* @param string $id The message id
* @param integer $number The number to use to find the indice of the message
* @param mixed $number The number or tag to use to find the indice of the message
Copy link
Member

Choose a reason for hiding this comment

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

documenting it as integer|string would be more precise than mixed

@guilhermeblanco
Copy link
Contributor Author

@Koc it's a different subject.
Symfony claim to support tagging in translations, but it doesn't. It converts everything to an integer and it never works.
My original motivation to create this patch was a kind of gender specific, but that's why I haven't even considered as that. If you guys really want, I can work on that one, since the other PR you highlighted is overly complex.

@sroddy
Copy link

sroddy commented Sep 21, 2012

@guilhermeblanco my implementation takes inspiration from a well-know use case (facebook) and relies on a format (variable-name/gender/number) which is quite common to translators (pluralization forms). It enforces a great separation of concerns (the programmers that just have to specify gender/number of the variables // the translators that can deal for each language how to use/ignore each variable, or just provide a generic translation). It unifies the use of the trans() method (no more switching between trans and transChoice). It could be improved more and more, making it possible to pass to the options directly objects that implement an interface that provides getGender()/getNumber()/__toString() methods (for now it works with a simple array with gender/number/string keys). The logic behind it is a port of a system that already works on my company's website (www.fubles.com) that is a social network with a feed/message system that has to deal with different gender/number of both the sender and receiver of the messages. it's translated in 4 different languages (and more are coming).

Anyways I think we really should talk first about features, rather than the actual implementation. I bet that everyone who approaches for the first time Doctrine DBAL/ORM or the Symfony Form component would say that they look overly complex (I bet someone would still say that even at the 4th or 5ft time ;-) ). But after having understood all the power and the benefits of their use, most of them will definetly start to love them.

@guilhermeblanco
Copy link
Contributor Author

@sroddy By no means I wanted to degrade your effort.
I indeed think your patch is valid, I just think there's a simpler way to achieve the same thing.
The purpose of my patch is to fix something Symfony advertises as supported, but it's not. We could really speak about your approach that does not only fixes what I highlighted here, but also fixes the gender issue.

Can we chat about possible approaches later on IRC? Unfortunately, my time is very limited (I play 3 roles at work), but we could organize something. Your algorithm is O(n^2), which I think with more refinement it could be optimized, which is I mentioned before I found your patch overly complex. =)

@fabpot
Copy link
Member

fabpot commented Dec 6, 2012

Closing this PR because of #6009 and because it cannot work alongside the current behavior.

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.

8 participants