Issue 2625 - [cms] Crowdin synchronisation script

cms/converters.py

Index: cms/converters.py
diff --git a/cms/converters.py b/cms/converters.py
index 8f602f3f36149a3b70ac17f6054197a84756b056..b90fadacd89ed532d8727bf4757a1abbcccfe914 100644
--- a/cms/converters.py
+++ b/cms/converters.py
@@ -125,7 +125,7 @@ class Converter:
       params[name.strip()] = value.strip()
     params[key] = "".join(lines)
 
-  def localize_string(self, name, default, localedata, escapes):
+  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 escape(s):
       return re.sub(r".",
         lambda match: escapes.get(match.group(0), match.group(0)),
@@ -147,6 +147,17 @@ class Converter:
       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.

+
     # Insert fixed strings
     for i, fixed_string in enumerate(fixed_strings, 1):
       result = result.replace("{%d}" % i, fixed_string)
@@ -185,9 +196,8 @@ class Converter:
       name, comment, default = match.groups()
       default = to_html(default).strip()
 
-      # Note: We currently ignore the comment, it is only relevant when
-      # generating the master translation.
-      return self.localize_string(name, default, self._params["localedata"], escapes)
+      # 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 re.sub(
       r"{{\s*"
@@ -352,15 +362,14 @@ class TemplateConverter(Converter):
     return result
 
   def translate(self, default, name, comment=None):
-    # Note: We currently ignore the comment, it is only relevant when
-    # generating the master translation.
+    # 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, localedata, html_escapes))
+    return jinja2.Markup(self.localize_string(name, default, comment, localedata, html_escapes))
 
   def get_string(self, name, page):
-    localedata = self._params["source"].read_locale(self._params["locale"], page)
-    default = localedata[name]
-    return jinja2.Markup(self.localize_string(name, default, localedata, html_escapes))
+    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))
 
   def get_page_content(self, page, locale=None):
     from cms.utils import get_page_params