Skip to content

[Translation] make intl+icu format seamless by handling it in MessageCatalogue #29220

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

Merged
merged 1 commit into from
Nov 15, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Translation\Catalogue;

use Symfony\Component\Translation\MessageCatalogueInterface;

/**
* Merge operation between two catalogues as follows:
* all = source ∪ target = {x: x ∈ source ∨ x ∈ target}
Expand All @@ -32,10 +34,11 @@ protected function processDomain($domain)
'new' => array(),
'obsolete' => array(),
);
$intlDomain = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX;

foreach ($this->source->all($domain) as $id => $message) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->source->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
Expand All @@ -45,7 +48,7 @@ protected function processDomain($domain)
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

namespace Symfony\Component\Translation\Catalogue;

use Symfony\Component\Translation\MessageCatalogueInterface;

/**
* Target operation between two catalogues:
* intersection = source ∩ target = {x: x ∈ source ∧ x ∈ target}
Expand All @@ -33,6 +35,7 @@ protected function processDomain($domain)
'new' => array(),
'obsolete' => array(),
);
$intlDomain = $domain.MessageCatalogueInterface::INTL_DOMAIN_SUFFIX;

// For 'all' messages, the code can't be simplified as ``$this->messages[$domain]['all'] = $target->all($domain);``,
// because doing so will drop messages like {x: x ∈ source ∧ x ∉ target.all ∧ x ∈ target.fallback}
Expand All @@ -46,7 +49,7 @@ protected function processDomain($domain)
foreach ($this->source->all($domain) as $id => $message) {
if ($this->target->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->source->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
Expand All @@ -59,7 +62,7 @@ protected function processDomain($domain)
if (!$this->source->has($id, $domain)) {
$this->messages[$domain]['all'][$id] = $message;
$this->messages[$domain]['new'][$id] = $message;
$this->result->add(array($id => $message), $domain);
$this->result->add(array($id => $message), $this->target->defines($id, $intlDomain) ? $intlDomain : $domain);
if (null !== $keyMetadata = $this->target->getMetadata($id, $domain)) {
$this->result->setMetadata($id, $keyMetadata, $domain);
}
Expand Down
21 changes: 20 additions & 1 deletion src/Symfony/Component/Translation/Dumper/FileDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,26 @@ public function dump(MessageCatalogue $messages, $options = array())
throw new RuntimeException(sprintf('Unable to create directory "%s".', $directory));
}
}
// save file

$intlDomain = $domain.MessageCatalogue::INTL_DOMAIN_SUFFIX;
$intlMessages = $messages->all($intlDomain);

if ($intlMessages) {
$intlPath = $options['path'].'/'.$this->getRelativePath($intlDomain, $messages->getLocale());
file_put_contents($intlPath, $this->formatCatalogue($messages, $intlDomain, $options));

$messages->replace(array(), $intlDomain);

try {
if ($messages->all($domain)) {
file_put_contents($fullpath, $this->formatCatalogue($messages, $domain, $options));
}
continue;
} finally {
$messages->replace($intlMessages, $intlDomain);
}
}

file_put_contents($fullpath, $this->formatCatalogue($messages, $domain, $options));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
*/
interface IntlFormatterInterface
{
const DOMAIN_SUFFIX = '+intl-icu';

/**
* Formats a localized message using rules defined by ICU MessageFormat.
*
Expand Down
44 changes: 37 additions & 7 deletions src/Symfony/Component/Translation/MessageCatalogue.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,41 @@ public function getLocale()
*/
public function getDomains()
{
return array_keys($this->messages);
$domains = array();
$suffixLength = \strlen(self::INTL_DOMAIN_SUFFIX);

foreach ($this->messages as $domain => $messages) {
if (\strlen($domain) > $suffixLength && false !== $i = strpos($domain, self::INTL_DOMAIN_SUFFIX, -$suffixLength)) {
$domain = substr($domain, 0, $i);
}
$domains[$domain] = $domain;
}

return array_values($domains);
}

/**
* {@inheritdoc}
*/
public function all($domain = null)
{
if (null === $domain) {
return $this->messages;
if (null !== $domain) {
return ($this->messages[$domain.self::INTL_DOMAIN_SUFFIX] ?? array()) + ($this->messages[$domain] ?? array());
}

$allMessages = array();
$suffixLength = \strlen(self::INTL_DOMAIN_SUFFIX);

foreach ($this->messages as $domain => $messages) {
if (\strlen($domain) > $suffixLength && false !== $i = strpos($domain, self::INTL_DOMAIN_SUFFIX, -$suffixLength)) {
$domain = substr($domain, 0, $i);
$allMessages[$domain] = $messages + ($allMessages[$domain] ?? array());
} else {
$allMessages[$domain] = ($allMessages[$domain] ?? array()) + $messages;
}
}

return isset($this->messages[$domain]) ? $this->messages[$domain] : array();
return $allMessages;
}

/**
Expand All @@ -77,7 +99,7 @@ public function set($id, $translation, $domain = 'messages')
*/
public function has($id, $domain = 'messages')
{
if (isset($this->messages[$domain][$id])) {
if (isset($this->messages[$domain][$id]) || isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id])) {
return true;
}

Expand All @@ -93,14 +115,18 @@ public function has($id, $domain = 'messages')
*/
public function defines($id, $domain = 'messages')
{
return isset($this->messages[$domain][$id]);
return isset($this->messages[$domain][$id]) || isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id]);
}

/**
* {@inheritdoc}
*/
public function get($id, $domain = 'messages')
{
if (isset($this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id])) {
return $this->messages[$domain.self::INTL_DOMAIN_SUFFIX][$id];
}

if (isset($this->messages[$domain][$id])) {
return $this->messages[$domain][$id];
}
Expand All @@ -117,7 +143,7 @@ public function get($id, $domain = 'messages')
*/
public function replace($messages, $domain = 'messages')
{
$this->messages[$domain] = array();
unset($this->messages[$domain], $this->messages[$domain.self::INTL_DOMAIN_SUFFIX]);

$this->add($messages, $domain);
}
Expand All @@ -144,6 +170,10 @@ public function addCatalogue(MessageCatalogueInterface $catalogue)
}

foreach ($catalogue->all() as $domain => $messages) {
if ($intlMessages = $catalogue->all($domain.self::INTL_DOMAIN_SUFFIX)) {
$this->add($intlMessages, $domain.self::INTL_DOMAIN_SUFFIX);
$messages = array_diff_key($messages, $intlMessages);
}
$this->add($messages, $domain);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
*/
interface MessageCatalogueInterface
{
const INTL_DOMAIN_SUFFIX = '+intl-icu';

/**
* Gets the catalogue locale.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ public function testGetResultFromSingleDomain()
);
}

public function testGetResultFromIntlDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a', 'b' => 'old_b'),
'messages+intl-icu' => array('d' => 'old_d', 'c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b'), 'messages+intl-icu' => array('d' => 'old_d'))),
new MessageCatalogue('en', array('messages+intl-icu' => array('a' => 'new_a', 'c' => 'new_c')))
)->getResult()
);
}

public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ public function testGetResultFromSingleDomain()
);
}

public function testGetResultFromIntlDomain()
{
$this->assertEquals(
new MessageCatalogue('en', array(
'messages' => array('a' => 'old_a'),
'messages+intl-icu' => array('c' => 'new_c'),
)),
$this->createOperation(
new MessageCatalogue('en', array('messages' => array('a' => 'old_a'), 'messages+intl-icu' => array('b' => 'old_b'))),
new MessageCatalogue('en', array('messages' => array('a' => 'new_a'), 'messages+intl-icu' => array('c' => 'new_c')))
)->getResult()
);
}

public function testGetResultWithMetadata()
{
$leftCatalogue = new MessageCatalogue('en', array('messages' => array('a' => 'old_a', 'b' => 'old_b')));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,30 @@ public function testDump()
@unlink($tempDir.'/messages.en.concrete');
}

public function testDumpIntl()
{
$tempDir = sys_get_temp_dir();

$catalogue = new MessageCatalogue('en');
$catalogue->add(array('foo' => 'bar'), 'd1');
$catalogue->add(array('bar' => 'foo'), 'd1+intl-icu');
$catalogue->add(array('bar' => 'foo'), 'd2+intl-icu');

$dumper = new ConcreteFileDumper();
@unlink($tempDir.'/d2.en.concrete');
$dumper->dump($catalogue, array('path' => $tempDir));

$this->assertStringEqualsFile($tempDir.'/d1.en.concrete', 'foo=bar');
@unlink($tempDir.'/d1.en.concrete');

$this->assertStringEqualsFile($tempDir.'/d1+intl-icu.en.concrete', 'bar=foo');
@unlink($tempDir.'/d1+intl-icu.en.concrete');

$this->assertFileNotExists($tempDir.'/d2.en.concrete');
$this->assertStringEqualsFile($tempDir.'/d2+intl-icu.en.concrete', 'bar=foo');
@unlink($tempDir.'/d2+intl-icu.en.concrete');
}

public function testDumpCreatesNestedDirectoriesAndFile()
{
$tempDir = sys_get_temp_dir();
Expand All @@ -56,7 +80,7 @@ class ConcreteFileDumper extends FileDumper
{
public function formatCatalogue(MessageCatalogue $messages, $domain, array $options = array())
{
return '';
return http_build_query($messages->all($domain), '', '&');
}

protected function getExtension()
Expand Down
35 changes: 28 additions & 7 deletions src/Symfony/Component/Translation/Tests/MessageCatalogueTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ public function testGetLocale()

public function testGetDomains()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array(), 'domain2' => array()));
$catalogue = new MessageCatalogue('en', array('domain1' => array(), 'domain2' => array(), 'domain2+intl-icu' => array(), 'domain3+intl-icu' => array()));

$this->assertEquals(array('domain1', 'domain2'), $catalogue->getDomains());
$this->assertEquals(array('domain1', 'domain2', 'domain3'), $catalogue->getDomains());
}

public function testAll()
Expand All @@ -37,24 +37,43 @@ public function testAll()
$this->assertEquals(array('foo' => 'foo'), $catalogue->all('domain1'));
$this->assertEquals(array(), $catalogue->all('domain88'));
$this->assertEquals($messages, $catalogue->all());

$messages = array('domain1+intl-icu' => array('foo' => 'bar')) + $messages + array(
'domain2+intl-icu' => array('bar' => 'foo'),
'domain3+intl-icu' => array('biz' => 'biz'),
);
$catalogue = new MessageCatalogue('en', $messages);

$this->assertEquals(array('foo' => 'bar'), $catalogue->all('domain1'));
$this->assertEquals(array('bar' => 'foo'), $catalogue->all('domain2'));
$this->assertEquals(array('biz' => 'biz'), $catalogue->all('domain3'));

$messages = array(
'domain1' => array('foo' => 'bar'),
'domain2' => array('bar' => 'foo'),
'domain3' => array('biz' => 'biz'),
);
$this->assertEquals($messages, $catalogue->all());
}

public function testHas()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2+intl-icu' => array('bar' => 'bar')));

$this->assertTrue($catalogue->has('foo', 'domain1'));
$this->assertTrue($catalogue->has('bar', 'domain2'));
$this->assertFalse($catalogue->has('bar', 'domain1'));
$this->assertFalse($catalogue->has('foo', 'domain88'));
}

public function testGetSet()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar'), 'domain2+intl-icu' => array('bar' => 'foo')));
$catalogue->set('foo1', 'foo1', 'domain1');

$this->assertEquals('foo', $catalogue->get('foo', 'domain1'));
$this->assertEquals('foo1', $catalogue->get('foo1', 'domain1'));
$this->assertEquals('foo', $catalogue->get('bar', 'domain2'));
}

public function testAdd()
Expand All @@ -75,7 +94,7 @@ public function testAdd()

public function testReplace()
{
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain1+intl-icu' => array('bar' => 'bar')));
$catalogue->replace($messages = array('foo1' => 'foo1'), 'domain1');

$this->assertEquals($messages, $catalogue->all('domain1'));
Expand All @@ -89,16 +108,18 @@ public function testAddCatalogue()
$r1 = $this->getMockBuilder('Symfony\Component\Config\Resource\ResourceInterface')->getMock();
$r1->expects($this->any())->method('__toString')->will($this->returnValue('r1'));

$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo'), 'domain2' => array('bar' => 'bar')));
$catalogue = new MessageCatalogue('en', array('domain1' => array('foo' => 'foo')));
$catalogue->addResource($r);

$catalogue1 = new MessageCatalogue('en', array('domain1' => array('foo1' => 'foo1')));
$catalogue1 = new MessageCatalogue('en', array('domain1' => array('foo1' => 'foo1'), 'domain2+intl-icu' => array('bar' => 'bar')));
$catalogue1->addResource($r1);

$catalogue->addCatalogue($catalogue1);

$this->assertEquals('foo', $catalogue->get('foo', 'domain1'));
$this->assertEquals('foo1', $catalogue->get('foo1', 'domain1'));
$this->assertEquals('bar', $catalogue->get('bar', 'domain2'));
$this->assertEquals('bar', $catalogue->get('bar', 'domain2+intl-icu'));

$this->assertEquals(array($r, $r1), $catalogue->getResources());
}
Expand Down
Loading