Issue 2936 - Support syntax for duplicate translatable strings

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 115 matching lines @@
     params[key] = "".join(lines)
 
   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))
 
+    locale = self._params["locale"]
+    is_default_locale = locale == self._params["defaultlocale"]
+
+    # Handle duplicated strings
+    if not default:
+      try:
+        default = self._params["localedata"][name]

[Wladimir Palant, 2015/08/24 11:07:17]

This logic is wrong, the default value should never be localized - it's the
English string specified in the page source. As you have it now, placeholders
will be messed up.

[kzar, 2015/08/24 14:29:45]

Acknowledged.

+      except KeyError:
+        raise Exception("Text not yet defined for string %s on page %s" %
+                        (name, self._params["page"]))
+    elif is_default_locale:
+      self._params["localedata"][name] = default

[Wladimir Palant, 2015/08/24 11:07:17]

1) This should be using localedata parameter instead of
self._params["localedata"].

2) I don't think that messing up localization data is a good approach, e.g.
because this localization data will be reused by other files and lead to strange
errors. Also, it would be a mess with placeholders specified differently in the
default value and in localization files.

Why not add some property for string defaults we've seen already? IMHO the logic
here should be:

  if default:
    # Maybe warn if the same default value is specified multiple times?
    self._seen_default[(page, name)] = default
  else:
    default = self._seen_default[(page, name)]  # might throw

That's it, completely independent of which locale we are processing.

[kzar, 2015/08/24 14:29:46]

Done.

+
     # 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"]:
+    if is_default_locale:
       result = default
     elif name in localedata:
       result = localedata[name].strip()
     else:
       result = default
       self.missing_translations += 1
     self.total_translations += 1
 
-    # 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)
-
+    if default:

[Wladimir Palant, 2015/08/24 11:07:17]

Why add this check? The string is still there, regardless of whether a default
value has been specified.

[kzar, 2015/08/24 14:29:45]

Well as a consumer of this callback I would be surprised to receive multiple
callbacks for the same string. Also we would be missing the string comment after
the first callback.

[Wladimir Palant, 2015/08/24 15:00:21]

That's something one has to count on anyway.
True, we can save the comment as well.

+      # 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 17 matching lines @@
       result = re.sub(
         r"%s([^<>]*?)%s" % (re_escape("<%s>" % tag), re_escape("</%s>" % tag)),
         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()
-
+      if default:
+        default = to_html(default).strip()
       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"(?:(?:\[(.*?)\])?" # Optional comment
+        r"\s+"
+        r"((?:(?!{{).|" # Translatable text
+          r"{{(?:(?!}}).)*}}" # Nested translation
+        r")*?)"
+      r")?"
       r"}}",
       lookup_string,
       text,
       flags=re.S
     )
 
   def process_links(self, text):
     def process_link(match):
       pre, attr, url, post = match.groups()
       url = jinja2.Markup(url).unescape()
@@ skipping 190 matching lines @@
         stack.pop()
       stack[-1]["subitems"].append(item)
       stack.append(item)
     return structured
 
 converters = {
   "html": RawConverter,
   "md": MarkdownConverter,
   "tmpl": TemplateConverter,
 }