Skip to content

Commit

Permalink
Make trans + %count% parameter resolve plurals
Browse files Browse the repository at this point in the history
  • Loading branch information
nicolas-grekas committed Oct 6, 2018
1 parent d870a85 commit dc5f3bf
Show file tree
Hide file tree
Showing 38 changed files with 888 additions and 431 deletions.
8 changes: 7 additions & 1 deletion UPGRADE-4.2.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ FrameworkBundle
set the "APP_ENV" environment variable instead.
* The `--no-debug` console option has been deprecated,
set the "APP_DEBUG" environment variable to "0" instead.
* The `Templating\Helper\TranslatorHelper::transChoice()` method has been deprecated, use the `trans()` one instead with a `%count%` parameter.

Messenger
---------
Expand Down Expand Up @@ -219,10 +220,15 @@ Translation
-----------

* The `TranslatorInterface` has been deprecated in favor of `Symfony\Contracts\Translation\TranslatorInterface`
* The `Translator::transChoice()` has been deprecated in favor of using `Translator::trans()` with intl message format
* The `Translator::transChoice()` method has been deprecated in favor of using `Translator::trans()` with "%count%" as the parameter driving plurals
* The `MessageSelector`, `Interval` and `PluralizationRules` classes have been deprecated, use `IdentityTranslator` instead
* The `Translator::getFallbackLocales()` and `TranslationDataCollector::getFallbackLocales()` method have been marked as internal

TwigBundle
----------

* The `transchoice` tag and filter have been deprecated, use the `trans` ones instead with a `%count%` parameter.

Validator
---------

Expand Down
3 changes: 3 additions & 0 deletions UPGRADE-5.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ FrameworkBundle
set the "APP_ENV" environment variable instead.
* The `--no-debug` console option has been removed,
set the "APP_DEBUG" environment variable to "0" instead.
* The `Templating\Helper\TranslatorHelper::transChoice()` method has been removed, use the `trans()` one instead with a `%count%` parameter.

HttpFoundation
--------------
Expand Down Expand Up @@ -196,11 +197,13 @@ Translation
* The `TranslatorInterface` has been removed in favor of `Symfony\Contracts\Translation\TranslatorInterface`
* The `MessageSelector`, `Interval` and `PluralizationRules` classes have been removed, use `IdentityTranslator` instead
* The `Translator::getFallbackLocales()` and `TranslationDataCollector::getFallbackLocales()` method are now internal
* The `Translator::transChoice()` method has been removed in favor of using `Translator::trans()` with "%count%" as the parameter driving plurals

TwigBundle
----------

* The default value (`false`) of the `twig.strict_variables` configuration option has been changed to `%kernel.debug%`.
* The `transchoice` tag and filter have been removed, use the `trans` ones instead with a `%count%` parameter.

Validator
--------
Expand Down
5 changes: 3 additions & 2 deletions src/Symfony/Bridge/Twig/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ CHANGELOG
4.2.0
-----

* add bundle name suggestion on wrongly overridden templates paths
* added `name` argument in `debug:twig` command and changed `filter` argument as `--filter` option
* add bundle name suggestion on wrongly overridden templates paths
* added `name` argument in `debug:twig` command and changed `filter` argument as `--filter` option
* deprecated the `transchoice` tag and filter, use the `trans` ones instead with a `%count%` parameter

4.1.0
-----
Expand Down
37 changes: 27 additions & 10 deletions src/Symfony/Bridge/Twig/Extension/TranslationExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Symfony\Bridge\Twig\TokenParser\TransChoiceTokenParser;
use Symfony\Bridge\Twig\TokenParser\TransDefaultDomainTokenParser;
use Symfony\Bridge\Twig\TokenParser\TransTokenParser;
use Symfony\Component\Translation\LegacyTranslatorTrait;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Symfony\Contracts\Translation\TranslatorTrait;
Expand All @@ -29,6 +28,8 @@
* Provides integration of the Translation component with Twig.
*
* @author Fabien Potencier <[email protected]>
*
* @final since Symfony 4.2
*/
class TranslationExtension extends AbstractExtension
{
Expand All @@ -38,21 +39,28 @@ class TranslationExtension extends AbstractExtension
trans as private doTrans;
}

use LegacyTranslatorTrait {
transChoice as private doTransChoice;
}

private $translator;
private $translationNodeVisitor;

public function __construct(TranslatorInterface $translator = null, NodeVisitorInterface $translationNodeVisitor = null)
/**
* @param TranslatorInterface|null $translator
*/
public function __construct($translator = null, NodeVisitorInterface $translationNodeVisitor = null)
{
if (null !== $translator && !$translator instanceof LegacyTranslatorInterface && !$translator instanceof TranslatorInterface) {
throw new \TypeError(sprintf('Argument 1 passed to %s() must be an instance of %s, %s given.', __METHOD__, TranslatorInterface::class, \is_object($translator) ? \get_class($translator) : \gettype($translator)));
}
$this->translator = $translator;
$this->translationNodeVisitor = $translationNodeVisitor;
}

/**
* @deprecated since Symfony 4.2
*/
public function getTranslator()
{
@trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2.', __METHOD__), E_USER_DEPRECATED);

return $this->translator;
}

Expand All @@ -63,7 +71,7 @@ public function getFilters()
{
return array(
new TwigFilter('trans', array($this, 'trans')),
new TwigFilter('transchoice', array($this, 'transchoice')),
new TwigFilter('transchoice', array($this, 'transchoice'), array('deprecated' => '4.2', 'alternative' => 'trans" with parameter "%count%')),
);
}

Expand Down Expand Up @@ -101,19 +109,28 @@ public function getTranslationNodeVisitor()
return $this->translationNodeVisitor ?: $this->translationNodeVisitor = new TranslationNodeVisitor();
}

public function trans($message, array $arguments = array(), $domain = null, $locale = null)
public function trans($message, array $arguments = array(), $domain = null, $locale = null, $count = null)
{
if (null !== $count) {
$arguments['%count%'] = $count;
}
if (null === $this->translator) {
return $this->doTrans($message, $arguments, $domain, $locale);
}

return $this->translator->trans($message, $arguments, $domain, $locale);
}

/**
* @deprecated since Symfony 4.2, use the trans() method instead with a %count% parameter
*/
public function transchoice($message, $count, array $arguments = array(), $domain = null, $locale = null)
{
if (null === $this->translator || !$this->translator instanceof LegacyTranslatorInterface) {
return $this->doTransChoice($message, $count, array_merge(array('%count%' => $count), $arguments), $domain, $locale);
if (null === $this->translator) {
return $this->doTrans($message, array_merge(array('%count%' => $count), $arguments), $domain, $locale);
}
if ($this->translator instanceof TranslatorInterface) {
return $this->translator->trans($message, array_merge(array('%count%' => $count), $arguments), $domain, $locale);
}

return $this->translator->transChoice($message, $count, array_merge(array('%count%' => $count), $arguments), $domain, $locale);
Expand Down
19 changes: 11 additions & 8 deletions src/Symfony/Bridge/Twig/Node/TransNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,12 @@ public function compile(Compiler $compiler)
$method = !$this->hasNode('count') ? 'trans' : 'transChoice';

$compiler
->write('echo $this->env->getExtension(\'Symfony\Bridge\Twig\Extension\TranslationExtension\')->getTranslator()->'.$method.'(')
->write('echo $this->env->getExtension(\'Symfony\Bridge\Twig\Extension\TranslationExtension\')->trans(')
->subcompile($msg)
;

$compiler->raw(', ');

if ($this->hasNode('count')) {
$compiler
->subcompile($this->getNode('count'))
->raw(', ')
;
}

if (null !== $vars) {
$compiler
->raw('array_merge(')
Expand All @@ -98,7 +91,17 @@ public function compile(Compiler $compiler)
->raw(', ')
->subcompile($this->getNode('locale'))
;
} elseif ($this->hasNode('count')) {
$compiler->raw(', null');
}

if ($this->hasNode('count')) {
$compiler
->raw(', ')
->subcompile($this->getNode('count'))
;
}

$compiler->raw(");\n");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ public function testTrans($template, $expected, array $variables = array())
$this->assertEquals($expected, $this->getTemplate($template)->render($variables));
}

/**
* @group legacy
* @dataProvider getTransChoiceTests
*/
public function testTransChoice($template, $expected, array $variables = array())
{
$this->testTrans($template, $expected, $variables);
}

/**
* @expectedException \Twig\Error\SyntaxError
* @expectedExceptionMessage Unexpected token. Twig was looking for the "with", "from", or "into" keyword in "index" at line 3.
Expand All @@ -64,6 +73,7 @@ public function testTransComplexBody()
}

/**
* @group legacy
* @expectedException \Twig\Error\SyntaxError
* @expectedExceptionMessage A message inside a transchoice tag must be a simple text in "index" at line 2.
*/
Expand All @@ -87,6 +97,69 @@ public function getTransTests()

array('{% trans into "fr"%}Hello{% endtrans %}', 'Hello'),

// trans with count
array(
'{% trans from "messages" %}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples{% endtrans %}',
'There is no apples',
array('count' => 0),
),
array(
'{% trans %}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples{% endtrans %}',
'There is 5 apples',
array('count' => 5),
),
array(
'{% trans %}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples (%name%){% endtrans %}',
'There is 5 apples (Symfony)',
array('count' => 5, 'name' => 'Symfony'),
),
array(
'{% trans with { \'%name%\': \'Symfony\' } %}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples (%name%){% endtrans %}',
'There is 5 apples (Symfony)',
array('count' => 5),
),
array(
'{% trans into "fr"%}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples{% endtrans %}',
'There is no apples',
array('count' => 0),
),
array(
'{% trans count 5 into "fr"%}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples{% endtrans %}',
'There is 5 apples',
),

// trans filter
array('{{ "Hello"|trans }}', 'Hello'),
array('{{ name|trans }}', 'Symfony', array('name' => 'Symfony')),
array('{{ hello|trans({ \'%name%\': \'Symfony\' }) }}', 'Hello Symfony', array('hello' => 'Hello %name%')),
array('{% set vars = { \'%name%\': \'Symfony\' } %}{{ hello|trans(vars) }}', 'Hello Symfony', array('hello' => 'Hello %name%')),
array('{{ "Hello"|trans({}, "messages", "fr") }}', 'Hello'),

// trans filter with count
array('{{ "{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples"|trans(count=count) }}', 'There is 5 apples', array('count' => 5)),
array('{{ text|trans(count=5, arguments={\'%name%\': \'Symfony\'}) }}', 'There is 5 apples (Symfony)', array('text' => '{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples (%name%)')),
array('{{ "{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples"|trans({}, "messages", "fr", count) }}', 'There is 5 apples', array('count' => 5)),
);
}

/**
* @group legacy
*/
public function getTransChoiceTests()
{
return array(
// trans tag
array('{% trans %}Hello{% endtrans %}', 'Hello'),
array('{% trans %}%name%{% endtrans %}', 'Symfony', array('name' => 'Symfony')),

array('{% trans from elsewhere %}Hello{% endtrans %}', 'Hello'),

array('{% trans %}Hello %name%{% endtrans %}', 'Hello Symfony', array('name' => 'Symfony')),
array('{% trans with { \'%name%\': \'Symfony\' } %}Hello %name%{% endtrans %}', 'Hello Symfony'),
array('{% set vars = { \'%name%\': \'Symfony\' } %}{% trans with vars %}Hello %name%{% endtrans %}', 'Hello Symfony'),

array('{% trans into "fr"%}Hello{% endtrans %}', 'Hello'),

// transchoice
array(
'{% transchoice count from "messages" %}{0} There is no apples|{1} There is one apple|]1,Inf] There is %count% apples{% endtranschoice %}',
Expand Down Expand Up @@ -145,8 +218,8 @@ public function testDefaultTranslationDomain()
{%- trans from "custom" %}foo{% endtrans %}
{{- "foo"|trans }}
{{- "foo"|trans({}, "custom") }}
{{- "foo"|transchoice(1) }}
{{- "foo"|transchoice(1, {}, "custom") }}
{{- "foo"|trans(count=1) }}
{{- "foo"|trans({"%count%":1}, "custom") }}
{% endblock %}
',

Expand Down Expand Up @@ -174,12 +247,12 @@ public function testDefaultTranslationDomainWithNamedArguments()
{%- block content %}
{{- "foo"|trans(arguments = {}, domain = "custom") }}
{{- "foo"|transchoice(count = 1) }}
{{- "foo"|transchoice(count = 1, arguments = {}, domain = "custom") }}
{{- "foo"|trans(count = 1) }}
{{- "foo"|trans(count = 1, arguments = {}, domain = "custom") }}
{{- "foo"|trans({}, domain = "custom") }}
{{- "foo"|trans({}, "custom", locale = "fr") }}
{{- "foo"|transchoice(1, arguments = {}, domain = "custom") }}
{{- "foo"|transchoice(1, {}, "custom", locale = "fr") }}
{{- "foo"|trans(arguments = {"%count%":1}, domain = "custom") }}
{{- "foo"|trans({"%count%":1}, "custom", locale = "fr") }}
{% endblock %}
',

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Bridge/Twig/Tests/Node/TransNodeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function testCompileStrict()

$this->assertEquals(
sprintf(
'echo $this->env->getExtension(\'Symfony\Bridge\Twig\Extension\TranslationExtension\')->getTranslator()->trans("trans %%var%%", array_merge(array("%%var%%" => %s), %s), "messages");',
'echo $this->env->getExtension(\'Symfony\Bridge\Twig\Extension\TranslationExtension\')->trans("trans %%var%%", array_merge(array("%%var%%" => %s), %s), "messages");',
$this->getVariableGetterWithoutStrictCheck('var'),
$this->getVariableGetterWithStrictCheck('foo')
),
Expand Down
30 changes: 26 additions & 4 deletions src/Symfony/Bridge/Twig/Tests/Translation/TwigExtractorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,21 @@ public function testExtract($template, $messages)
}
}

/**
* @group legacy
* @dataProvider getLegacyExtractData
*/
public function testLegacyExtract($template, $messages)
{
$this->testExtract($template, $messages);
}

public function getExtractData()
{
return array(
array('{{ "new key" | trans() }}', array('new key' => 'messages')),
array('{{ "new key" | trans() | upper }}', array('new key' => 'messages')),
array('{{ "new key" | trans({}, "domain") }}', array('new key' => 'domain')),
array('{{ "new key" | transchoice(1) }}', array('new key' => 'messages')),
array('{{ "new key" | transchoice(1) | upper }}', array('new key' => 'messages')),
array('{{ "new key" | transchoice(1, {}, "domain") }}', array('new key' => 'domain')),
array('{% trans %}new key{% endtrans %}', array('new key' => 'messages')),
array('{% trans %} new key {% endtrans %}', array('new key' => 'messages')),
array('{% trans from "domain" %}new key{% endtrans %}', array('new key' => 'domain')),
Expand All @@ -67,11 +73,27 @@ public function getExtractData()

// make sure 'trans_default_domain' tag is supported
array('{% trans_default_domain "domain" %}{{ "new key"|trans }}', array('new key' => 'domain')),
array('{% trans_default_domain "domain" %}{{ "new key"|transchoice }}', array('new key' => 'domain')),
array('{% trans_default_domain "domain" %}{% trans %}new key{% endtrans %}', array('new key' => 'domain')),

// make sure this works with twig's named arguments
array('{{ "new key" | trans(domain="domain") }}', array('new key' => 'domain')),
);
}

/**
* @group legacy
*/
public function getLegacyExtractData()
{
return array(
array('{{ "new key" | transchoice(1) }}', array('new key' => 'messages')),
array('{{ "new key" | transchoice(1) | upper }}', array('new key' => 'messages')),
array('{{ "new key" | transchoice(1, {}, "domain") }}', array('new key' => 'domain')),

// make sure 'trans_default_domain' tag is supported
array('{% trans_default_domain "domain" %}{{ "new key"|transchoice }}', array('new key' => 'domain')),

// make sure this works with twig's named arguments
array('{{ "new key" | transchoice(domain="domain", count=1) }}', array('new key' => 'domain')),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
* Token Parser for the 'transchoice' tag.
*
* @author Fabien Potencier <[email protected]>
*
* @deprecated since Symfony 4.2, use the "trans" tag with a "%count%" parameter instead
*/
class TransChoiceTokenParser extends TransTokenParser
{
Expand All @@ -38,6 +40,8 @@ public function parse(Token $token)
$lineno = $token->getLine();
$stream = $this->parser->getStream();

@trigger_error(sprintf('The "transchoice" tag is deprecated since Symfony 4.2, use the "trans" one instead with a "%count%" parameter in %s line %d.', $stream->getSourceContext()->getName(), $lineno), E_USER_DEPRECATED);

$vars = new ArrayExpression(array(), $lineno);

$count = $this->parser->getExpressionParser()->parseExpression();
Expand Down
Loading

0 comments on commit dc5f3bf

Please sign in to comment.