Issue 2109 - Allow for translation of app independent repositories

localeTools.py

 # coding: utf-8
 
 # 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 re, os, sys, codecs, json, urllib, urllib2
 from StringIO import StringIO
 from ConfigParser import SafeConfigParser
 from zipfile import ZipFile
@@ skipping 90 matching lines @@
         yield (key, self[key])
         done.add(key)
 
 def escapeEntity(value):
   return value.replace('&', '&amp;').replace('<', '&lt;').replace('>', '&gt;').replace('"', '&quot;')
 
 def unescapeEntity(value):
   return value.replace('&amp;', '&').replace('&lt;', '<').replace('&gt;', '>').replace('&quot;', '"')
 
 def mapLocale(type, locale):
-  if type == 'chrome' or type == 'ISO-15897':

[Sebastian Noack, 2016/02/12 16:33:58]

Nit: type in {'chrome', 'ISO-15897'}

But why do we need to support both in the first place?

[Wladimir Palant, 2016/02/12 17:13:04]

This logic is wrong - you cannot decide on the mapping based on both name_format
and target_platforms, you have to choose one.

In fact, what we care about here is name_format - not our target platform but
rather which format we chose for locale naming. Consequently, langMappingChrome
should be renamed into langMappingISO and langMappingGecko into langMappingBCP.

[kzar, 2016/02/12 17:28:15]

Done.

-    return langMappingChrome.get(locale, locale)
-  else:
-    return langMappingGecko.get(locale, locale)
+  mapping = langMappingChrome if type == 'ISO-15897' else langMappingGecko
+  return mapping.get(locale, locale)
 
 def parseDTDString(data, path):
   result = []
   currentComment = [None]
 
   parser = ParserCreate()
   parser.UseForeignDTD(True)
   parser.SetParamEntityParsing(XML_PARAM_ENTITY_PARSING_ALWAYS)
 
   def ExternalEntityRefHandler(context, base, systemId, publicId):
@@ skipping 142 matching lines @@
 
   # Delete description from translations
   for key, value in parsed.iteritems():
     if "description" in value:
       del value["description"]
 
   file = codecs.open(path, 'wb', encoding='utf-8')
   json.dump(parsed, file, ensure_ascii=False, sort_keys=True, indent=2, separators=(',', ': '))
   file.close()
 
-def setupTranslations(projectName, localeConfig, key):
+def setupTranslations(localeConfig, projectName, key):
   # Make a new set from the locales list, mapping to Crowdin friendly format
-  locales = set(map(lambda locale: mapLocale(localeConfig['name_format'],

[Sebastian Noack, 2016/02/12 16:33:58]

Why not using a set expression?

  {mapLocale(localeConfig['name_format'], locale) for locale in
localeConfig['locales']}

[kzar, 2016/02/12 17:28:15]

Good point, Done.

-                                             locale),
-                    localeConfig['locales'].keys()))
+  locales = {mapLocale(localeConfig['name_format'], locale)
+             for locale in localeConfig['locales']}
 
   # Fill up with locales that we don't have but the browser supports
   if 'chrome' in localeConfig['target_platforms']:
     for locale in chromeLocales:
-      locales.add(mapLocale('chrome', locale))

[Wladimir Palant, 2016/02/12 17:13:04]

mapLocale('ISO-15897', ...) please, that's the format we have here.

[kzar, 2016/02/12 18:25:26]

Yep, already changed those in second patch set. Same below.

+      locales.add(mapLocale('ISO-15897', locale))
 
   if 'gecko' in localeConfig['target_platforms']:
     firefoxLocales = urllib2.urlopen('http://www.mozilla.org/en-US/firefox/all.html').read()
     for match in re.finditer(r'&lang=([\w\-]+)"', firefoxLocales):
-      locales.add(mapLocale('gecko', match.group(1)))

[Wladimir Palant, 2016/02/12 17:13:04]

mapLocale('BCP-47', ...) please, that's the format we get here.

+      locales.add(mapLocale('BCP-47', match.group(1)))
     langPacks = urllib2.urlopen('https://addons.mozilla.org/en-US/firefox/language-tools/').read()
     for match in re.finditer(r'<tr>.*?</tr>', langPacks, re.S):
       if match.group(0).find('Install Language Pack') >= 0:
         match2 = re.search(r'lang="([\w\-]+)"', match.group(0))
         if match2:
-          locales.add(mapLocale('gecko', match2.group(1)))

[Wladimir Palant, 2016/02/12 17:13:04]

mapLocale('BCP-47', ...) please, that's the format we get here.

+          locales.add(mapLocale('BCP-47', match2.group(1)))
 
   allowed = set()
   allowedLocales = urllib2.urlopen('http://crowdin.net/page/language-codes').read()
   for match in re.finditer(r'<tr>\s*<td\b[^<>]*>([\w\-]+)</td>', allowedLocales, re.S):
     allowed.add(match.group(1))
   if not allowed.issuperset(locales):
     print 'Warning, following locales aren\'t allowed by server: ' + ', '.join(locales - allowed)
 
   locales = list(locales & allowed)
   locales.sort()
@@ skipping 14 matching lines @@
   body += '--%s--\r\n' % boundary
 
   body = body.encode('utf-8')
   request = urllib2.Request(url, StringIO(body))
   request.add_header('Content-Type', 'multipart/form-data; boundary=%s' % boundary)
   request.add_header('Content-Length', len(body))
   result = urllib2.urlopen(request).read()
   if result.find('<success') < 0:
     raise Exception('Server indicated that the operation was not successful\n' + result)
 
-def updateTranslationMaster(metadata, dir, projectName, localeConfig, key):
+def updateTranslationMaster(localeConfig, metadata, dir, projectName, key):
   result = json.load(urllib2.urlopen('http://api.crowdin.net/api/project/%s/info?key=%s&json=1' % (projectName, key)))
 
   existing = set(map(lambda f: f['name'], result['files']))
   add = []
   update = []
   for file in os.listdir(dir):
     path = os.path.join(dir, file)
     if os.path.isfile(path):
-      if localeConfig['file_format'] == 'chrome-json':
-        if file.endswith('.json'):
-          data = preprocessChromeLocale(path, metadata, True)
-          newName = file
-        else:
-          fileHandle = codecs.open(path, 'rb', encoding='utf-8')
-          data = json.dumps({file: {'message': fileHandle.read()}})
-          fileHandle.close()
-          newName = file + '.json'
+      if localeConfig['file_format'] == 'chrome-json' and file.endswith('.json'):
+        data = preprocessChromeLocale(path, metadata, True)
+        newName = file
+      elif localeConfig['file_format'] == 'chrome-json':
+        fileHandle = codecs.open(path, 'rb', encoding='utf-8')
+        data = json.dumps({file: {'message': fileHandle.read()}})
+        fileHandle.close()
+        newName = file + '.json'
       else:
         data = toJSON(path)
         newName = file + '.json'
 
       if data:
         if newName in existing:
           update.append((newName, data))
           existing.remove(newName)
         else:
           add.append((newName, data))
 
   if len(add):
     titles = urllib.urlencode([('titles[%s]' % name, re.sub(r'\.json', '', name)) for name, data in add])
     postFiles(add, 'http://api.crowdin.net/api/project/%s/add-file?key=%s&type=chrome&%s' % (projectName, key, titles))
   if len(update):
     postFiles(update, 'http://api.crowdin.net/api/project/%s/update-file?key=%s' % (projectName, key))
   for file in existing:
     result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/delete-file?key=%s&file=%s' % (projectName, key, file)).read()
     if result.find('<success') < 0:
       raise Exception('Server indicated that the operation was not successful\n' + result)
 
-def uploadTranslations(metadata, dir, locale, projectName, localeConfig, key):
+def uploadTranslations(localeConfig, metadata, dir, locale, projectName, key):
   files = []
   for file in os.listdir(dir):
     path = os.path.join(dir, file)
     if os.path.isfile(path):
-      if localeConfig['file_format'] == 'chrome-json':
-        if file.endswith('.json'):
-          data = preprocessChromeLocale(path, metadata, False)
-          newName = file
-        else:
-          fileHandle = codecs.open(path, 'rb', encoding='utf-8')

[Sebastian Noack, 2016/02/12 16:33:58]

Using the "with" statement?

[kzar, 2016/02/12 17:28:15]

I really want to avoid unrelated changes this time. Infact I realise to that end
refactoring the conditional logic here (and below) was a mistake. I've put it
back.

[Sebastian Noack, 2016/02/12 17:38:17]

Well, apparently you change this code anyway. Not too important though.

-          data = json.dumps({file: {'message': fileHandle.read()}})
-          fileHandle.close()
-          newName = file + '.json'
+      if localeConfig['file_format'] == 'chrome-json' and file.endswith('.json'):
+        data = preprocessChromeLocale(path, metadata, False)
+        newName = file
+      elif localeConfig['file_format'] == 'chrome-json':
+        fileHandle = codecs.open(path, 'rb', encoding='utf-8')
+        data = json.dumps({file: {'message': fileHandle.read()}})
+        fileHandle.close()
+        newName = file + '.json'
       else:
         data = toJSON(path)
         newName = file + '.json'
 
       if data:
         files.append((newName, data))
   if len(files):
     postFiles(files, 'http://api.crowdin.net/api/project/%s/upload-translation?key=%s&language=%s' % (
       projectName, key, mapLocale(localeConfig['name_format'], locale))
     )
 
 def getTranslations(localeConfig, projectName, key):
   result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/export?key=%s' % (projectName, key)).read()
   if result.find('<success') < 0:
     raise Exception('Server indicated that the operation was not successful\n' + result)
 
   result = urllib2.urlopen('http://api.crowdin.net/api/project/%s/download/all.zip?key=%s' % (projectName, key)).read()
   zip = ZipFile(StringIO(result))
   dirs = {}
+
+  normalizedDefaultLocale = localeConfig['default_locale']
+  if localeConfig['name_format'] == 'ISO-15897':
+    normalizedDefaultLocale = normalizedDefaultLocale.replace('_', '-')
+  normalizedDefaultLocale = mapLocale(localeConfig['name_format'],
+                                      normalizedDefaultLocale)
+
   for info in zip.infolist():
     if not info.filename.endswith('.json'):
       continue
 
     dir, file = os.path.split(info.filename)
-    if not re.match(r'^[\w\-]+$', dir) or dir == localeConfig['default_locale']:
+    if not re.match(r'^[\w\-]+$', dir) or dir == normalizedDefaultLocale:
       continue
     if localeConfig['file_format'] == 'chrome-json' and file.count('.') == 1:
       origFile = file
     else:
       origFile = re.sub(r'\.json$', '', file)
     if (localeConfig['file_format'] == 'gecko-dtd' and
         not origFile.endswith('.dtd') and
         not origFile.endswith('.properties')):
       continue
 
@@ skipping 12 matching lines @@
     if data == '[]':
       continue
 
     if not dir in dirs:
       dirs[dir] = set()
     dirs[dir].add(origFile)
 
     path = os.path.join(localeConfig['base_path'], dir, origFile)
     if not os.path.exists(os.path.dirname(path)):
       os.makedirs(os.path.dirname(path))
-    if localeConfig['file_format'] == 'chrome-json':
-      if origFile.endswith('.json'):
-        postprocessChromeLocale(path, data)
-      else:
-        data = json.loads(data)
-        if origFile in data:
-          fileHandle = codecs.open(path, 'wb', encoding='utf-8')

[Sebastian Noack, 2016/02/12 16:33:58]

As discussed before, please use io.open instead codecs.open. The API is more
safe/explicit when it comes to unicode handling. And codecs.open() is currently
been discussed to be deprecated.

[kzar, 2016/02/12 17:28:15]

As above, want to avoid unrelated changes and this code shouldn't have been even
modified in the first place.

[Sebastian Noack, 2016/02/12 17:38:17]

Also here, as you change that code anyway, it's good time to tidy it up. But I
guess it's not too important.

-          fileHandle.write(data[origFile]['message'])
-          fileHandle.close()
+    if localeConfig['file_format'] == 'chrome-json' and file.endswith('.json'):
+      postprocessChromeLocale(path, data)
+    elif localeConfig['file_format'] == 'chrome-json':
+      data = json.loads(data)
+      if origFile in data:
+        fileHandle = codecs.open(path, 'wb', encoding='utf-8')
+        fileHandle.write(data[origFile]['message'])
+        fileHandle.close()
     else:
       fromJSON(path, data)
 
   # Remove any extra files
   for dir, files in dirs.iteritems():
     baseDir = os.path.join(localeConfig['base_path'], dir)
     if not os.path.exists(baseDir):
       continue
     for file in os.listdir(baseDir):
       path = os.path.join(baseDir, file)
       if os.path.isfile(path) and (file.endswith('.json') or file.endswith('.properties') or file.endswith('.dtd')) and not file in files:
         os.remove(path)