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.

Sign up for GitHub

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

Jump to bottom

[ticket/10345] Add a system to allow multiple plural forms #363

Closed
wants to merge 1 commit into from
Closed

[ticket/10345] Add a system to allow multiple plural forms #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
170 changes: 169 additions & 1 deletion phpBB/includes/session.php
View file
Open in desktop
Expand Up @@ -1972,12 +1972,15 @@ function lang()
{
if (is_int($args[$i]))
{
$use_plural_form = $this->get_plural_form($args[$i]);
$numbers = array_keys($lang);

foreach ($numbers as $num)
{
if ($num > $args[$i])
if ($num > $use_plural_form)
{
// This is basically just some lazy backwards compatible stuff.
// If the key we need to use does not exist, it takes the previous one.
break;
}

Expand All @@ -1999,6 +2002,171 @@ function lang()
return call_user_func_array('sprintf', $args);
}

/**
* Determine which plural form we should use.
* For some languages this is not as simple as for English.
*/
function get_plural_form($number, $force_rule = false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Parameters should be documented in the docblock.

Apparently there is a return value also, which should be documented as well.

{
if ($number == 0)
{
// We use special language strings for 0, so it's "no users" instead of "0 users"
return 0;
}

// Default to english system
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't see what $plural_rule is actually set to on github due to it cutting off the later part of that line, but in common.php below you default rule to 1 which is not English.

Also, if "English" stays it should be capitalized.

Adding a constant for the default rule seems like it might be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

English is Rule #1 as Mozilla starts counting with 0,
see wiki article for details: http://wiki.phpbb.com/Plural_Rules#Rule_.231

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I misread this, default is 1 which is English.

Does everything work as expected if language pack provides no plural forms (i.e. any of the currently existing ones) and we treat it as English instead of no plurals (0)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code acts like it does now, the only thing that is a must is, that the lang-key is an array with at least one element.
The only thing I changed is to determinate which "plural"-case we use.

$plural_rule = ($force_rule !== false) ? $force_rule : ((isset($this->lang['PLURAL_RULE'])) ? $this->lang['PLURAL_RULE'] : 1);
$plural_rule = max(0, min($plural_rule, 15));
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless there is an actual reason for this clamping, invalid input should be rejected.

The min part is in particular highly questionable, you're going to silently alter plural rule 16 into "icelandic".


switch ($plural_rule)
{
case 0:
/**
* Families: Asian (Chinese, Japanese, Korean, Vietnamese), Persian, Turkic/Altaic (Turkish), Thai, Lao
* 1 - everything: 1, 2, ...
Copy link
Contributor

Choose a reason for hiding this comment

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

This switch statement needs proper attribution, both for credit and so that we know whom to blame if they got any part of it wrong.

*/
return 1;

case 1:
/**
* Families: Germanic (Danish, Dutch, English, Faroese, Frisian, German, Norwegian, Swedish), Finno-Ugric (Estonian, Finnish, Hungarian), Language isolate (Basque), Latin/Greek (Greek), Semitic (Hebrew), Romanic (Italian, Portuguese, Spanish, Catalan)
* 1 - 1
* 2 - everything else: 2, 3, ...
*/
return ($number == 1) ? 1 : 2;

case 2:
/**
* Families: Romanic (French, Brazilian Portuguese)
* 1 - 1 normaly this would also apply to 0
* 2 - everything else: 2, 3, ...
*/
return ($number == 1) ? 1 : 2;

case 3:
/**
* Families: Baltic (Latvian)
* 1 - ends in 1, not 11: 1, 21, ... 101, 121, ...
* 2 - everything else: 2, 3, ... 10, 11, 12, ... 20, 22, ...
*/
return (($number % 10 == 1) && ($number % 100 != 11)) ? 1 : 2;

case 4:
/**
* Families: Celtic (Scottish Gaelic)
* 1 - is 1 or 11: 1, 11
* 2 - is 2 or 12: 2, 12
* 3 - others between 3 and 19: 3, 4, ... 10, 13, ... 18, 19
* 4 - everything else: 20, 21, ...
*/
return ($number == 1 || $number == 11) ? 1 : (($number == 2 || $number == 12) ? 2 : (($number >= 3 && $number <= 19) ? 3 : 4));

case 5:
/**
* Families: Romanic (Romanian)
* 1 - 1
* 2 - ends in 01-19: 2, 3, ... 19, 101, 102, ... 119, 201, ...
* 3 - everything else: 20, 21, ...
*/
return ($number == 1) ? 1 : ((($number == 0) || (($number % 100 > 0) && ($number % 100 < 20))) ? 2 : 3);

case 6:
/**
* Families: Baltic (Lithuanian)
* 1 - ends in 1, not 11: 1, 21, 31, ... 101, 121, ...
* 2 - ends in 0 or ends in 10-20: 10, 11, 12, ... 19, 20, 30, 40, ...
* 3 - everything else: 2, 3, ... 8, 9, 22, 23, ... 29, 32, 33, ...
*/
return (($number % 10 == 1) && ($number % 100 != 11)) ? 1 : ((($number % 10 < 2) || (($number % 100 >= 10) && ($number % 100 < 20))) ? 2 : 3);

case 7:
/**
* Families: Slavic (Croatian, Serbian, Russian, Ukrainian)
* 1 - ends in 1, not 11: 1, 21, 31, ... 101, 121, ...
* 2 - ends in 2-4, not 12-14: 2, 3, 4, 22, 23, 24, 32, ...
* 3 - everything else: 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 25, 26, ...
*/
return (($number % 10 == 1) && ($number % 100 != 11)) ? 1 : ((($number % 10 >= 2) && ($number % 10 <= 4) && (($number % 100 < 10) || ($number % 100 >= 20))) ? 2 : 3);

case 8:
/**
* Families: Slavic (Slovak, Czech)
* 1 - 1
* 2 - 2, 3, 4
* 3 - everything else: 5, 6, 7, ...
*/
return ($number == 1) ? 1 : ((($number >= 2) && ($number <= 4)) ? 2 : 3);

case 9:
/**
* Families: Slavic (Polish)
* 1 - 1
* 2 - ends in 2-4, not 12-14: 2, 3, 4, 22, 23, 24, 32, ... 104, 122, ...
* 3 - everything else: 5, 6, ... 11, 12, 13, 14, 15, ... 20, 21, 25, ...
*/
return ($number == 1) ? 1 : ((($number % 10 >= 2) && ($number % 10 <= 4) && (($number % 100 < 12) || ($number % 100 > 14))) ? 2 : 3);

case 10:
/**
* Families: Slavic (Slovenian, Sorbian)
* 1 - ends in 01: 1, 101, 201, ...
* 2 - ends in 02: 2, 102, 202, ...
* 3 - ends in 03-04: 3, 4, 103, 104, 203, 204, ...
* 4 - everything else: 5, 6, 7, 8, 9, 10, 11, ...
*/
return ($number % 100 == 1) ? 1 : (($number % 100 == 2) ? 2 : ((($number % 100 == 3) || ($number % 100 == 4)) ? 3 : 4));

case 11:
/**
* Families: Celtic (Irish Gaeilge)
* 1 - 1
* 2 - 2
* 3 - is 3-6: 3, 4, 5, 6
* 4 - is 7-10: 7, 8, 9, 10
* 5 - everything else: 0, 11, 12, ...
*/
return ($number == 1) ? 1 : (($number == 2) ? 2 : (($number >= 3 && $number <= 6) ? 3 : (($number >= 7 && $number <= 10) ? 4 : 5)));

case 12:
/**
* Families: Semitic (Arabic)
* 1 - 1
* 2 - 2
* 3 - ends in 03-10: 3, 4, ... 10, 103, 104, ... 110, 203, 204, ...
* 4 - ends in 11-99: 11, ... 99, 111, 112, ...
* 5 - everything else: 100, 101, 102, 200, 201, 202, ...
*/
return ($number == 1) ? 1 : (($number == 2) ? 2 : ((($number % 100 >= 3) && ($number % 100 <= 10)) ? 3 : ((($number % 100 >= 11) && ($number % 100 <= 99)) ? 4 : 5)));

case 13:
/**
* Families: Semitic (Maltese)
* 1 - 1
* 2 - ends in 01-10: 2, 3, ... 9, 10, 101, 102, ...
* 3 - ends in 11-19: 11, 12, ... 18, 19, 111, 112, ...
* 4 - everything else: 20, 21, ...
*/
return ($number == 1) ? 1 : ((($number == 0) || (($number % 100 > 1) && ($number % 100 < 11))) ? 2 : ((($number % 100 > 10) && ($number % 100 < 20)) ? 3 : 4));

case 14:
/**
* Families: Slavic (Macedonian)
* 1 - ends in 1: 1, 11, 21, ...
* 2 - ends in 2: 2, 12, 22, ...
* 3 - everything else: 3, 4, ... 10, 13, 14, ... 20, 23, ...
*/
return ($number % 10 == 1) ? 1 : (($number % 10 == 2) ? 2 : 3);

case 15:
/**
* Families: Icelandic
* 1 - ends in 1, not 11: 1, 21, 31, ... 101, 121, 131, ...
* 2 - everything else: 2, 3, ... 10, 11, 12, ... 20, 22, ...
*/
return (($number % 10 == 1) && ($number % 100 != 11)) ? 1 : 2;
}
}

/**
* Add Language Items - use_db and use_help are assigned where needed (only use them to force inclusion)
*
Expand Down
5 changes: 5 additions & 0 deletions phpBB/language/en/common.php
View file
Open in desktop
Expand Up @@ -45,6 +45,11 @@
'DATE_FORMAT' => '|d M Y|', // 01 Jan 2007 (with Relative days enabled)
'USER_LANG' => 'en-gb',

// You can define different rules for the determination of plural forms here.
// See http://wiki.phpbb.com/Plural_Rules for more information
// or ask the translation manager for help.
'PLURAL_RULE' => 1,

'1_DAY' => '1 day',
'1_MONTH' => '1 month',
'1_YEAR' => '1 year',
Expand Down
18 changes: 18 additions & 0 deletions tests/user/lang_test.php
View file
Open in desktop
Expand Up @@ -54,5 +54,23 @@ public function test_user_lang_sprintf()
// Bug PHPBB3-9949
$this->assertEquals($user->lang('ARRY', 1, 2), '1 post');
$this->assertEquals($user->lang('ARRY', 1, 's', 2), '1 post');

// Bug PHPBB3-10345
Copy link
Contributor

Choose a reason for hiding this comment

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

s/bug/ticket/ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shall i change the bug in the previous line as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

I read the diff before reading the ticket, and it looked like a new feature as opposed to a bug fix, thus my comment. But the ticket is actually a bug. But we now use "ticket" in commit messages for all kinds of tickets. I would say go with whichever makes more sense to you. Changing bug to ticket on 9949 is ok with me.

$user = new user;
$user->lang = array(
'PLURAL_RULE' => 13,
'ARRY' => array(
0 => '%d is 0', // 0
1 => '%d is 1', // 1
2 => '%d ends with 01-10', // ending with 01-10
3 => '%d ends with 11-19', // ending with 11-19
4 => '%d is part of the last rule', // everything else
),
);
$this->assertEquals($user->lang('ARRY', 0), '0 is 0');
$this->assertEquals($user->lang('ARRY', 1), '1 is 1');
$this->assertEquals($user->lang('ARRY', 103), '103 ends with 01-10');
$this->assertEquals($user->lang('ARRY', 15), '15 ends with 11-19');
$this->assertEquals($user->lang('ARRY', 300), '300 is part of the last rule');
}
}