Simple CMS as Anwiki replacement

sitescripts/web/sources.py

 # coding: utf-8
 
 # This file is part of the Adblock Plus web scripts,
 # Copyright (C) 2006-2013 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 30 matching lines @@
     elif self.has_locale(default_locale, checked_page):
       if not self.has_locale(locale, checked_page):
         locale = default_locale
     else:
       print >>sys.stderr, "Warning: Link to %s cannot be resolved" % page
 
     if page == default_page:
       page = ""
 
     path = "/%s/%s" % (locale, page)
-    return locale, urlparse.urlunparse(list(parsed[0:2]) + [path] + list(parsed[3:]))

[Sebastian Noack, 2013/10/29 11:04:17]

If you just wrap path in to a tuple instead of a list, then there is no need to
coerce the other two tuples to a list:

parsed[0:2] + (path,) + parsed[3:]

+    return locale, urlparse.urlunparse(parsed[0:2] + (path,) + parsed[3:])
 
   def read_config(self):
     configdata = self.read_file("settings.ini")
     config = SafeConfigParser()
     config.readfp(StringIO(configdata))
     return config
 
   #
   # Page helpers
   #
@@ skipping 49 matching lines @@
   def has_static(self, filename):
     return self.has_file(self.static_filename(filename))
 
   def read_static(self, filename):
     return self.read_file(self.static_filename(filename), binary=True)
 
   #
   # Locale helpers
   #
 
-  @staticmethod
-  def locale_filename(locale, page):
-    return "locales/%s/%s.json" % (locale, page)

[Sebastian Noack, 2013/10/29 11:04:17]

Why don't you call localizable_file_filename() and just append .json here? That
way you don't have to duplicate logic:

@classmethod
def locale_filename(cls, *args):
  return cls.localizable_file_filename(*args) + '.json'

+  @classmethod
+  def locale_filename(cls, locale, page):
+    return cls.localizable_file_filename(locale, page + ".json")
 
   def list_locales(self):
     result = set()
     for filename in self.list_files("locales"):
       if "/" in filename:
         locale, path = filename.split("/", 1)
         result.add(locale)
     return result
 
   def has_locale(self, locale, page):
@@ skipping 39 matching lines @@
   def has_include(self, include, format):
     return self.has_file(self.include_filename(include, format))
 
   def read_include(self, include, format):
     return self.read_file(self.include_filename(include, format))
 
 class MercurialSource(Source):
   def __init__(self, repo):
     command = ["hg", "-R", repo, "archive", "-r", "default",
         "-t", "uzip", "-p", ".", "-"]
-    data, _ = subprocess.Popen(command, stdout=subprocess.PIPE).communicate()

[Sebastian Noack, 2013/10/29 11:04:17]

Have a look at subprocess.check_output(), which is a shortcut for
subprocess.Popen(..., stdout=subprocess.PIPE).communicate()[0]

+    data = subprocess.check_output(command)
     self._archive = zipfile.ZipFile(StringIO(data), mode="r")
 
     command = ["hg", "-R", repo, "id", "-n", "-r", "default"]
-    self.version, _ = subprocess.Popen(command, stdout=subprocess.PIPE).communicate()
-    self.version = self.version.strip()
+    self.version = subprocess.check_output(command).strip()
 
   def __enter__(self):
     return self
 
   def __exit__(self, type, value, traceback):
     self.close()
     return False
 
   def close(self):
     self._archive.close()
 
   def has_file(self, filename):
     try:
       self._archive.getinfo("./%s" % filename)
     except KeyError:
       return False
     return True
 
   def read_file(self, filename, binary=False):
     result = self._archive.read("./%s" % filename)
     if not binary:
       result = result.decode("utf-8")
     return result
 
   def list_files(self, subdir):
-    prefix = "./" + subdir + "/"

[Sebastian Noack, 2013/10/29 11:04:17]

You should use format strings when concatenating more than two strings.

+    prefix = "./%s/" % subdir
     for filename in self._archive.namelist():
       if filename.startswith(prefix):
         yield filename[len(prefix):]
 
 class FileSource(Source):
   def __init__(self, dir):
     self._dir = dir
 
   def __enter__(self):
     return self
@@ skipping 20 matching lines @@
     def do_list(dir, relpath):
       files = os.listdir(dir)
       for filename in files:
         path = os.path.join(dir, filename)
         if os.path.isfile(path):
           result.append(relpath + filename)
         elif os.path.isdir(path):
           do_list(path, relpath + filename + "/")
     do_list(self.get_path(subdir), "")
     return result