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, localedata, escapes):
+  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
+      # Store the default string if required, e.g. for the translations script
+      if self._params["record_default_strings"]:

[Sebastian Noack, 2015/07/08 13:03:20]

Is there any particular reason we call that function only for the default
locale? This shouldn't be necessary as we only generate the default locale when
we extract translations, right?

Also note that record_default_settings will also be available as variable in
jinja templates, if you add it to the _params dictionary. So maybe it would be
better and simpler to patch Converter.localize_string in cms.bin.translate
instead passing that callback around? Note that this would be in line with how
cms.bin.generate_static_files is implemented.

[Wladimir Palant, 2015/07/08 23:11:08]

That's assumptions, the kind that might not be true tomorrow.
Patch a method to memoize return value without any assumptions about parameters
is one thing, wrapping such a complex function something entirely different -
that would move many assumptions about its functionality into a completely
unrelated piece of code. If anything, we could set the callback as an attribute
on Converter *instances* from cms.utils.get_page_params - "inheriting" this
callback would introduce significant complexity however and doesn't seem to be
worth it.

[Sebastian Noack, 2015/07/09 21:26:55]

Or the other way around. As far as I see it this code shouldn't be in the
default locale code path and the name shouldn't imply it. This will make the
logic of that "param" less complicated to understand.
 
Fair enough.

[Wladimir Palant, 2015/07/10 21:24:04]

True. If we generalize the parameter name into something like
"localized_string_callback" then we can just as well record all strings and let
the callback choose which ones it needs. It should be called with |result| as
parameter rather than |default| of course.

[kzar, 2015/07/11 19:21:17]

Done.

+        self._params["record_default_strings"](page, name, default,
+                                               comment, fixed_strings)
     elif name in localedata:
       result = localedata[name].strip()
     else:
       result = default
       self.missing_translations += 1
     self.total_translations += 1
 
     # Insert fixed strings
     for i, fixed_string in enumerate(fixed_strings, 1):
       result = result.replace("{%d}" % i, fixed_string)
@@ skipping 25 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: We currently ignore the comment, it is only relevant when
-      # generating the master translation.
-      return self.localize_string(name, default, 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: We currently ignore the comment, it is only relevant when
-    # generating the master translation.
-    localedata = self._params["localedata"]
-    return jinja2.Markup(self.localize_string(name, default, 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):
     localedata = self._params["source"].read_locale(self._params["locale"], page)
     default = localedata[name]
-    return jinja2.Markup(self.localize_string(name, default, localedata, html_escapes))
+    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,
 }