Issue 2340 - Don`t generate pages if less than 30% have been translated

cms/bin/generate_static_pages.py

Index: cms/bin/generate_static_pages.py
===================================================================
--- a/cms/bin/generate_static_pages.py
+++ b/cms/bin/generate_static_pages.py
@@ -15,28 +15,33 @@
 # You should have received a copy of the GNU General Public License
 # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
 
 import sys
 import os
 import re
 import errno
 import codecs
+import ConfigParser
+import functools
 import logging
 
-from cms.utils import process_page
+from cms.utils import get_page_params, process_page
 from cms.sources import MercurialSource
 
+MIN_TRANSLATED = 0.3
+
 def memoize(func):
   memoized = {}
   def wrapper(*args):
     try:
       return memoized[args]
     except KeyError:
       return memoized.setdefault(args, func(*args))
+  wrapper.original = func

[Sebastian Noack, 2015/05/06 16:15:30]

I'm not sure if I like that hack. Note that it wouldn't be neccessarry if you
would pass the dict object used for caching as an argument:

  cached_links = {}
  source.resolve_link = memoize(source.resolve_link, cached_links)

  ...

  cached_links.clear()

[Wladimir Palant, 2015/05/06 17:21:59]

True, I don't like passing in a dictionary either however. Given that it can
only really be used to clear the cache, I can just expose that functionality.

[Sebastian Noack, 2015/05/06 19:12:15]

Well, allowing to pass in a dict-like object, also helps other use cases, like
implementing different kind of caches (like and LRU cache or using an external
cache backend). Or to share a cache for different functions. It's actually quite
common practice to take a cache argument when implementing a memoize() function,
django for example does that.

It also has the advantage that you might not mistakenly call .clear_cache() on a
function that wasn't actually decorated with memoize(). In fact it took my a
while to figure out where the "original" argument came from.

Also note that it's common practice, at least for decorators that are supposed
to work with any callable object, to use functools.update_wrapper() to copy
attribute from the decorated function to wrapper. That's not necessary here, but
if this decorator becomes more widely used, we might want to do that. And adding
additional attributes can cause conflicts then.

However, I'm not feeling too strong about it. So LGTM if you disagree.

[Wladimir Palant, 2015/05/06 19:20:35]

Thank you. Indeed, I do feel that the current approach is better than just
manipulating the dictionary used by memoize() and potentially breaking
assumptions that this decorator makes.

   return wrapper
 
 def generate_pages(repo, output_dir):
   known_files = set()
 
   def write_file(path_parts, contents, binary=False):
     encoding = None if binary else "utf-8"
     outfile = os.path.join(output_dir, *path_parts)
@@ -68,27 +73,51 @@ def generate_pages(repo, output_dir):
     source.read_locale = memoize(source.read_locale)
     source.read_include = memoize(source.read_include)
 
     config = source.read_config()
     defaultlocale = config.get("general", "defaultlocale")
     locales = list(source.list_locales())
     if defaultlocale not in locales:
       locales.append(defaultlocale)
+
+    # First pass: compile the list of pages with given translation level
+    pagelist = set()
     for page, format in source.list_pages():
       for locale in locales:
-        if locale == defaultlocale or source.has_locale(locale, page):
-          pagedata = process_page(source, locale, page, format)
+        if locale == defaultlocale:
+          pagelist.add((locale, page))
+        elif source.has_locale(locale, page):
+          params = get_page_params(source, locale, page, format)
+          if params["translation_ratio"] >= MIN_TRANSLATED:
+            pagelist.add((locale, page))
 
-          # Make sure links to static files are versioned
-          pagedata = re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
-          pagedata = re.sub(r'(<link\s[^<>]*\bhref="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
-          pagedata = re.sub(r'(<img\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
+    # Override existance check to avoid linking to pages we don't generate
+    def has_locale(orig_function, locale, page):
+      try:
+        page = config.get("locale_overrides", page)
+      except ConfigParser.Error:
+        pass
+      if (defaultlocale, page) in pagelist:
+        return locale != defaultlocale and (locale, page) in pagelist
+      else:

[Sebastian Noack, 2015/05/06 16:15:30]

Nit: Redundant else statement. (Yes, I know you like them)

[Wladimir Palant, 2015/05/06 17:21:59]

Done and made the logic here slightly more obvious.

+        return orig_function(locale, page)
+    source.has_locale = functools.partial(has_locale, source.has_locale)

[Sebastian Noack, 2015/05/06 16:15:30]

It took me a while to understand what you are doing here. Would be more clear
IMO, if you just use a closure variable:

  orig_has_locale = source.has_locale
  def has_locale(locale, page):
    ..
    return orig_has_locale(locale, page)
  source.has_locale = has_locale

[Wladimir Palant, 2015/05/06 17:21:59]

I thought you were a fan of clever solutions :)

+    source.resolve_link = memoize(source.resolve_link.original)
 
-          write_file([locale] + page.split("/"), pagedata)
+    # Second pass: actually generate pages this time
+    for locale, page in pagelist:
+      pagedata = process_page(source, locale, page)
+
+      # Make sure links to static files are versioned
+      pagedata = re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
+      pagedata = re.sub(r'(<link\s[^<>]*\bhref="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
+      pagedata = re.sub(r'(<img\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
+
+      write_file([locale] + page.split("/"), pagedata)
 
     for filename in source.list_localizable_files():
       for locale in locales:
         if source.has_localizable_file(locale, filename):
           filedata = source.read_localizable_file(locale, filename)
           write_file([locale] + filename.split("/"), filedata, binary=True)
 
     for filename in source.list_static():