Skip to content

Commit

Permalink
[Translation] Fixed regression: When only one rule is passed to trans…
Browse files Browse the repository at this point in the history
…Choice(), this rule should be used
  • Loading branch information
webmozart committed Aug 23, 2013
1 parent ee3cb88 commit 0951b8d
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 31 deletions.
8 changes: 8 additions & 0 deletions src/Symfony/Component/Translation/MessageSelector.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* MessageSelector.
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Bernhard Schussek <bschussek@gmail.com>
*
* @api
*/
Expand Down Expand Up @@ -73,7 +74,14 @@ public function choose($message, $number, $locale)
}

$position = PluralizationRules::get($number, $locale);

if (!isset($standardRules[$position])) {
// when there's exactly one rule given, and that rule is a standard
// rule, use this rule
if (1 === count($parts) && isset($standardRules[0])) {
return $standardRules[0];
}

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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ public function getTransChoiceTests()
array('There are 0 apples', 'There is 1 apple|There are %count% apples', 0, array('%count%' => 0)),
array('There is 1 apple', 'There is 1 apple|There are %count% apples', 1, array('%count%' => 1)),
array('There are 10 apples', 'There is 1 apple|There are %count% apples', 10, array('%count%' => 10)),
// custom validation messages may be coded with a fixed value
array('There are 2 apples', 'There are 2 apples', 2, array('%count%' => 2)),
);
}
}
72 changes: 45 additions & 27 deletions src/Symfony/Component/Translation/Tests/MessageSelectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,56 +25,74 @@ public function testChoose($expected, $id, $number)
$this->assertEquals($expected, $selector->choose($id, $number, 'en'));
}

public function testReturnMessageIfExactlyOneStandardRuleIsGiven()
{
$selector = new MessageSelector();

$this->assertEquals('There are two apples', $selector->choose('There are two apples', 2, 'en'));
}

/**
* @expectedException InvalidArgumentException
* @dataProvider getNonMatchingMessages
* @expectedException \InvalidArgumentException
*/
public function testChooseWhenNoEnoughChoices()
public function testThrowExceptionIfMatchingMessageCannotBeFound($id, $number)
{
$selector = new MessageSelector();

$selector->choose('foo', 10, 'en');
$selector->choose($id, $number, 'en');
}

public function getNonMatchingMessages()
{
return array(
array('{0} There is no apple|{1} There is one apple', 2),
array('{1} There is one apple|]1,Inf] There are %count% apples', 0),
array('{1} There is one apple|]2,Inf] There are %count% apples', 2),
array('{0} There is no apple|There is one apple', 2),
);
}

public function getChooseTests()
{
return array(
array('There is no apples', '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 0),
array('There is no apples', '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 0),
array('There is no apples', '{0}There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 0),
array('There is no apple', '{0} There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 0),
array('There is no apple', '{0} There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 0),
array('There is no apple', '{0}There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 0),

array('There is one apple', '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 1),
array('There is one apple', '{0} There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 1),

array('There is %count% apples', '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 10),
array('There is %count% apples', '{0} There is no apples|{1} There is one apple|]1,Inf]There is %count% apples', 10),
array('There is %count% apples', '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples', 10),
array('There are %count% apples', '{0} There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 10),
array('There are %count% apples', '{0} There is no apple|{1} There is one apple|]1,Inf]There are %count% apples', 10),
array('There are %count% apples', '{0} There is no apple|{1} There is one apple|]1,Inf] There are %count% apples', 10),

array('There is %count% apples', 'There is one apple|There is %count% apples', 0),
array('There is one apple', 'There is one apple|There is %count% apples', 1),
array('There is %count% apples', 'There is one apple|There is %count% apples', 10),
array('There are %count% apples', 'There is one apple|There are %count% apples', 0),
array('There is one apple', 'There is one apple|There are %count% apples', 1),
array('There are %count% apples', 'There is one apple|There are %count% apples', 10),

array('There is %count% apples', 'one: There is one apple|more: There is %count% apples', 0),
array('There is one apple', 'one: There is one apple|more: There is %count% apples', 1),
array('There is %count% apples', 'one: There is one apple|more: There is %count% apples', 10),
array('There are %count% apples', 'one: There is one apple|more: There are %count% apples', 0),
array('There is one apple', 'one: There is one apple|more: There are %count% apples', 1),
array('There are %count% apples', 'one: There is one apple|more: There are %count% apples', 10),

array('There is no apples', '{0} There is no apples|one: There is one apple|more: There is %count% apples', 0),
array('There is one apple', '{0} There is no apples|one: There is one apple|more: There is %count% apples', 1),
array('There is %count% apples', '{0} There is no apples|one: There is one apple|more: There is %count% apples', 10),
array('There is no apple', '{0} There is no apple|one: There is one apple|more: There are %count% apples', 0),
array('There is one apple', '{0} There is no apple|one: There is one apple|more: There are %count% apples', 1),
array('There are %count% apples', '{0} There is no apple|one: There is one apple|more: There are %count% apples', 10),

array('', '{0}|{1} There is one apple|]1,Inf] There is %count% apples', 0),
array('', '{0} There is no apples|{1}|]1,Inf] There is %count% apples', 1),
array('', '{0}|{1} There is one apple|]1,Inf] There are %count% apples', 0),
array('', '{0} There is no apple|{1}|]1,Inf] There are %count% apples', 1),

// Indexed only tests which are Gettext PoFile* compatible strings.
array('There are %count% apples', 'There is one apple|There are %count% apples', 0),
array('There is one apple', 'There is one apple|There are %count% apples', 1),
array('There are %count% apples', 'There is one apple|There are %count% apples', 2),

// Tests for float numbers
array('There is almost one apple', '{0} There is no apples|]0,1[ There is almost one apple|{1} There is one apple|[1,Inf] There is more than one apple', 0.7),
array('There is one apple', '{0} There is no apples|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 1),
array('There is more than one apple', '{0} There is no apples|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 1.7),
array('There is no apples', '{0} There is no apples|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0),
array('There is no apples', '{0} There is no apples|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0.0),
array('There is no apples', '{0.0} There is no apples|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0),
array('There is almost one apple', '{0} There is no apple|]0,1[ There is almost one apple|{1} There is one apple|[1,Inf] There is more than one apple', 0.7),
array('There is one apple', '{0} There is no apple|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 1),
array('There is more than one apple', '{0} There is no apple|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 1.7),
array('There is no apple', '{0} There is no apple|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0),
array('There is no apple', '{0} There is no apple|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0.0),
array('There is no apple', '{0.0} There is no apple|]0,1[There are %count% apples|{1} There is one apple|[1,Inf] There is more than one apple', 0),
);
}
}
7 changes: 3 additions & 4 deletions src/Symfony/Component/Translation/Tests/TranslatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -277,16 +277,15 @@ public function testTransChoiceFallbackBis()
$this->assertEquals('10 things', $translator->transChoice('some_message2', 10, array('%count%' => 10)));
}

/**
* @expectedException \InvalidArgumentException
*/
public function testTransChoiceFallbackWithNoTranslation()
{
$translator = new Translator('ru', new MessageSelector());
$translator->setFallbackLocale('en');
$translator->addLoader('array', new ArrayLoader());

$this->assertEquals('10 things', $translator->transChoice('some_message2', 10, array('%count%' => 10)));
// consistent behavior with Translator::trans(), which returns the string
// unchanged if it can't be found
$this->assertEquals('some_message2', $translator->transChoice('some_message2', 10, array('%count%' => 10)));
}
}

Expand Down

0 comments on commit 0951b8d

Please sign in to comment.