Issue 5477 - Import everything from imported locales

packagerChrome.py

 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import errno
 import io
 import json
 import os
 import re
 from StringIO import StringIO
@@ skipping 172 matching lines @@
         ).encode('utf-8')
 
 
 def toJson(data):
     return json.dumps(
         data, ensure_ascii=False, sort_keys=True,
         indent=2, separators=(',', ': ')
     ).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
     # separator instead.
     convert_locale_code = lambda code: code.replace('-', '_')
 
     # We need to map Chrome locales to Gecko locales. Start by mapping Chrome
     # locales to themselves, merely with the dash as separator.
     locale_mapping = {convert_locale_code(l): l for l in localeTools.chromeLocales}
 
@@ skipping 16 matching lines @@
             fileName, keys = item
             parts = map(lambda n: source if n == '*' else n, fileName.split('/'))
             sourceFile = os.path.join(os.path.dirname(item.source), *parts)
             incompleteMarker = os.path.join(os.path.dirname(sourceFile), '.incomplete')
             if not os.path.exists(sourceFile) or os.path.exists(incompleteMarker):
                 continue
 
             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()
                     importList = filter(lambda k: not k.startswith('_'), importList)
                     if keys == '=*':
                         importList = map(lambda k: '=' + k, importList)
                     keys = ' '.join(importList)
 
                 for stringID in keys.split():
                     noMangling = False
                     if stringID.startswith('='):
                         stringID = stringID[1:]
                         noMangling = True
 
                     if stringID in sourceData:
                         if noMangling:
                             key = re.sub(r'\W', '_', stringID)
                         else:
                             key = re.sub(r'\..*', '', parts[-1]) + '_' + re.sub(r'\W', '_', stringID)
                         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)
 
             files[targetFile] = toJson(data)
 
 
 def truncate(text, length_limit):
     if len(text) <= length_limit:
         return text
     return text[:length_limit - 1].rstrip() + u'\u2026'
@@ skipping 95 matching lines @@
     if metadata.has_section('convert_js'):
         convertJS(params, files)
 
     if metadata.has_section('preprocess'):
         files.preprocess(
             [f for f, _ in metadata.items('preprocess')],
             {'needsExt': True}
         )
 
     if metadata.has_section('import_locales'):
-        importGeckoLocales(params, files)
+        import_locales(params, files)
 
     files['manifest.json'] = createManifest(params, files)
     if type == 'chrome':
         fixTranslationsForCWS(files)
 
     if devenv:
         import buildtools
         import random
         files.read(os.path.join(buildtools.__path__[0], 'chromeDevenvPoller__.js'), relpath='devenvPoller__.js')
         files['devenvVersion__'] = str(random.random())
 
         if metadata.has_option('general', 'testScripts'):
             files['qunit/index.html'] = createScriptPage(
                 params, 'testIndex.html.tmpl', ('general', 'testScripts')
             )
 
     zipdata = files.zipToString()
     signature = None
     pubkey = None
     if keyFile != None:
         signature = signBinary(zipdata, keyFile)
         pubkey = getPublicKey(keyFile)
     writePackage(outFile, pubkey, signature, zipdata)