Hi,

There is a line in the module's code like this:

$vote_text = format_plural($votes, 'vote', 'votes');

I believe it should be changed to sth like this:

$vote_text = format_plural($votes, '1 vote', '@count votes');

Otherwise it is not possible to properly translate vote/votes to languages which have more than 1 plural form (Polish for instance).

Here is a sample translation for Polish (properly working with hacked module's code as shown above):

"Plural-Forms: nplurals=3; plural=((n==1)?(0):(((((n%10)>=2)&&((n%10)<=4))&&(((n%100)<10)||((n%100)>=20)))?(1):2));\n"

#: /fivestar/preview/node/average/dual/5/0/1
msgid "1 vote"
msgid_plural "@count votes"
msgstr[0] "1 głos"
msgstr[1] "@count głosy"
msgstr[2] "@count[2] głosów"

Best regards,
Bartek

Comments

quicksketch’s picture

Status:Active» Needs review
StatusFileSize
new884 bytes

Thanks, I wasn't aware this was the proper way to use format_plural. Since I need to wrap the count in a span for CSS themability, could you verify this is the correct approach?

bartekg’s picture

You may want to have a look at the format_plural documentation:
http://api.drupal.org/api/function/format_plural

The most important bits in this case are:

Do not use @count in the singular string.

and

Note that you do not need to include @count in this array. This replacement is done automatically for the plural case.

I believe the final solution should be:

$output .= ' <span class="total-votes">'. format_plural($votes, '<span>1</span> vote', '<span>@count</span> votes') .'</span>';

Regards,
Bartek

PS.
And thanks a lot for a great module!

quicksketch’s picture

Status:Needs review» Fixed

Thanks, I've put it into the latest CVS!

mooffie’s picture

Version:5.x-1.10» 5.x-1.x-dev
Status:Fixed» Needs review
StatusFileSize
new990 bytes
new1.06 KB

We left out the parentheses...

Here they are. There are two ways to do the parentheses. 'version1' is closer to the original code. 'version2' is shorter. you decide.

bartekg’s picture

It's not me to decide however I suggest to go with the second version.

If not, I believe the first version should be changed a bit. The following line:

$output .= ' <span class="total-votes">'. t('(!total_votes_text)', array('!total_votes_text' => $total_votes_text)) .'</span>';

probably should be changed to:

$output .= ' <span class="total-votes">('. $total_votes_text .')</span>';

In other words: there is no need to use t() function here since the output is already translated by format_plural.

mooffie’s picture

Status:Needs review» Reviewed & tested by the community

It's not me to decide however I suggest to go with the second version.

I too prefer 'version2'. I didn't say so because I didn't want to influence Nate's thoughts. But my rationale isn't linguistic: it's just that this extra level of abstraction --this extra t()-- looks ludicrous; let's do everything in format_plural().

$output .= ' <span class="total-votes">('. $total_votes_text .')</span>';

I did consider this possibility for an instant, but I don't like it: 1. we shouldn't force desginers to have these parentheses in their theme; 2. do we really know that all languages use these specific two characters to delimit text?

mooffie’s picture

I too prefer 'version2'.

Well. No, I really don't know which version is the more 'beautiful' one. Nate, you decide. Or consult some maven. Or just throw a coin ;-)

mooffie’s picture

StatusFileSize
new987 bytes

I've changed my mind. I think what bgola suggests, in #5, is the way to go.

The parentheses, I believe, belong more strongly to the theme than to the text. Here's the new patch, 'version3'.

If the designer wants to get rid of the parentheses, he'll have to override theme_fivestar_summary(). In Drupal 6 we should make this function a .tpl file to make it easy.

quicksketch’s picture

Status:Reviewed & tested by the community» Fixed

Thanks moofie! I agree, the last patch is the way I'd take it also. Committed in 5 and 6 branches.

Anonymous’s picture

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.