Issue 5477 - Import everything from imported locales

packagerChrome.py

Index: packagerChrome.py
diff --git a/packagerChrome.py b/packagerChrome.py
index 3e1a452a1a5b79fa7ff7316930ac911aab8162a6..42c7a1c68580a8513c2d22e31cf9206deb3ce951 100644
--- a/packagerChrome.py
+++ b/packagerChrome.py
@@ -190,7 +190,30 @@ def toJson(data):
     ).encode('utf-8') + '\n'
 
 
-def importGeckoLocales(params, files):
+def import_string_webext(data, key, source):
+    """Overwrites data[key] with source."""
+    data[key] = source
+
+
+def import_string_gecko(data, key, value):
+    """Only sets {'message': value} in data-dictionary, after stripping

[Vasily Kuznetsov, 2017/08/21 10:43:18]

Do you think it would be difficult to make this docstring (and the one above)
conform to PEP-257[1]? Namely, that the first line of a docstring should be an
imperative sentence (e.g. "Compute the answer.") and any follow up lines should
be separated from the first line by an empty space.

[1] https://www.python.org/dev/peps/pep-0257

[tlucas, 2017/08/21 11:33:01]

Do you mean full conformity, including params?
For the docstring above, from what i see it'd be sufficient to just rephrase it
to e.g. "Import source-dict into data", since it's already a 1-liner - or am i
missing something?

[Vasily Kuznetsov, 2017/08/21 11:53:15]

Yeah, just rephrase and reformat where appropriate. I don't think you need to
add the parameters unless you consider them useful and would like to add them,
PEP-257 doesn't require any of that.

[tlucas, 2017/08/21 12:28:50]

Done.

+    undesired gecko-style access keys.
+    """
+
+    # Remove access keys from possible gecko-style

[Wladimir Palant, 2017/08/21 10:57:21]

Nit: Gecko should be capitalized (here and in the docstring above).

[tlucas, 2017/08/21 11:33:00]

Acknowledged.

[tlucas, 2017/08/21 12:28:50]

Done.

+    # translations
+    match = re.search(r'^(.*?)\s*\(&.\)$', value)

[Wladimir Palant, 2017/08/21 10:57:21]

For reference, the regexp used by Firefox code is slightly different and
considers potential ellipsis as well:
https://hg.adblockplus.org/adblockplus/file/86f4e19aafd5/lib/utils.js#l430

Not that this is worth changing now, we are definitely not going to add more
imports from Gecko at this point. So if it wasn't relevant so far, it isn't
going to be in future either.

[tlucas, 2017/08/21 11:33:00]

Acknowledged.

+    if match:
+        value = match.group(1)
+    else:
+        index = value.find('&')
+        if index >= 0:
+            value = value[0:index] + value[index + 1:]
+
+    data[key] = {'message': value}
+
+
+def import_locales(params, files):
     import localeTools
 
     # FIXME: localeTools doesn't use real Chrome locales, it uses dash as
@@ -227,12 +250,21 @@ def importGeckoLocales(params, files):
             data = json.loads(files[targetFile].decode('utf-8'))
 
             try:
+                # The WebExtensions (.json) and Gecko format provide
+                # translations differently and/or provide additional
+                # information like e.g. "placeholders". We want to adhere to
+                # that and preserve the addtional info.
+
                 if sourceFile.endswith('.json'):
                     with io.open(sourceFile, 'r', encoding='utf-8') as handle:
-                        sourceData = {k: v['message'] for k, v in json.load(handle).iteritems()}
+                        sourceData = json.load(handle)
+

[Vasily Kuznetsov, 2017/08/21 10:43:18]

I don't feel very strong about it, but it seems that this empty line doesn't
improve readability (like, for example the lines that separate two blocks that
do different things and each consist of multiple lines). Maybe we don't need it
(as well as the line 265)?

[tlucas, 2017/08/21 11:33:01]

Acknowledged.

[tlucas, 2017/08/21 12:28:50]

Done.

+                    import_string = import_string_webext
                 else:
                     sourceData = localeTools.readFile(sourceFile)
 
+                    import_string = import_string_gecko
+
                 # Resolve wildcard imports
                 if keys == '*' or keys == '=*':
                     importList = sourceData.keys()
@@ -255,16 +287,7 @@ def importGeckoLocales(params, files):
                         if key in data:
                             print 'Warning: locale string %s defined multiple times' % key
 
-                        # Remove access keys
-                        value = sourceData[stringID]
-                        match = re.search(r'^(.*?)\s*\(&.\)$', value)
-                        if match:
-                            value = match.group(1)
-                        else:
-                            index = value.find('&')
-                            if index >= 0:
-                                value = value[0:index] + value[index + 1:]
-                        data[key] = {'message': value}
+                        import_string(data, key, sourceData[stringID])
             except Exception as e:
                 print 'Warning: error importing locale data from %s: %s' % (sourceFile, e)
 
@@ -380,7 +403,7 @@ def createBuild(baseDir, type='chrome', outFile=None, buildNum=None, releaseBuil
         )
 
     if metadata.has_section('import_locales'):
-        importGeckoLocales(params, files)
+        import_locales(params, files)
 
     files['manifest.json'] = createManifest(params, files)
     if type == 'chrome':