6
\$\begingroup\$

I've created a simple static website generator called pjen. The GitHub repository can be found here.

import os

class pjen:
    def __init__(self, path=None):
        """
        By default, the path will be set to the current working directory, 
        or the path can be specified using the 'path' keyword parameter.
        """

        if path is None:
            self.path = os.getcwd() 
        else:
            self.path = path

        self.path += "/website"

    def create_project(self):
        """
        Creates an initial file structure for a project at the specified path. 

        An exception is raised if a project already exists in the desired location.

        Creates the following file structure:

        website
          |-> templates
              |-> group1
          |-> images
          |-> scss
          |-> css
          |-> scripts

        """

        if not os.path.exists(self.path):

            #make the root directory
            os.makedirs(self.path)

            os.makedirs(self.path+"/templates")
            os.makedirs(self.path+"/templates/group1")          
            os.makedirs(self.path+"/images")
            os.makedirs(self.path+"/scss")
            os.makedirs(self.path+"/css")
            os.makedirs(self.path+"/scripts")

            print("Created project in {}".format(self.path))

        else:
            raise IOError("A directory already exists at: {}".format(self.path))


    def _count_indent(self, str, tab_equiv=4):
        """
        Returns the number of leading spaces. A tab is counted as 'tab_equiv' spaces.
        """

        i = 0
        count = 0

        while(str[i] == " " or str[i] == "\t"):

            if str[i] == " ":
                count += 1
            if str[i] == "\t":
                count += 4

            i += 1

        return count

    def _sanatise_file_list(self, l, fname=None):
        """
        Removes blacklisted files from the input list 'l'. In addition, a file with 
        the name 'fname' can also be removed.
        """

        blacklist = [".DS_Store"]

        if fname:
            blacklist.append(fname)

        for item in blacklist:
            try:
                l.pop(l.index(item))
            except ValueError:
                pass

    def generate(self):
        """
        Iterates through all the directories in the 'templates' directory, inserting all the 
        inputs into each template. 
        """

        #get a list of template groups
        static_groups = os.listdir(self.path+"/templates")
        self._sanatise_file_list(static_groups)

        print("Found {} group(s)".format(len(static_groups)))

        #iterate through each of the template groups
        for group in static_groups:

            #get each of the files in the group
            files = os.listdir(self.path+"/templates/"+group)

            #remove the template and hidden file
            self._sanatise_file_list(files, "template.html")

            #open the template 
            with open(self.path+"/templates/" + group + "/template.html", "r") as template:

                #iterate though the files that need to be generated
                for f in files:

                    #create a new static page
                    with open(self.path + "/" + f, "w") as page:

                        print("Generating file: {}".format(f))

                        #open up the input
                        with open(self.path + "/templates/" + group + "/" + f, "r") as my_input:

                            #iterate through the input and extract the various sections
                            css = ""
                            html = ""
                            scripts = ""

                            #flag to determine the section of the input file
                            in_section = None

                            #iterate through the input file and set the appropriate flag
                            for line in my_input.readlines():
                                if line.lstrip().startswith("{{ css }}"):
                                    in_section = "css"
                                elif line.lstrip().startswith("{{ html }}"):
                                    in_section = "html"
                                elif line.lstrip().startswith("{{ scripts }}"):
                                    in_section = "scripts"
                                else:
                                    if in_section == "css":
                                        css += line
                                    if in_section == "html":
                                        html += line
                                    if in_section == "scripts":
                                        scripts += line


                            #reset the file pointer as the template can be read many times
                            template.seek(0)

                            #iterate through the template file
                            for line in template.readlines():

                                if line.lstrip().startswith("{{ css }}"):

                                    indent = self._count_indent(line)

                                    #if there is css in the input file then insert it
                                    if css != "":

                                        for insert_line in [x + "\n" for x in css.split("\n")]:

                                            page.write(" "*indent + insert_line)

                                #if there is html in the input file then insert it
                                elif line.lstrip().startswith("{{ html }}"):

                                    indent = self._count_indent(line)

                                    if html != "":

                                        for insert_line in [x + "\n" for x in html.split("\n")]:

                                            page.write(" "*indent + insert_line)

                                #if there are script links in the input file then insert them
                                elif line.lstrip().startswith("{{ scripts }}"):

                                    indent = self._count_indent(line)

                                    if scripts != "":

                                        for insert_line in [x + "\n" for x in scripts.split("\n")]:

                                            page.write(" "*indent + insert_line)

                                #otherwise copy the template text
                                else:
                                    page.write(line)


if __name__ == "__main__":
    p = pjen()
    p.generate()
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

create_project doesn't have the best name. It should be create_project_folders if you want to be exact. Even if you don't want to be verbose, you're print call should be clearer about what exactly it does. I would flip your test, and use if os.path.exists(self.path) to raise IOError so then the rest of the function doesn't need to nested. Note that it also shows the flow better. In pseudocode:

create_project function:
    if project folder exists:
        raise error

    make folders

Speaking of making the folders, you repeatedly call the same function with new strings each time. This is perfect place to use a for loop. In addition, the strings should really be constants. Probably a set of constants at the class level:

class pjen:

    PROJECT_FOLDERS = (
                        "/templates",
                        "/templates/group1",
                        "/images",
                        "/scss",
                        "/css",
                        "/scripts",
                      )

And now you can iterate over these folders to make them in create_project:

def create_project(self):

    if os.path.exists(self.path):
        raise IOError("A directory already exists at: {}".format(self.path))

    for folder in pjen.PROJECT_FOLDERS:
        os.makedirs(self.path + folder)

    print("Created project in {}".format(self.path))

Note that I haven't included os.makedirs(self.path). That's because Python makes all the directories necessary, not just the last one in the list (this is, in fact, why it's makedirs and not makedir). In the same way, there's no need to call os.makedirs(self.path + "/templates") and `os.makedirs(self.path + "/templates/group1"), as the latter would create both folders. However I kept it as a constant so that you can see the full list of folders created as that can be more readable, and you may want/need to iterate over all project folders at some point.

Don't use str as a variable name, as it will shadow the built in str function. Using string is usually better. I also don't like _count_indent starting with an underscore. That's a convention to signal that the function is private and shouldn't be used externally. I don't see why that should be the case here, apart from that you don't expect people to need it. But I'd personally leave the underscore off.

l is a bad name for an input list, either input_list or file_list would be much better. If fname is None then it'll be fine as it just wont have any effect on a list of strings. Also instead of appending a single fname to blacklist you could make a tuple:

    blacklist = (".DS_Store", fname)

In fact I'd iterate directly over that rather than assigning it to blacklist.

Also you're using pop to remove items, but you can just use remove. pop returns the value after removing that, if you don't need that then you don't need pop.

def _sanatise_file_list(self, l, fname=None):
    """
    Removes blacklisted files from the input list 'l'. In addition, a file with 
    the name 'fname' can also be removed.
    """

    for item in (".DS_Store", fname):
        try:
            l.pop(l.index(item))
        except ValueError:
            pass

generate is far too long as a function. You should break it up into smaller functions. Ideally one job per function. This helps readability, clarity and testing.

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

is is broken:

>>> 1000 is 10**3
False
>>> 1000 == 10**3
True

You can read more about this here, but in short it's better to use ==.


Ternary statements:

I really like ternaries, so I may be a little biased, but you can use ternary statements to simplify logic:

    if path is None:
        self.path = os.getcwd() 
    else:
        self.path = path

into:

    self.path = path if path != None else os.getcwd()

while loop:

Instead of using a while loop, you can simply iterate using a for loop:

(also, Python does not require explicit brackets around the loop)

(also, str[i] cannot be both " " and "\t", so you can use a elif for the second one.)

    while(str[i] == " " or str[i] == "\t"):

        if str[i] == " ":
            count += 1
        if str[i] == "\t":
            count += 4

        i += 1

    return count

into:

    for item in str:
        if item == " ":
            count += 1
        elif item == "\t":
            count += 4

    return count
\$\endgroup\$
2
  • \$\begingroup\$ Your for loop isn't equivalent because you're not breaking when you encounter a non-whitespace character. \$\endgroup\$ Commented Dec 16, 2015 at 14:50
  • \$\begingroup\$ Thanks for the suggestions - I really like the ternary statements and plan to add these in. As @SuperBiasedMan alluded to, I've used the while loop as I thought it was cleaner. \$\endgroup\$
    – soarjay
    Commented Dec 16, 2015 at 21:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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