Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Implement better text domain merging support #4053

Closed
wants to merge 1 commit into from

3 participants

@DASPRiD
Collaborator

This PR addresses the quite inefficient merging support I implemented with my last PR. It now also ensures that only compatible text domains are merged.

@ThomasCantonnet ThomasCantonnet commented on the diff
library/Zend/I18n/Translator/Translator.php
((6 lines not shown))
- if ($translation === null || $translation['message'] === '') {
+ if ($translation === null || $translation === '') {

Isn't this equivalent to empty($translation) ?

@DASPRiD Collaborator
DASPRiD added a note

Nope:

$string = '0';
var_dump(empty($string));
bool(true)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Bittarman Bittarman closed this pull request from a commit
Ryan Mauger Merge branch 'hotfix/4053'
Close #4053
* hotfix/4053:
  Implement better text domain merging support
3a650b3
@Bittarman Bittarman closed this in 3a650b3
@Bittarman Bittarman referenced this pull request from a commit
Ryan Mauger Merge branch 'hotfix/4053' into develop
Close #4053

* hotfix/4053:
  Implement better text domain merging support
33da31d
@ghost Unknown referenced this pull request from a commit
Ryan Mauger Merge branch 'hotfix/4053'
Close #4053
* hotfix/4053:
  Implement better text domain merging support
a78ea48
@ghost Unknown referenced this pull request from a commit
Ryan Mauger Merge branch 'hotfix/4053' into develop
Close #4053

* hotfix/4053:
  Implement better text domain merging support
118766f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 16, 2013
  1. @DASPRiD
This page is out of date. Refresh to see the latest.
View
10 library/Zend/I18n/Translator/Plural/Rule.php
@@ -72,6 +72,16 @@ public function evaluate($number)
}
/**
+ * Get number of possible plural forms.
+ *
+ * @return integer
+ */
+ public function getNumPlurals()
+ {
+ return $this->numPlurals;
+ }
+
+ /**
* Evaluate a part of an ast.
*
* @param array $ast
View
28 library/Zend/I18n/Translator/TextDomain.php
@@ -10,6 +10,7 @@
namespace Zend\I18n\Translator;
use ArrayObject;
+use Zend\I18n\Exception;
use Zend\I18n\Translator\Plural\Rule as PluralRule;
/**
@@ -51,4 +52,31 @@ public function getPluralRule()
return $this->pluralRule;
}
+
+ /**
+ * Merge another text domain with the current one.
+ *
+ * The plural rule of both text domains must be compatible for a successful
+ * merge. We are only validating the number of plural forms though, as the
+ * same rule could be made up with different expression.
+ *
+ * @param TextDomain $textDomain
+ * @return TextDomain
+ * @throws Exception\RuntimeException
+ */
+ public function merge(TextDomain $textDomain)
+ {
+ if ($this->getPluralRule()->getNumPlurals() !== $textDomain->getPluralRule()->getNumPlurals()) {
+ throw new Exception\RuntimeException('Plural rule of merging text domain is not compatible with the current one');
+ }
+
+ $this->exchangeArray(
+ array_replace(
+ $this->getArrayCopy(),
+ $textDomain->getArrayCopy()
+ )
+ );
+
+ return $this;
+ }
}
View
70 library/Zend/I18n/Translator/Translator.php
@@ -347,9 +347,9 @@ public function translatePlural(
$locale = null
) {
$locale = $locale ?: $this->getLocale();
- $translation = $this->getTranslatedMessage($singular, $locale, $textDomain, true);
+ $translation = $this->getTranslatedMessage($singular, $locale, $textDomain);
- if ($translation === null || $translation['message'] === '') {
+ if ($translation === null || $translation === '') {

Isn't this equivalent to empty($translation) ?

@DASPRiD Collaborator
DASPRiD added a note

Nope:

$string = '0';
var_dump(empty($string));
bool(true)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if (null !== ($fallbackLocale = $this->getFallbackLocale())
&& $locale !== $fallbackLocale
) {
@@ -365,31 +365,31 @@ public function translatePlural(
return ($number == 1 ? $singular : $plural);
}
- $index = $translation['plural_rule']->evaluate($number);
+ $index = $this->messages[$textDomain][$locale]
+ ->getPluralRule()
+ ->evaluate($number);
- if (!isset($translation['message'][$index])) {
+ if (!isset($translation[$index])) {
throw new Exception\OutOfBoundsException(sprintf(
'Provided index %d does not exist in plural array', $index
));
}
- return $translation['message'][$index];
+ return $translation[$index];
}
/**
* Get a translated message.
*
- * @param string $message
- * @param string $locale
- * @param string $textDomain
- * @param boolean $returnPluralRule
+ * @param string $message
+ * @param string $locale
+ * @param string $textDomain
* @return string|null
*/
protected function getTranslatedMessage(
$message,
$locale = null,
- $textDomain = 'default',
- $returnPluralRule = false
+ $textDomain = 'default'
) {
if ($message === '') {
return '';
@@ -399,28 +399,8 @@ protected function getTranslatedMessage(
$this->loadMessages($textDomain, $locale);
}
- if (is_array($this->messages[$textDomain][$locale])) {
- foreach ($this->messages[$textDomain][$locale] as $textDomain) {
- if (isset($textDomain[$message])) {
- if ($returnPluralRule) {
- return array(
- 'message' => $textDomain[$message],
- 'plural_rule' => $textDomain->getPluralRule()
- );
- } else {
- return $textDomain[$message];
- }
- }
- }
- } elseif (isset($this->messages[$textDomain][$locale][$message])) {
- if ($returnPluralRule) {
- return array(
- 'message' => $this->messages[$textDomain][$locale][$message],
- 'plural_rule' => $this->messages[$textDomain][$locale]->getPluralRule()
- );
- } else {
- return $this->messages[$textDomain][$locale][$message];
- }
+ if (isset($this->messages[$textDomain][$locale][$message])) {
+ return $this->messages[$textDomain][$locale][$message];
}
return null;
@@ -559,13 +539,7 @@ protected function loadMessagesFromRemote($textDomain, $locale)
}
if (isset($this->messages[$textDomain][$locale])) {
- if (!is_array($this->messages[$textDomain][$locale])) {
- $this->messages[$textDomain][$locale] = array(
- $this->messages[$textDomain][$locale]
- );
- }
-
- $this->messages[$textDomain][$locale][] = $loader->load($locale, $textDomain);
+ $this->messages[$textDomain][$locale]->merge($loader->load($locale, $textDomain));
} else {
$this->messages[$textDomain][$locale] = $loader->load($locale, $textDomain);
}
@@ -601,13 +575,7 @@ protected function loadMessagesFromPatterns($textDomain, $locale)
}
if (isset($this->messages[$textDomain][$locale])) {
- if (!is_array($this->messages[$textDomain][$locale])) {
- $this->messages[$textDomain][$locale] = array(
- $this->messages[$textDomain][$locale]
- );
- }
-
- $this->messages[$textDomain][$locale][] = $loader->load($locale, $filename);
+ $this->messages[$textDomain][$locale]->merge($loader->load($locale, $filename));
} else {
$this->messages[$textDomain][$locale] = $loader->load($locale, $filename);
}
@@ -645,13 +613,7 @@ protected function loadMessagesFromFiles($textDomain, $locale)
}
if (isset($this->messages[$textDomain][$locale])) {
- if (!is_array($this->messages[$textDomain][$locale])) {
- $this->messages[$textDomain][$locale] = array(
- $this->messages[$textDomain][$locale]
- );
- }
-
- $this->messages[$textDomain][$locale][] = $loader->load($locale, $file['filename']);
+ $this->messages[$textDomain][$locale]->merge($loader->load($locale, $file['filename']));
} else {
$this->messages[$textDomain][$locale] = $loader->load($locale, $file['filename']);
}
View
6 tests/ZendTest/I18n/Translator/Plural/RuleTest.php
@@ -150,4 +150,10 @@ public function testCompleteRules($rule, $expectedValues)
$this->assertEquals((int) $expectedValues[$i], $rule->evaluate($i));
}
}
+
+ public function testGetNumPlurals()
+ {
+ $rule = Rule::fromString('nplurals=9; plural=n');
+ $this->assertEquals(9, $rule->getNumPlurals());
+ }
}
View
22 tests/ZendTest/I18n/Translator/TextDomainTest.php
@@ -43,4 +43,26 @@ public function testPluralRuleDefault()
$this->assertEquals(1, $domain->getPluralRule()->evaluate(1));
$this->assertEquals(0, $domain->getPluralRule()->evaluate(2));
}
+
+ public function testMerging()
+ {
+ $domainA = new TextDomain(array('foo' => 'bar', 'bar' => 'baz'));
+ $domainB = new TextDomain(array('baz' => 'bat', 'bar' => 'bat'));
+ $domainA->merge($domainB);
+
+ $this->assertEquals('bar', $domainA['foo']);
+ $this->assertEquals('bat', $domainA['bar']);
+ $this->assertEquals('bat', $domainA['baz']);
+ }
+
+ public function testMergingIncompatibleTextDomains()
+ {
+ $this->setExpectedException('Zend\I18n\Exception\RuntimeException', 'is not compatible');
+
+ $domainA = new TextDomain();
+ $domainB = new TextDomain();
+ $domainB->setPluralRule(PluralRule::fromString('nplurals=3; plural=n'));
+
+ $domainA->merge($domainB);
+ }
}
View
5 tests/ZendTest/I18n/Translator/TranslatorTest.php
@@ -97,9 +97,8 @@ public function testTranslationFromSeveralTranslationFiles()
$this->assertEquals('Nachricht 9', $translator->translate('Message 9')); //translation-more-de_DE.php
$this->assertEquals('Nachricht 10 - 0', $translator->translatePlural('Message 10', 'Message 10', 1)); //translation-de_DE.php
$this->assertEquals('Nachricht 10 - 1', $translator->translatePlural('Message 10', 'Message 10', 2)); //translation-de_DE.php
- $this->assertEquals('Nachricht 11 - 0', $translator->translatePlural('Message 11', 'Message 11', 0)); //translation-more-de_DE.php
- $this->assertEquals('Nachricht 11 - 1', $translator->translatePlural('Message 11', 'Message 11', 1)); //translation-more-de_DE.php
- $this->assertEquals('Nachricht 11 - 2', $translator->translatePlural('Message 11', 'Message 11', 2)); //translation-more-de_DE.php
+ $this->assertEquals('Nachricht 11 - 0', $translator->translatePlural('Message 11', 'Message 11', 1)); //translation-more-de_DE.php
+ $this->assertEquals('Nachricht 11 - 1', $translator->translatePlural('Message 11', 'Message 11', 2)); //translation-more-de_DE.php
}
public function testFactoryCreatesTranslatorWithCache()
View
22 tests/ZendTest/I18n/Translator/_files/testarray/translation-more-de_DE-incompatible.php
@@ -0,0 +1,22 @@
+<?php
+/**
+ * Zend Framework (http://framework.zend.com/)
+ *
+ * @link http://github.com/zendframework/zf2 for the canonical source repository
+ * @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
+ * @license http://framework.zend.com/license/new-bsd New BSD License
+ * @package Zend_I18n
+ */
+
+return array(
+ '' => array(
+ 'plural_forms' => 'nplurals=3; plural=(n==0 ? 0 : (n == 1 ? 1 : 2));'
+ ),
+ 'Message 2' => 'Nachricht 2',
+ 'Message 9' => 'Nachricht 9',
+ 'Message 11' => array(
+ 'Nachricht 11 - 0',
+ 'Nachricht 11 - 1',
+ 'Nachricht 11 - 2',
+ ),
+);
View
3  tests/ZendTest/I18n/Translator/_files/testarray/translation-more-de_DE.php
@@ -10,13 +10,12 @@
return array(
'' => array(
- 'plural_forms' => 'nplurals=3; plural=(n==0 ? 0 : (n == 1 ? 1 : 2));'
+ 'plural_forms' => 'nplurals=2; plural=n!=1;'
),
'Message 2' => 'Nachricht 2',
'Message 9' => 'Nachricht 9',
'Message 11' => array(
'Nachricht 11 - 0',
'Nachricht 11 - 1',
- 'Nachricht 11 - 2',
),
);
Something went wrong with that request. Please try again.