11
\$\begingroup\$

I was hoping I might get some of the brains in Stack Overflow to take a look at my Python static blogger application. I've been using it a few years in a pretty hacked up form. Lately I decided to clean it up and put it up on Github. I'd love some smarter Python programmers to give me advice and wisdom into ways to improve, optimize, and simplify the code.

The program is here: https://github.com/mshea/Pueblo

Some philosophies:

  1. I don't want more features. I want it as simple as it can be.

  2. I always prefer native modules. My ISP doesn't let me install new modules so the Markdown one is the only one I use outside of the defaults.

  3. I'd like to keep it to a single script unless splitting it up makes things much easier or simple.

  4. I'm particularly interested in any potential security concerns. Right now I don't see any.

  5. I'm not big into flexibility. I'd prefer it to do it one way really well rather than many ways poorly. If people want a flexible blogging platform, go with WordPress.

#!/usr/local/bin/python
#
# Pueblo: Python Markdown Static Blogger    
#
# 17 December 2011
#
# A single Python script to build a simple blog from a directory full of markdown files.
#
# This script requires the Markdown python implementation available at:
# http://pypi.python.org/pypi/Markdown/2.1.0
#
# This script requires markdown files using the following multimarkdown metadata as the first three lines
# of the processed .txt markdown files as follows:
#
# Title: the Title of your Document
# Author: Joe Blow
# Date: 15 December 2011
#
# The program will generate an index.html homepage file, an archive.html archive file, 
# and an index.xml RSS file.
#
# Header and footer data can be edited in the variables throughout the program. 
#
# This script expects the following additional files:
# style.css: The main site's stylesheet.
# iphone.css: The mobile version of the site's stylesheet.
# sidebar.html: A secondary set of data usually displayed as a sidebar.
#
# Instructions
# Install the Markdown python module.
# Configure this script by changing the configuration variables below.
# Put your static markdown .txt files in the configured directory
# Run the script either manually, with a regular cronjob, or as a CGI script.
# View the output at index.html

config = {
    "directory": ".", # No trailing slash.
    "site_url": "http://yoursite.net/", # Must have a trailing slash.
    "site_title": "Your Website",
    "site_description": "Your blog tagline.",
    "google_analytics_tag": "UA-111111-1",
    "author_name": "Your Name",
    "author_bio_link": "about.html",
    "amazon_tag": "mikesheanet-20",
    "twitter_tag": "twitterid",
    "author_email": "your@emailaddress.com",
    "header_image_url": "",
    "header_image_width": "",
    "header_image_height": "",
    "sidebar_on_article_pages": False,
    "minify_html": False,
}

nonentryfiles = []

# Main Program
import glob, re, rfc822, time, cgi, datetime, markdown
from time import gmtime, strftime, localtime, strptime
def rebuildsite ():
    textfiles = glob.glob(config["directory"]+"//*.txt")
    for nonfile in nonentryfiles:
        textfiles.remove(config["directory"]+"/"+nonfile)
    indexdata = []

    # Rip through the stack of .txt markdown files and build HTML pages from it.
    for eachfile in textfiles:
        eachfile = eachfile.replace(config["directory"]+"\\", "")
        content = open(eachfile).read()
        lines = re.split("\n", content)
        title = re.sub("(Title: )|(  )", "", lines[0])
        title = cgi.escape(title)
        urltitle = title.replace("&", "%26")
        author = lines[1].replace("Author: ","")
        date = re.sub("(  )|(\n)|(Date: )","",lines[2])
        numdate = strftime("%Y-%m-%d", strptime(date, "%d %B %Y"))
        content = markdown.markdown(re.sub("(Title:.*\n)|(Author:.*\n)|(Date:.*\n\n)|    ", "", content))
        summary = re.sub("<[^<]+?>","", content)
        summary = summary.replace("\n", " ")[0:200]
        htmlfilenamefull = htmlfilename = eachfile.replace(".txt", ".html")
        htmlfilename = htmlfilename.replace(config["directory"]+"/", "")
        postname = htmlfilename.replace(".html", "")
        # Build the HTML file, add a bit of footer text.
        htmlcontent = [buildhtmlheader("article", title, date)]
        htmlcontent.append(content)
        htmlcontent.append(buildhtmlfooter("article", urltitle))
        htmlfile = open(htmlfilenamefull, "w")
        htmlfile.write(minify("".join(htmlcontent)))
        htmlfile.close()
        if numdate <= datetime.datetime.now().strftime("%Y-%m-%d"):
            indexdata.append([[numdate],[title],[summary],[htmlfilename],[content]])

    # The following section builds index.html, archive.html and index.xml.  
    indexdata.sort()
    indexdata.reverse()
    indexbody=archivebody=rssbody=""
    count=0

    for indexrow in indexdata:
        dateobject = strptime(indexrow[0][0], "%Y-%m-%d")
        rssdate = strftime("%a, %d %b %Y 06:%M:%S +0000", dateobject)
        nicedate = strftime("%d %B %Y", dateobject)
        articleitem = '''
<h2><a href="%(article_link)s">%(article_title)s</a></h2>
<p>%(date)s - %(summary)s...</p>
'''     % {
        'article_link': indexrow[3][0],
        'article_title': indexrow[1][0],
        'date': nicedate,
        'summary': indexrow[2][0],
        }

        rssitem = '''
<item>
<title>%(title)s</title>
<link>%(link)s</link>
<guid>%(link)s</guid>
<pubDate>%(pubdate)s</pubDate>
<description>%(description)s</description>
<content:encoded>
<![CDATA[%(cdata)s]]>
</content:encoded>
</item>
'''     % {
        'title': indexrow[1][0],
        'link': config["site_url"]+indexrow[3][0],
        'pubdate': rssdate,
        'description': indexrow[2][0],
        'cdata': indexrow[4][0],
        }

        count = count + 1
        if count < 15:
            rssbody = rssbody + rssitem
        if count < 30:
            indexbody = indexbody+articleitem
        archivebody = archivebody + articleitem
    sidebardata = open(config["directory"]+"/sidebar.html").read()
    rssdatenow = rfc822.formatdate()

    indexdata = [buildhtmlheader("index", config["site_title"], "none")]
    indexdata.append(indexbody)
    indexdata.append("<h2><a href=\"archive.html\">View All %(article_count)s Articles</a></h2>\n</div>\n" 
        % { 'article_count': str(count) })
    indexdata.append(buildhtmlfooter("index", ""))
    indexfile = open(config["directory"]+"/index.html", "w").write(minify("".join(indexdata)))

    archivedata = [buildhtmlheader("archive", config["site_title"]+" Article Archive", "none")]
    archivedata.append(archivebody)
    archivedata.append("\n</div>\n")
    archivedata.append(buildhtmlfooter("archive", ""))
    archivefile = open (config["directory"]+"/archive.html", "w").write(minify("".join(archivedata)))

    rsscontent = '''<?xml version="1.0" encoding="UTF-8"?>
<rss version="2.0"
xmlns:atom="http://www.w3.org/2005/Atom"
xmlns:content="http://purl.org/rss/1.0/modules/content/"
xmlns:dc="http://purl.org/dc/elements/1.1/\"
>

<channel>
<title>%(site_title)s</title>
<link>%(site_url)s</link>
<description>%(site_description)s</description>
<pubDate>%(rssdatenow)s</pubDate>
<language>en</language>
<atom:link href="%(site_url)sindex.xml" rel="self" type="application/rss+xml" />
%(rssbody)s
</channel>
</rss>
''' % {
    'site_url': config["site_url"],
    'site_title': config["site_title"],
    'site_description': config["site_description"],
    'rssdatenow': rssdatenow,
    'rssbody': rssbody,
    }

    rssfile = open(config["directory"]+"/index.xml", "w").write(minify(rsscontent))

# Subroutine to build out the page's HTML header
def buildhtmlheader(type, title, date):
    if config["header_image_url"] != "":
        headerimage = '''
<img class="headerimg" src="%(header_image_url)s" alt="%(site_title)s: %(site_description)s" height="%(header_image_height)s" width="%(header_image_width)s" />
'''     % {
        'header_image_url': config["header_image_url"],
        'site_title': config["site_title"],
        'site_description': config["site_description"],
        'header_image_height': config["header_image_height"],
        'header_image_width': config["header_image_width"],
        }

    htmlheader = ['''
<!DOCTYPE html>
<html>
<head>
<title>%(title)s</title>
<link rel="stylesheet" type="text/css" media="screen and (min-width: 481px)" href="style.css">
<link rel="stylesheet" type="text/css" media="only screen and (max-width: 480px)" href="iphone.css">
<link rel="alternate" type="application/rss+xml" title="%(title)s" href="index.xml">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<meta name="viewport" content="user-scalable=no, width=device-width" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="apple-mobile-web-app-status-bar-style" content="black" />
<script type="text/javascript">
var _gaq = _gaq || [];
_gaq.push(['_setAccount', '%(google_analytics_tag)s']);
_gaq.push(['_trackPageview']);
(function() {  var ga = document.createElement('script');
 ga.type = 'text/javascript';
 ga.async = true;
ga.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js';
var s = document.getElementsByTagName('script')[0];
s.parentNode.insertBefore(ga, s);
})();
</script>
</head>
<body>
'''     % { 
        'title': title, 
        'google_analytics_tag': config["google_analytics_tag"], 
        } ]

    # Tons of conditional checks lay ahead. Does it use a header image 
    # and do you want the sidebar on article pages?
    if config["sidebar_on_article_pages"] != True and type == "article":
        htmlheader.append("\n<div class=\"article_container\">\n")
    else:
        htmlheader.append("\n<div class=\"container\">\n")
    if config["header_image_url"] != "" and type == "index":
        htmlheader.append(headerimage)
    elif config["header_image_url"] != "" and type != "index":
        htmlheader.append("<a href=\"/\">\n" + headerimage + "</a>\n")
    elif config["header_image_url"] == "" and type == "index":

        htmlheader.append('''
<div class="header">
<h1>%(site_title)s</h1>
<p>%(site_description)s</p>
</div>
'''     % {
        'site_title': config["site_title"],
        'site_description': config["site_description"],
        } )

    elif config["header_image_url"] == "" and type != "index":
        htmlheader.append('''
<p class="return_link">
<a href="index.html">%(site_title)s</a>
</p>
'''     % {
        'site_title': config["site_title"]
        } )
    if type == "index":
        htmlheader.append("\n<div class=\"article_list\">\n")
    elif type == "archive":
        htmlheader.append("\n<div class=\"article_list\">\n<h1>Article Archive</h1>\n")
    elif type == "article":
        htmlheader.append('''
<div class="article">
<h1>%(title)s</h1>
<p>by <a href="%(author_bio_link)s">%(author_name)s</a> on %(date)s</p>
'''     % {
        'author_bio_link': config["author_bio_link"],
        'title': title,
        'author_name': config["author_name"],
        'date': date,
        } )
    return "".join(htmlheader)

# Subroutine to remove all line breaks to make for some packed fast HTML
def minify(content):
    if config["minify_html"]:
        content = re.sub("\n","",content)
    return content

# Subroutine to build out the footer.
def buildhtmlfooter (type, urltitle):
    footer_parts = []
    sidebardata = open(config["directory"]+"/sidebar.html").read()
    if type == "index" or type == "archive" or config["sidebar_on_article_pages"]:
        footer_parts.append(sidebardata)
    if type == "article":
        footer_parts.append(
'''
<p>Send feedback to <a href="mailto:%(email)s">%(email)s</a> or <a href="http://twitter.com/share?via=%(twitter_tag)s&text=%(urltitle)s">share on twitter</a>.</p>
'''     % {
        'email': config['author_email'], 
        'twitter_tag': config['twitter_tag'], 
        'urltitle': urltitle,
        })
    footer_parts.append("\n</div>\n</body>\n</html>")
    return "".join(footer_parts)

# This program is designed to run as a CGI script so you can rebuild your site by hitting a URL.
print "Content-type: text/html\n\n"
rebuildsite()
print "<html><head><title>Site Rebuilt</title></head><body><h1>Site Rebuilt</h1></body></html>"
\$\endgroup\$
8
\$\begingroup\$

Here are a few suggestions:

String comparisons should be done with == (and !=) instead of is (and is not). is works, but implies you are comparing identity (which is usually a memory address), whereas == is comparing value. See https://stackoverflow.com/a/2988117/331473 for more information.


Your boolean configs (eg: minify_html) should be actual boolean values True/False instead of 1/0. Also, when you are checking for these, you should drop the comparison. Example:

if minify_html == 1:     # or minify_html == True 
   ...                   #    (if you've converted these to booleans)

can be written as:

if minify_html:
    ...

Using module level vars as configs is generally ok for a few things, but once you've got a whole catalog of things, it can be a bit much to keep track of. Several times while reviewing your code I found myself saying "Where did that var come from?".

If you want to address this, you could put these in a dictionary so it's "namespaced" so to speak. Example:

config = {
    "site_url": "...",
    "site_name": "..."
}

then in your code, you can more easily spot config bits:

if config['minify_html']:
    ....

There's more cleanup that can be done, but these are just the first few things that popped out at me. One more thing... I don't have time address it now, but the long chain of if/elses in buildhtmlheader could be refactored a bit to make things a little less redundant.

\$\endgroup\$
7
\$\begingroup\$
  1. I prefer emphasize constants in UPPERCASE.
  2. Imports should usually be on separate lines, e.g.:

    Yes: import os
         import sys
    
    No:  import sys, os
    
  3. Use spaces instead of tabs.

  4. Avoid to name variables as built-in functions/objects. I mean file. If you need this function later, it will take time to figure out why it doesn't work.
  5. With keyword closes file automatically if exception happens. it's preferable for opening files.

    with open(filename, 'r') as f:
        content = f.read()
    
  6. If you need to import your module or part of it, your code will execute rebuildsite each time. To prevent this, use condition

    if __name__ == '__main__':
        print "Content-type: text/html\n\n"
        rebuildsite()
        print "Site Rebuilt</body></html>"
    
  7. To join path is better with os.path.join

    os.path.join(DIRECTORY, 'sidebar.html')
    
  8. More efficient way to concatenate strings in python is to join a lists, use triple quotes for new-line and % for formatting:

    parts = [
        '''
    <!DOCTYPE html>
    <html>
    <head>
      <title>%(title)s</title>
    </head>
    <body>''' % {'title' : title}
    ]
    
    if not SIDEBAR_ON_ARTICLE_PAGES and type == "article":
        parts.append('''
    <div class="article_container">
    ''')
    
    return ''.join(patrs)
    

Follow other PEP8 recommendations

\$\endgroup\$
5
\$\begingroup\$

I like the other two answers so far and I see you've already added some of the advice in.

I have two small additions:

  1. this might be shorter but it is harder to read:

    for nonfile in nonentryfiles: textfiles.remove(config["directory"]+"/"+nonfile) # Except for non-entry files
    

    I'm guessing you've come from a language that emphasizes being consise but instead in python prefer to do things that make it easier to understand in siz months when you want to tweak something

    for nonfile in nonentryfiles:
        textfiles.remove(config["directory"] + "/" + nonfile)
    

    new line whitespace can make it that much easier to read. (also I put spaces in your string concatination. Makes it easier to read too.)

    Another whitespace would be all those if's else's elif's .... its too hard to find where one starts and another finishes.

    try instead:

    if test:
        ...
    else:
        ...
    
    
    if test:
        ...
    elif something else:
        ...
    

    Makes it easier to find each if statement.

  2. Commenting every line like: # Open up the file isn't really going to help. It just makes the useful comments harder to find in all the noise.

    Instead try to find the comments that generalize the section of code and remove the rest. Or if you had to do something unusual leave them in too. Think "if I had no idea what this code does which lines would I need explaining."

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you. I guess I was falling for the old "keep the lines fewer" idea. I get that clarity in code is most important. I'm refactoring now. Thank you for the excellent feedback. \$\endgroup\$ – Mike Shea Dec 19 '11 at 0:56
  • 1
    \$\begingroup\$ I updated the code above based on the feedback you guys sent. I'd always appreciate more tips and tricks. Thank you for the style guide! I'll check that out right now. Learning a lot from you guys. Thank you. \$\endgroup\$ – Mike Shea Dec 19 '11 at 3:18
3
\$\begingroup\$

I'd strongly suggest moving the HTML content out of the code and into separate files; I know you want to keep this simple and single-file but in this case there some real benefits to treating the HTML as a resource rather than as code:

  1. HTML is a finicky format. It's easy to screw up an escape or lose a bracket and mess up the output. With separate files it's trivial to preview them and verify them independent of possible code bugs.
  2. Since HMTL is so finicky, keeping it inline with code also means you'll have to worry about an extra quote character somewhere causing code parsing errors.
  3. It's easier to iterate on tweaky stuff like changes to CSS using an HTML specific editing tool than by messing with tags in a Python IDE or a plain text editor

I'd also use string.Template for any complex string formatting opertations. The % operator is great for single substitutions but Template is better for keeping the code clean:

 example = Template('''
  <span font ="${font}"> Good ${timeofday}, ${salute} ${name}<spam>       
 ''')

 print example.substitute(font='sanserif', timeofday='evening', salute='Mr.', name='Bond')

Plus you can use safe_substitute to allow more error-tolerant display so you don't bork your server.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.