Simple CMS as Anwiki replacement

See https://hg.adblockplus.org/wwwnew/file/ for the source data this is intended to work on. Currently it can process pages specified in Markdown (slightly extended to allow specifying page-specific parameters) or Jinja2 formats. Unfortunately, Markdown source isn`t very readable with the localizable strings removed. In addition, this will also process static files, both of the localized and unlocalized variety. What`s currently missing is code reuse (includes) which is required to support some of the more complicated pages, especially the home page.

There are two scripts here: generate_static_pages.py converts an entire repository into static files (intended to be run on the server regularly), test_server.py provides a web server for testing that operates on a directory - you can edit the files and see your changes apply immediately.

[Wladimir Palant, 2013-10-24 11:42:20]

Various improvements

[Wladimir Palant, 2013-10-24 11:49:59]

A bunch of improvements in the new revision:

* Implemented "raw" format (mostly unprocessed HTML source) to ease migration
from Anwiki.
* Somewhat unified the processing for various formats, e.g. specifying
parameters at the beginning of the file is now possible for all formats.
* Refer to other sitescripts modules via relative paths.
* Charset conversion happens when files are read/written, not at random points
in the program flow.
* Links to stylesheets, scripts and images get a repository revision appended to
them, to ensure that old versions don't get stuck in cache.
* Test server will now restart automatically if code is changed.
* Moved default locale/page setting into settings.ini (was hardcoded).

[Wladimir Palant, 2013-10-24 21:32:01]

Completed functionality

[Wladimir Palant, 2013-10-24 21:41:52]

As far as I am concerned, all the necessary functionality is there now (minus
our Anwiki extensions pulling in external data but these should be easy to
reimplement as Jinja filters). Changes in this revision:

* Slightly improved the test server to act more like the real one (redirect to
default locale/page if nothing is specified).
* Added a syntax to include other files from a page.
* Added configurable "locale overrides": /en/firefox, /en/chrome and /en/opera
all share the same locale, this has to be considered in a bunch of places.
* Removed an explicit parameter to stop ToC generation, this can be solved
entirely in templates.

[Wladimir Palant, 2013-10-24 21:44:35]

Note that everything is documented under
https://hg.adblockplus.org/wwwnew/file/tip/README.md now. The repository also
contains more examples now.

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

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
File sitescripts/web/bin/generate_static_pages.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:42: pass
I would check if we got EEXIST here and only then fail silently and otherwise
re-raise the error. Otherwise for example when you don't have permissions to
create the directory, the code below will fail because of the directory doesn't
exist, which is misleading.

try:
  os.makedirs(os.path.dirname(outfile))
except OSError, e:
  if e.errno != errno.EEXIST:
    raise

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:55: pagedata =
re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
When you inject text into regular expressions always use re.escape().

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:79: os.rmdir(path)
You might want to remove directories recursively. So that when a/b/c.html is
removed and there aren't any other files within a will be removed too, instead
of removing only a/b.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
File sitescripts/web/bin/test_server.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
sitescripts/web/bin/test_server.py:34: if not "/" in path:
In Python there is a "not in" operator, that should be used instead of not + in
operator:

if "/" not in path:

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
sitescripts/web/bin/test_server.py:55: mime = "text/js; charset=utf-8"
This isn't the mime type for javascript, "application/javascript" would be the
correct mime type.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/conver...
File sitescripts/web/converters.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/conver...
sitescripts/web/converters.py:67: while len(links):
len() isn't needed here. Lists evaluate to True while non empty. So you can just
use "while links:"

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/conver...
sitescripts/web/converters.py:94: text = re.sub(r"(<a
[^<>]*\b(href)=\")([^<>\"]+)(\")", process_link, text)
I would use \s instead of a whitespace, after the tag name.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/conver...
sitescripts/web/converters.py:189: for match in re.finditer(r'<h(\d)
[^<>]*\bid="([^<>"]+)"[^<>]*>(.*?)</h\1>', content, re.S):
I would use \s instead of a whitespace, after the tag name.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/source...
File sitescripts/web/sources.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/source...
sitescripts/web/sources.py:51: return locale,
urlparse.urlunparse(list(parsed[0:2]) + [path] + list(parsed[3:]))
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:]

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/source...
sitescripts/web/sources.py:123: return "locales/%s/%s.json" % (locale, page)
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'

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/source...
sitescripts/web/sources.py:183: data, _ = subprocess.Popen(command,
stdout=subprocess.PIPE).communicate()
Have a look at subprocess.check_output(), which is a shortcut for
subprocess.Popen(..., stdout=subprocess.PIPE).communicate()[0]

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/source...
sitescripts/web/sources.py:214: prefix = "./" + subdir + "/"
You should use format strings when concatenating more than two strings.

[Wladimir Palant, 2013-11-04 09:49:21]

All comments should be addressed unless noted otherwise. A few additional
changes I've implemented:

* Added some caching to make sure generate_static_pages doesn't hit the disk too
often.
* Localized strings can now contain <strong> and <em> tag in addition to <a>.
* translate filter in Jinja2 templates can insert links as well now.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
File sitescripts/web/bin/generate_static_pages.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:55: pagedata =
re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
On 2013/10/29 11:04:17, sebastian wrote:
> When you inject text into regular expressions always use re.escape().

I'm not injecting into a regular expression here, it's the replacement term.
Using re.escape() can be fatal here, consider the following:

    re.sub("(a)", r"\1%s" % re.escape("$"), "abcd")

Given that source.version is known to be alphanumerical we can skip escaping
here. I've fixed this in a some other places however.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:79: os.rmdir(path)
We are already removing directories recursively. remove_unknown("foo/a/b") will
remove foo/a/b/c.html and then return to remove_unknown("foo/a") that will
remove foo/a/b as an empty directory. It will then return to
remove_unknown("foo") which will remove foo/a. The top-level directory is never
removed which is intended.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
File sitescripts/web/bin/test_server.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
sitescripts/web/bin/test_server.py:55: mime = "text/js; charset=utf-8"
On 2013/10/29 11:04:17, sebastian wrote:
> This isn't the mime type for javascript, "application/javascript" would be the
> correct mime type.

Actually, application/x-javascript.

[Sebastian Noack, 2013-11-04 13:28:18]

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
File sitescripts/web/bin/generate_static_pages.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:55: pagedata =
re.sub(r'(<script\s[^<>]*\bsrc="/[^"<>]+)', r"\1?%s" % source.version, pagedata)
On 2013/11/04 09:49:21, Wladimir Palant wrote:
> On 2013/10/29 11:04:17, sebastian wrote:
> > When you inject text into regular expressions always use re.escape().
> 
> I'm not injecting into a regular expression here, it's the replacement term.
> Using re.escape() can be fatal here, consider the following:
> 
>     re.sub("(a)", r"\1%s" % re.escape("$"), "abcd")
> 
> Given that source.version is known to be alphanumerical we can skip escaping
> here. I've fixed this in a some other places however.

Yes, you are right.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/ge...
sitescripts/web/bin/generate_static_pages.py:79: os.rmdir(path)
On 2013/11/04 09:49:21, Wladimir Palant wrote:
> We are already removing directories recursively. remove_unknown("foo/a/b")
will
> remove foo/a/b/c.html and then return to remove_unknown("foo/a") that will
> remove foo/a/b as an empty directory. It will then return to
> remove_unknown("foo") which will remove foo/a. The top-level directory is
never
> removed which is intended.

You are right.

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
File sitescripts/web/bin/test_server.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
sitescripts/web/bin/test_server.py:55: mime = "text/js; charset=utf-8"
On 2013/11/04 09:49:21, Wladimir Palant wrote:
> On 2013/10/29 11:04:17, sebastian wrote:
> > This isn't the mime type for javascript, "application/javascript" would be
the
> > correct mime type.
> 
> Actually, application/x-javascript.

See http://tools.ietf.org/html/rfc4329#section-7.2

[Wladimir Palant, 2013-11-04 16:11:04]

Fixed MIME type

[Wladimir Palant, 2013-11-04 16:11:35]

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
File sitescripts/web/bin/test_server.py (right):

http://codereview.adblockplus.org/17817001/diff/140001/sitescripts/web/bin/te...
sitescripts/web/bin/test_server.py:55: mime = "text/js; charset=utf-8"
On 2013/11/04 13:28:18, sebastian wrote:
> See http://tools.ietf.org/html/rfc4329#section-7.2

Interesting, wasn't aware of it. Apparently, nginx also changed their default a
few months ago (http://hg.nginx.org/nginx/diff/ae3fd1ca62e0/conf/mime.types), we
should make sure this is reflected in our server config.

[Sebastian Noack, 2013-11-05 11:36:31]

LGTM