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

fallback to sourcecode if pluralform exists but is not translated #110

Closed
wants to merge 3 commits into from
Closed

fallback to sourcecode if pluralform exists but is not translated #110

wants to merge 3 commits into from

Conversation

Blaimi
Copy link
Contributor

@Blaimi Blaimi commented Mar 22, 2019

If a po-File has the msgid but no translations, translatePlural() translates to an empty string.

This change makes it consistent with the behaviour of translate() which also returns the source-code version if the message is not translated.

example:

msgid "Total (%1$s dataset)"
msgid_plural "Total (%1$s datasets)"
msgstr[0] ""
msgstr[1] ""

@Blaimi
Copy link
Contributor Author

Blaimi commented Mar 22, 2019

UnitTest is missing

@@ -417,7 +417,11 @@ public function translatePlural(
);
}

return $translation[$index];
if ($translation[$index] !== '') {

Choose a reason for hiding this comment

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

Maybe better to use empty()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

translate() checks also against the empty string – but check against null is missing here 😱

Choose a reason for hiding this comment

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

empty() will check empty string and null

@michalbundyra
Copy link
Member

@Blaimi

UnitTest is missing

Are you willing to add it? It can't be merged without tests. Adding it shouldn't be complicated. Thanks!

@Blaimi
Copy link
Contributor Author

Blaimi commented Mar 22, 2019

Are you willing to add it? It can't be merged without tests. Adding it shouldn't be complicated. Thanks!
Jep, that's a good task for one of our Junior-Developers ;)

@froschdesign froschdesign added this to the 2.10.0 milestone Sep 19, 2019
weierophinney added a commit that referenced this pull request Nov 18, 2019
@weierophinney
Copy link
Member

Thanks, @Blaimi; merged to develop for release with 2.10.0.

@weierophinney
Copy link
Member

I rebased off of develop and resolved conflicts when merging.

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