Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Fix inability to translate to languages which don't have plural forms #6827

Closed
wants to merge 6 commits into from

Conversation

hissy
Copy link

@hissy hissy commented Nov 1, 2014

Some languages (e.g. Japanese, Chinese, etc.) does not have plural forms. With these languages, getTranslatedMessage function returns string value instead of array, even in translatePlural function. However, translatePlural function can not treat a string value. This issue is critical for us.

@hissy
Copy link
Author

hissy commented Nov 2, 2014

@malukenho You are right. Fixed it.

@DASPRiD
Copy link
Member

DASPRiD commented Nov 2, 2014

Please add a unit test.

@hissy
Copy link
Author

hissy commented Nov 2, 2014

I've added a test for this issue. Please check it.

@sasezaki
Copy link
Contributor

sasezaki commented Nov 2, 2014

@hissy
I think there is better way on this unit-test.
・separate test* function like testPluralsShouldHandleString when adding any fix.
translation-ja_JP.php is not good naming, eg plurals_not_array.php is better.

@DASPRiD
Copy link
Member

DASPRiD commented Nov 2, 2014

Can you create a new unit test please instead of adding to existing ones? That makes it much clearer what is actually tested.

@@ -401,6 +401,8 @@ public function translatePlural(
}

return ($number == 1 ? $singular : $plural);
} elseif (is_string($translation)) {
$translation = (array) $translation;
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, This (array) cast can be into array($translation)?, to be clear for fallback.

@hissy
Copy link
Author

hissy commented Nov 2, 2014

Thank you for your review. I've separated my test from testTranslatePlurals.

IMHO, This (array) cast can be into array($translation)?, to be clear for fallback.

Should I change this? @DASPRiD

@DASPRiD
Copy link
Member

DASPRiD commented Nov 2, 2014

@hissy Makes sense, yes.

@hissy
Copy link
Author

hissy commented Nov 2, 2014

OK, fixed it.

mlocati added a commit to mlocati/zendframework2-i18n-v2 that referenced this pull request Nov 3, 2014
@weierophinney weierophinney added this to the 2.4.0 milestone Feb 19, 2015
weierophinney added a commit that referenced this pull request Feb 24, 2015
Fix inability to translate to languages which don't have plural forms
weierophinney added a commit that referenced this pull request Feb 24, 2015
@weierophinney
Copy link
Member

Merged to master for release with 2.4.

@hissy hissy deleted the no-plural-form-lang branch February 25, 2015 00:46
@hissy
Copy link
Author

hissy commented Feb 25, 2015

Thanks! 😄

weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
…ral-form-lang

Fix inability to translate to languages which don't have plural forms
weierophinney added a commit to zendframework/zend-i18n that referenced this pull request May 15, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants