php-gettext / Gettext

PHP library to collect and manipulate gettext (.po, .mo, .php, .json, etc)
MIT License
687 stars 134 forks source link

Translations::mergeWith modifies source translations #152

Closed lxg closed 7 years ago

lxg commented 7 years ago

When merging two Translations objects using the mergeWith method, the source catalog is modified in some situations. This is unexpected and undesired behaviour in my opinion.

The problem is in line 164 of Merge.php:

The Translation object from the source catalog is added to the target catalog and is now referenced in the target catalog. But modifications of the target catalog will now also modify the source catalog.

This is undesired for example in situations where a source catalog is generated by an extractor and is used to be merged with several other catalogs (in my case: target catalogs for different languages).

The solution is quite simple. Instead of referencing the source translation, create a clone:

diff --git a/src/Merge.php b/src/Merge.php
--- a/src/Merge.php
+++ b/src/Merge.php
@@ -161,7 +161,7 @@ class Merge
             if (($existing = $to->find($entry))) {
                 $existing->mergeWith($entry);
             } elseif ($options & self::ADD) {
-                $to[] = $entry;
+                $to[] = $entry->getClone();
             }
         }
     }
oscarotero commented 7 years ago

Yes, it's reasonable. Do you like to work in a PR? Thanks

lxg commented 7 years ago

Sure, here we go: https://github.com/oscarotero/Gettext/pull/153

oscarotero commented 7 years ago

Merged. thank you