Issue 2625 - [cms] Crowdin synchronisation script

cms/converters.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2015 Eyeo GmbH
 #
 # Adblock Plus is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License version 3 as
 # published by the Free Software Foundation.
 #
 # Adblock Plus is distributed in the hope that it will be useful,
@@ skipping 107 matching lines @@
     self._key = key
     self._attribute_parser = AttributeParser(self.whitelist)
 
     # Read in any parameters specified at the beginning of the file
     lines = params[key].splitlines(True)
     while lines and re.search(r"^\s*[\w\-]+\s*=", lines[0]):
       name, value = lines.pop(0).split("=", 1)
       params[name.strip()] = value.strip()
     params[key] = "".join(lines)
 
-  def localize_string(self, name, default, comment, localedata, escapes):

[kzar, 2015/06/15 14:24:28]

We previously threw away string comments, now we need to keep track of them to
help our translators.

+  def localize_string(self, page, name, default, comment, localedata, escapes):
     def escape(s):
       return re.sub(r".",
         lambda match: escapes.get(match.group(0), match.group(0)),
         s, flags=re.S)
     def re_escape(s):
       return re.escape(escape(s))
 
     # Extract tag attributes from default string
     default, saved_attributes, fixed_strings = self._attribute_parser.parse(default, self._params["page"])
 
     # Get translation
     locale = self._params["locale"]
     if locale == self._params["defaultlocale"]:
       result = default
     elif name in localedata:
       result = localedata[name].strip()
     else:
       result = default
       self.missing_translations += 1
     self.total_translations += 1
 
-    # Keep a record of the default translations
-    if locale == self._params["defaultlocale"]:

[Wladimir Palant, 2015/06/29 19:05:38]

This case is already checked above (getting translation), just move your code up
there.

[kzar, 2015/07/02 12:33:12]

Done.

-      localedata[name] = result
-      # Append a note about fixed strings to the string comment for translators

[kzar, 2015/06/15 14:24:28]

Without this translators would have way to know what fixed strings like {1} or
{2} represented in the strings they are translating. 

-      if fixed_strings:
-        comment = comment + " " if comment else ""
-        self._params["localecomments"][name] = comment + (
-          "[" + ", ".join(["%d: %s" % (i, s) for i, s in

[Wladimir Palant, 2015/06/29 19:05:38]

", ".join("{%d}: %s" (i, s) for i, s in ...)

In other words: You don't need to create a temporary list here, a generator will
do. Also, we can specifically mention {1}, no need to expect the translators to
make the connection from numbers to placeholders.

[kzar, 2015/07/02 12:33:13]

Done.

-                           enumerate(fixed_strings, 1)]) + "]"
-        )

[Wladimir Palant, 2015/06/29 19:05:37]

And what about comments for strings without fixed parts?

[kzar, 2015/07/02 12:33:13]

Whoops, good point.
Done.

+    # Perform callback with the string if required, e.g. for the translations script
+    callback = self._params["localized_string_callback"]
+    if callback:
+      callback(page, locale, name, result, comment, fixed_strings)
+
 
     # Insert fixed strings
     for i, fixed_string in enumerate(fixed_strings, 1):
       result = result.replace("{%d}" % i, fixed_string)
 
     # Insert attributes
     result = escape(result)
     def stringify_attribute((name, value)):
       return '%s="%s"' % (
         escape(name),
@@ skipping 19 matching lines @@
         r"<%s>\1</%s>" % (tag, tag),
         result, flags=re.S
       )
     return result
 
   def insert_localized_strings(self, text, escapes, to_html=lambda s: s):
     def lookup_string(match):
       name, comment, default = match.groups()
       default = to_html(default).strip()
 
-      # Note: The comment, it is only relevant when uploading translations

[Wladimir Palant, 2015/06/29 19:05:38]

This is the wrong place for this comment, it is describing function
localize_string().

[kzar, 2015/07/02 12:33:13]

Done.

-      return self.localize_string(name, default, comment, self._params["localedata"], escapes)
+      return self.localize_string(self._params["page"], name, default,
+                                  comment, self._params["localedata"], escapes)
 
     return re.sub(
       r"{{\s*"
       r"([\w\-]+)" # String ID
       r"(?:\[(.*?)\])?" # Optional comment
       r"\s+"
       r"((?:(?!{{).|" # Translatable text
         r"{{(?:(?!}}).)*}}" # Nested translation
       r")*?)"
       r"}}",
@@ skipping 144 matching lines @@
     module = template.make_module(self._params)
     for key, value in module.__dict__.iteritems():
       if not key.startswith("_"):
         self._params[key] = value
 
     result = unicode(module)
     result = self.process_links(result)
     return result
 
   def translate(self, default, name, comment=None):
-    # Note: The comment, it is only relevant when uploading translations

[Wladimir Palant, 2015/06/29 19:05:38]

Same here, this comment should be moved into function localize_string().

[kzar, 2015/07/02 12:33:13]

Done.

-    localedata = self._params["localedata"]
-    return jinja2.Markup(self.localize_string(name, default, comment, localedata, html_escapes))
+    return jinja2.Markup(self.localize_string(
+      self._params["page"], name, default, comment,
+      self._params["localedata"], html_escapes
+    ))
 
   def get_string(self, name, page):
-    default = self._params["source"].read_locale(self._params["locale"], page)[name]

[kzar, 2015/06/15 14:24:28]

This change is required so that strings included with the get_strings global
function are stored with the parent page. That way the translators have proper
context for these strings and locale files for the "master" language do not have
to be listed and read manually when synching with Crowdin.

[Wladimir Palant, 2015/06/29 19:05:38]

This change also happens to be wrong. Each page in the web.adblockplus.org
repository calls get_string("installation", "menu") - our translations have it
defined in the file menu.json rather than duplicated everywhere.

So, as you noticed here, merely filling up params["localedata"] doesn't work -
sometimes pages use strings from multiple locale files. What we probably need is
some kind of callback:

  if "record_default" in self._params:
    self._params["record_default"](name, default, comment, fixed_strings)

This callback can be specified as an optional parameter to
cms.utils.get_page_params().

[kzar, 2015/07/02 12:33:13]

Done.

-    localedata = self._params["localedata"]
-    return jinja2.Markup(self.localize_string(name, default, "", localedata, html_escapes))
+    localedata = self._params["source"].read_locale(self._params["locale"], page)
+    default = localedata[name]
+    return jinja2.Markup(self.localize_string(
+      page, name, default, "", localedata, html_escapes
+    ))
 
   def get_page_content(self, page, locale=None):
     from cms.utils import get_page_params
 
     if locale is None:
       locale = self._params["locale"]
     return get_page_params(self._params["source"], locale, page)
 
   def linkify(self, page, locale=None, **attrs):
     if locale is None:
@@ skipping 24 matching lines @@
         stack.pop()
       stack[-1]["subitems"].append(item)
       stack.append(item)
     return structured
 
 converters = {
   "html": RawConverter,
   "md": MarkdownConverter,
   "tmpl": TemplateConverter,
 }