NEW Merge i18nTextCollector with existing (fixes #1838)

This is a necessity for any further 3.1 pushes of master files to getlocalization.
Because we'd otherwise remove existing master strings for CTF etc,
which means we can no longer backport new translations to 3.0
(and there's no way for users to contribute translations to 3.0 via getlocalization).

It's still a very monolithic class, but at least I've refactored it to return
all collected strings without writing it to files (for easier testing).

i18n/i18nTextCollector.php

@@ -71,10 +71,21 @@ class i18nTextCollector extends Object {
 	 * @uses DataObject->collectI18nStatics()
 	 * 
 	 * @param array $restrictToModules
+	 * @param array $mergeWithExisting Merge new master strings with existing ones
+	 * already defined in language files, rather than replacing them. This can be useful
+	 * for long-term maintenance of translations across releases, because it allows
+	 * "translation backports" to older releases without removing strings these older releases
+	 * still rely on.
 	 */	
-	public function run($restrictToModules = null) {
-		//Debug::message("Collecting text...", false);
-		
+	public function run($restrictToModules = null, $mergeWithExisting = false) {
+		$entitiesByModule = $this->collect($restrictToModules, $mergeWithExisting);
+		// Write each module language file
+		if($entitiesByModule) foreach($entitiesByModule as $module => $entities) {
+			$this->getWriter()->write($entities, $this->defaultLocale, $this->baseSavePath . '/' . $module);
+		}
+	}
+
+	public function collect($restrictToModules = null, $mergeWithExisting = false) {
 		$modules = scandir($this->basePath);
 		$themeFolders = array();
 		
@@ -137,7 +148,31 @@ class i18nTextCollector extends Object {
 					$entitiesByModule[$othermodule][$fullName] = $spec;
 					unset($entitiesByModule[$module][$fullName]);
 				}
-			}			
+			}		
+
+			// Optionally merge with existing master strings
+			// TODO Support all defined source formats through i18n::get_translators().
+			//      Currently not possible because adapter instances can't be fully reset through the Zend API,
+			//      meaning master strings accumulate across modules
+			if($mergeWithExisting) {
+				$adapter = Injector::inst()->get('i18nRailsYamlAdapter');
+				$masterFile = "{$this->basePath}/{$module}/lang/" 
+					. $adapter->getFilenameForLocale($this->defaultLocale);
+				if(!file_exists($masterFile)) continue;
+
+				$adapter->addTranslation(array(
+					'content' => $masterFile,
+					'locale' => $this->defaultLocale
+				));
+				$entitiesByModule[$module] = array_merge(
+					array_map(
+						// Transform each master string from scalar value to array of strings
+						function($v) {return array($v);},
+						$adapter->getMessages($this->defaultLocale)
+					),
+					$entitiesByModule[$module]
+				);	
+			}
 		}
 
 		// Restrict modules we update to just the specified ones (if any passed)
@@ -147,10 +182,11 @@ class i18nTextCollector extends Object {
 			}
 		}
 
-		// Write each module language file
-		if($entitiesByModule) foreach($entitiesByModule as $module => $entities) {
-			$this->getWriter()->write($entities, $this->defaultLocale, $this->baseSavePath . '/' . $module);
-		}
+		return $entitiesByModule;
+	}
+
+	public function write($module, $entities) {
+		$this->getWriter()->write($entities, $this->defaultLocale, $this->baseSavePath . '/' . $module);
 	}
 	
 	/**

tasks/i18nTextCollectorTask.php

@@ -10,6 +10,12 @@ class i18nTextCollectorTask extends BuildTask {
 	protected $description = "
 		Traverses through files in order to collect the 'entity master tables'
 		stored in each module.
+
+		Parameters:
+		- locale: Sets default locale
+		- writer: Custom writer class (defaults to i18nTextCollector_Writer_RailsYaml)
+		- module: One or more modules to limit collection (comma-separated)
+		- merge: Merge new strings with existing ones already defined in language files (default: FALSE)
 	";
 	
 	public function init() {
@@ -32,6 +38,6 @@ class i18nTextCollectorTask extends BuildTask {
 		$writer = $request->getVar('writer');
 		if($writer) $c->setWriter(new $writer());
 		$restrictModules = ($request->getVar('module')) ? explode(',', $request->getVar('module')) : null;
-		return $c->run($restrictModules);
+		return $c->run($restrictModules, (bool)$request->getVar('merge'));
 	}
 }

tests/i18n/_fakewebroot/i18ntestmodule/templates/i18nTestModule.ss

@@ -1,3 +1,4 @@
 <% _t('i18nTestModule.MAINTEMPLATE',"Main Template") %>
 $Layout
-lonely _t() call that should be ignored
+lonely _t() call that should be ignored
+<% _t('i18nTestModule.NEWENTITY',"Not stored in master file yet") %>

tests/i18n/i18nTextCollectorTest.php

@@ -538,6 +538,30 @@ YAML;
 		
 		Config::inst()->update('SSViewer', 'theme', $theme);
 	}
+
+	public function testCollectMergesWithExisting() {
+		$defaultlocal = i18n::default_locale();
+		$local = i18n::get_locale();
+		i18n::set_locale('en_US'); 
+		i18n::set_default_locale('en_US');
+
+		$c = new i18nTextCollector();
+		$c->setWriter(new i18nTextCollector_Writer_Php());
+		$c->basePath = $this->alternateBasePath;
+		$c->baseSavePath = $this->alternateBaseSavePath;
+		
+		$entitiesByModule = $c->collect(null, true /* merge */);
+		$this->assertArrayHasKey(
+			'i18nTestModule.ENTITY',
+			$entitiesByModule['i18ntestmodule'],
+			'Retains existing entities'
+		);
+		$this->assertArrayHasKey(
+			'i18nTestModule.NEWENTITY',
+			$entitiesByModule['i18ntestmodule'],
+			'Adds new entities'
+		);
+	}
 	
 	public function testCollectFromFilesystemAndWriteMasterTables() {
 		$defaultlocal = i18n::default_locale();