4
\$\begingroup\$

Summary:

This project is a simple static blog site generator written in python, utilizing pandoc to convert posts written in Markdown to HTML. It is essentially a follow up to my previous post: blog site generator in shell script. I got a lot of positive feedback, and multiple people suggested that I rewrite this project in a proper language, like python. So I did. This should make it easier to maintain the code-base and add additional features. Since the previous post, the blog also got a tag-filtering feature, and provides an RSS feed.

Setup:

While reviewing the code is one thing, you might want to spin up a quick demo instance for testing. To do this, clone the μblog repository and run python mublog.py, followed by cd dst && python3 -m http.server 8000. The commit for this version is 21db98a (It has not been merged to the main branch yet).

Review:

As for the code review, I am especially interested in feedback on the following aspects:

  • Path Handling: The script handles a lot of different paths. Currently those are specified in the Config class, and assembled together in the places where they are needed to allow for modularity (e.g. if the user wants to change some path names). I wonder if I can improve this. Currently I have variables for all the important sub-directories, bu tmaybe it would make sense to split this up, so I have variables for the individual directory names, and use those to build the path names, so I have no hardcoded paths in the script at all?
  • RSS Generation / Path conversion: The blog is also able to generate a RSS feed file from the blog posts. This requires encoding of the content, so that special characters don't cause problems. Furthermore, relative links need to be replaced with absolute links, otherwise they won't work in the feed. For this I wrote a regex to extract relative links, and I attempt to replace them with absolute ones. (I feel like this is not ideal).
  • Handling of HTML: I already tried to have as little HTML code as possible hard-coded in the script. Therefore, I load "templates", and insert the relevant content. But some HTML bits are still hard-coded in the script. However, I feel like the entire handling of HTML could be improved - maybe by having specific small functions that generate one specific tag - e.g. generate_html_article_item(relevan_params)? Curious what would work best here.
  • General feedback regarding style and structure

Code:

import glob
import os
import shutil
import subprocess
import re
import urllib.parse
from string import Template
import html
from urllib.parse import urljoin


class Config:
    def __init__(self):
        self.dst_root_dir = "dst/"
        self.dst_posts_dir = f"{self.dst_root_dir}posts/"
        self.dst_css_dir = f"{self.dst_root_dir}css/"
        self.dst_assets_dir = f"{self.dst_root_dir}assets/"
        self.dst_js_dir = f"{self.dst_root_dir}js/"

        self.src_root_dir = "src/"
        self.src_posts_dir = f"{self.src_root_dir}posts/"
        self.src_css_dir = f"{self.src_root_dir}css/"
        self.src_assets_dir = f"{self.src_root_dir}assets/"
        self.src_templates_dir = f"{self.src_root_dir}templates/"

        self.blog_url = "https://my-blog.com/"
        self.blog_title = "John's Awesome Blog"
        self.blog_description = "Short description what the blog is about"
        self.blog_author_name = "John Doe"
        self.blog_author_mail = "[email protected]"
        self.blog_author_copyright = f"Copyright 2023 {self.blog_author_name}"
        self.post_ignore_prefix = "_"
        self.posts = []
        self.pages = []


class PathHandler:

    def __init__(self, dst_root_dir, src_root_dir, blog_url):
        self.dst_root_dir = dst_root_dir
        self.src_root_dir = src_root_dir
        self.blog_url = blog_url

    def generate_absolute_path(self, relative_path):
        if relative_path.startswith('/'):
            # Absolute path from the web-root directory
            return urljoin(self.blog_url, relative_path.lstrip('/'))
        elif relative_path.startswith('../'):
            # Relative path from a post
            return urljoin(self.blog_url, relative_path)
        else:
            # Relative path within a post
            return urljoin(self.blog_url, 'posts/' + relative_path)

    def is_text_link(self, text):
        return text.startswith('"') or text.startswith("'")

    def is_relative_path(self, path):
        absolute_prefixes = ["http://", "https://", "ftp://", "sftp://"]
        for prefix in absolute_prefixes:
            if path.startswith(prefix):
                return False
        return True

    def convert_relative_urls(self, ref_location):
        print(f"Relative URL found at: {ref_location}")


    def make_relative_urls_absolute(self, html_input: str):
        # Information: Matches relative URLs if they are in a link, a, script or img tag.
        # For a URL to be considered relative, it can't start with http:// or https:// or data://
        # It also excludes fragment identifiers (aka #)
        regex_pattern = r'''(?:url\(|<(?:link|a|script|img)[^>]+(?:src|href)\s*=\s*)(?!['"]?(?:data|http|https))['"]?([^'"\)\s>#]+)'''
        new_html = html_input
        offset = 0
        for match in re.finditer(regex_pattern, html_input):
            relative_url = match.group(1)
            absolute_url = self.generate_absolute_path(relative_url)

            Logger.log_info(f"Convert relative URL for RSS feed: {relative_url} => {absolute_url}")

            start_index = match.start(1) + offset
            end_index = match.end(1) + offset
            new_html = new_html[:start_index] + absolute_url + new_html[end_index:]
            offset += len(absolute_url) - len(relative_url)

        return new_html



class Helper:
    @staticmethod
    def check_pandoc_installed() -> None:
        if not shutil.which("pandoc"):
            Logger.log_fail("Pandoc is not installed. Please install Pandoc before continuing.")

    @staticmethod
    def strip_top_directory_in_path(path: str) -> str:
        parts = path.split(os.sep)
        return os.sep.join(parts[1:]) if len(parts) > 1 else path

    @staticmethod
    def clean_build_directory(directory: str) -> None:
        try:
            shutil.rmtree(directory, ignore_errors=True)
        except Exception as e:
            Logger.log_fail(f"Failed to remove old build directory: {str(e)}")

    @staticmethod
    def create_directory(directory: str) -> None:
        try:
            os.makedirs(directory, exist_ok=True)
        except Exception as e:
            Logger.log_fail(f"Failed to create directory: {str(e)}")

    @staticmethod
    def copy_files(src_path: str, dst_path: str) -> None:
        try:
            for f in glob.glob(f"{src_path}/*"):
                shutil.copy(f, dst_path)
        except Exception as e:
            Logger.log_fail(f"Failed to copy files: {str(e)}")

    @staticmethod
    def read_file_contents(file_path: str) -> list[str]:
        try:
            with open(file_path, 'r') as file:
                return file.readlines()
        except FileNotFoundError:
            Logger.log_fail(f"Failed to load file '{file_path}'.")

    @staticmethod
    def src_to_dst_path(src_file_path: str, dst_dir: str, dst_ext: str) -> str:
        file_name = os.path.basename(src_file_path)
        base_name, _ = os.path.splitext(file_name)
        return os.path.join(dst_dir, base_name + dst_ext)

    @staticmethod
    def replace_file_extension(file_path: str, ext: str) -> str:
        root, old_extension = os.path.splitext(file_path)

        if not old_extension:
            Logger.log_fail(f"The file path '{file_path}' does not have an extension to replace.")

        return root + '.' + ext.strip('.')

    @staticmethod
    def writefile(path: str, contents: str) -> None:
        with open(path, "w", encoding="utf-8") as f:
            f.write(contents)

    @staticmethod
    def substitute(mapping: dict[str, str], in_path: str, out_path: str = None) -> None:
        if not out_path:
            out_path = in_path

        template_text = Helper.read_file_contents(in_path)
        template = Template("".join(template_text))
        output = template.substitute(mapping)
        Helper.writefile(out_path, output)


class Blog:

    def __init__(self, config: Config):
        self.config = config

    def generate(self) -> None:
        Helper.check_pandoc_installed()
        self.clean_build_directory()
        self.create_output_directories()
        self.copy_files_to_dst()
        self.process_posts()
        self.process_pages()

    def clean_build_directory(self) -> None:
        Helper.clean_build_directory(self.config.dst_root_dir)

    def create_output_directories(self) -> None:
        directories = [
            self.config.dst_root_dir,
            self.config.dst_posts_dir,
            self.config.dst_css_dir,
            self.config.dst_assets_dir,
            self.config.dst_js_dir,
        ]
        for directory in directories:
            Helper.create_directory(directory)

    def copy_files_to_dst(self) -> None:
        Helper.copy_files(self.config.src_css_dir, self.config.dst_css_dir)
        Helper.copy_files(self.config.src_assets_dir, self.config.dst_assets_dir)

    def process_posts(self) -> None:
        builder = SiteBuilder(self.config)
        for file_path in glob.glob(self.config.src_posts_dir + "*.md"):
            if not os.path.basename(file_path).startswith(self.config.post_ignore_prefix):
                post = Post(self.config, file_path)

                self.config.posts.append(post)
                builder.generate_post(post)

        builder.generate_js()
        builder.generate_rss_feed()

    def process_pages(self) -> None:
        builder = SiteBuilder(self.config)
        for file_path in glob.glob(self.config.src_root_dir + "*.md"):
            page = Page(self.config, file_path)
            self.config.pages.append(page)
            if page.src_path.endswith("tags.md"):
                builder.generate_tags_page(page)
            elif page.src_path.endswith("articles.md"):
                builder.generate_articles_page(page)
            else:
                builder.generate_page(page)


class Page:

    def __init__(self, config: Config, src_page_path: str):
        self.config = config
        self.src_path = src_page_path
        self.dst_path = Helper.src_to_dst_path(src_page_path, self.config.dst_root_dir, ".html")


class Post:
    def __init__(self, config: Config, src_file_path: str):
        self.config = config
        self.title = ""
        self.description = ""
        self.date = ""
        self.tags = []
        self.src_path = ""
        self.dst_path = ""
        self.dst_path_remote = ""
        self.raw_file_contents = ""
        self.raw_html_content = ""

        self.validate_post(src_file_path)

        self.src_path = src_file_path
        self.dst_path = Helper.src_to_dst_path(src_file_path, self.config.dst_posts_dir, ".html")
        self.dst_path_remote = Helper.strip_top_directory_in_path(Helper.replace_file_extension(self.src_path, ".html"))
        self.filename = os.path.basename(self.dst_path)

    def validate_post(self, src_file_path: str) -> None:
        Logger.log_info(f"Processing {src_file_path} ...")
        self.raw_file_contents = Helper.read_file_contents(src_file_path)

        # Check that file is long enough to accommodate header
        if len(self.raw_file_contents) < 6:
            Logger.log_fail(f"Failed to validate header of {src_file_path}.")

        # Validation line 1: Starting marker
        if self.raw_file_contents[0].strip() != "---":
            Logger.log_fail(f"Failed to validate header of {src_file_path}")
            Logger.log_fail(f"The starting marker \"---\" is missing or incorrect")

        # Validation line 2: title field
        if not re.match(r'^title:\s*(\S+)', self.raw_file_contents[1]):
            Logger.log_fail(f'Failed to validate header of {src_file_path}')
            Logger.log_fail(f'The title field is missing, empty, or incorrect.')
        self.title = re.search(r'^title:\s*(.*?)\s*$', self.raw_file_contents[1]).group(1)

        # Validation line 3: description field
        if not re.match(r'^description:\s*(\S+)', self.raw_file_contents[2]):
            Logger.log_fail(f'Failed to validate header of {src_file_path}')
            Logger.log_fail(f'The description field is missing, empty, or incorrect.')
        self.description = re.search(r'^description:\s*(.*?)\s*$', self.raw_file_contents[2]).group(1)

        # Validation line 4: date field
        if not re.match(r'^date:\s*([0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])$)', self.raw_file_contents[3]):
            Logger.log_fail(f'Failed to validate header of {src_file_path}')
            Logger.log_fail(f'The date field is missing, empty, or not in the correct format (YYYY-MM-DD)')
        self.date = re.search(r'([0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])$)',
                              self.raw_file_contents[3]).group(1)

        # Validation line 5: tags field
        if not re.match(r'^tags:\s*(\S+)', self.raw_file_contents[4]):
            Logger.log_fail(f'Failed to validate header of {src_file_path}')
            Logger.log_fail(f'The tags field is missing, empty, or incorrect.')
        tag_values = re.search(r'^tags:\s*(.*?)\s*$', self.raw_file_contents[4]).group(1)
        self.tags = [tag for tag in re.findall(r'[^,\s][^,]*[^,\s]|[^,\s]', tag_values)]

        # Validation line 6: Ending marker
        if self.raw_file_contents[5].strip() != "---":
            Logger.log_fail(f"Failed to validate header of {src_file_path}")
            Logger.log_fail(f"The ending marker \"---\" is missing or incorrect")


class SiteBuilder:
    def __init__(self, config: Config):
        self.config = config

    def load_template(self, template_path: str) -> str:
        try:
            with open(template_path, encoding="utf-8") as f:
                return f.read()
        except FileNotFoundError:
            Logger.log_fail(f"Template file {template_path} not found.")
        except IOError:
            Logger.log_fail(f"Failed to open template file {template_path}.")

    def generate_js(self) -> None:
        js_template = self.load_template(self.config.src_templates_dir + "tags.js.template")
        entries = [
            f'    "{post.filename}": [{", ".join([f"{tag!r}" for tag in post.tags])}]'
            for post in self.config.posts
        ]
        mapping = "\n" + ",\n".join(entries) + "\n"
        substitutions = {"tag_mapping": mapping}
        js_data = Template(js_template).substitute(substitutions)
        Helper.writefile(self.config.dst_js_dir + "/tags.js", js_data)
        Logger.log_pass(f"Processed JS file.")

    def generate_post(self, post: Post) -> None:
        content = self.convert_md_html_with_pandoc(post.src_path)
        post.raw_html_content = content
        post_template = self.load_template(self.config.src_templates_dir + "post.template")

        # Generate the tags for the post
        post_tags = "<div class=\"tags\">"
        for tag in post.tags:
            tag_param = urllib.parse.urlencode({"tag": tag})
            post_tags += (
                f'<div class="tag-bubble" onclick="location.href=\'/articles.html?{tag_param}\'">'
                f"{tag}</div>"
            )
        post_tags += "</div>"

        substitutions = {
            "author_mail": self.config.blog_author_mail,
            "author_copyright": self.config.blog_author_copyright,
            "title": post.title,
            "content": content,
            "post_tags": post_tags,
            "css_dir": Helper.strip_top_directory_in_path(self.config.dst_css_dir),
            "js_dir": Helper.strip_top_directory_in_path(self.config.dst_js_dir),
        }
        post_data = Template(post_template).substitute(substitutions)
        Helper.writefile(post.dst_path, post_data)
        Logger.log_pass(f"Successfully processed {post.src_path}")

    def generate_tags_page(self, page: Page) -> None:
        unique_tags = list(set(tag for post in self.config.posts for tag in post.tags))
        tag_counts = {tag: sum(tag in post.tags for post in self.config.posts) for tag in unique_tags}
        sorted_tags = sorted(unique_tags, key=lambda tag: tag_counts[tag], reverse=True)

        content = self.convert_md_html_with_pandoc(page.src_path)
        content += "<div class=\"tags\">"
        for tag in sorted_tags:
            tag_count = tag_counts[tag]
            tag_param = urllib.parse.urlencode({"tag": tag})
            content += (
                f'<div class="tag-bubble" onclick="location.href=\'articles.html?{tag_param}\'">'
                f"{tag}<span>{tag_count}</span></div>"
            )
        content += "</div>"

        post_template = self.load_template(self.config.src_templates_dir + "page.template")
        substitutions = {
            "author_mail": self.config.blog_author_mail,
            "author_copyright": self.config.blog_author_copyright,
            "title": "Blog",
            "content": content,
            "css_dir": Helper.strip_top_directory_in_path(self.config.dst_css_dir),
            "js_dir": Helper.strip_top_directory_in_path(self.config.dst_js_dir),
        }
        page_data = Template(post_template).substitute(substitutions)
        Helper.writefile(page.dst_path, page_data)
        Logger.log_pass(f"Successfully processed {page.src_path}")

    def generate_articles_page(self, page: Page) -> None:
        content = self.convert_md_html_with_pandoc(page.src_path)
        content += "<article>\n"
        content += "<ul class=\"articles\">\n"
        for post in self.config.posts:
            content += (
                f'<li id=\"{post.filename}\"><b>[{post.date}]</b> <a href="{post.dst_path_remote}">{post.title}</a></li>\n'
            )
        content += "</ul>\n"
        content += "</article>"

        post_template = self.load_template(self.config.src_templates_dir + "page.template")
        substitutions = {
            "author_mail": self.config.blog_author_mail,
            "author_copyright": self.config.blog_author_copyright,
            "title": "Blog",
            "content": content,
            "css_dir": Helper.strip_top_directory_in_path(self.config.dst_css_dir),
            "js_dir": Helper.strip_top_directory_in_path(self.config.dst_js_dir),
        }
        page_data = Template(post_template).substitute(substitutions)
        Helper.writefile(page.dst_path, page_data)
        Logger.log_pass(f"Successfully processed {page.src_path}")

    def generate_page(self, page: Page) -> None:
        Logger.log_info(f"Generating page: {page.src_path} ...")
        content = self.convert_md_html_with_pandoc(page.src_path)
        post_template = self.load_template(self.config.src_templates_dir + "page.template")
        substitutions = {
            "author_mail": self.config.blog_author_mail,
            "author_copyright": self.config.blog_author_copyright,
            "title": "Blog",
            "content": content,
            "css_dir": Helper.strip_top_directory_in_path(self.config.dst_css_dir),
            "js_dir": Helper.strip_top_directory_in_path(self.config.dst_js_dir),
        }
        page_data = Template(post_template).substitute(substitutions)
        Helper.writefile(page.dst_path, page_data)
        Logger.log_pass(f"Successfully processed {page.src_path}")

    def generate_rss_feed(self):
        rss_template = self.load_template(self.config.src_templates_dir + "/rss.xml.template")
        rss_items = ""
        for post in self.config.posts:
            # Replace relative urls with absolute URLs
            edited = pm.make_relative_urls_absolute(post.raw_html_content)

            rss_items += "<item>\n"
            rss_items += f"<title>{html.escape(post.title)}</title>\n"
            link = self.config.blog_url + Helper.strip_top_directory_in_path(self.config.dst_posts_dir) + post.filename
            rss_items += f"<link>{link}</link>\n"
            rss_items += f"<description>{html.escape(edited)}</description>\n"
            rss_items += "</item>\n"

        substitutions = {
            "blog_title": self.config.blog_title,
            "blog_url": self.config.blog_url,
            "blog_description": self.config.blog_description,
            "rss_items": rss_items
        }
        feed = Template(rss_template).substitute(substitutions)
        Helper.writefile(self.config.dst_root_dir + "feed.xml", feed)
        Logger.log_pass(f"Successfully generated rss feed")

    @staticmethod
    def convert_md_html_with_pandoc(src_path: str) -> str:
        command = ["pandoc", src_path, "-f", "markdown", "-t", "html"]
        try:
            result = subprocess.run(command, check=True, capture_output=True, text=True)
            return result.stdout
        except subprocess.CalledProcessError:
            Logger.log_fail(f"Pandoc failed while processing {src_path}")


class Logger:
    PASS = "\033[32m[PASS]\033[0m"
    FAIL = "\033[31m[FAIL]\033[0m"
    INFO = "\033[34m[INFO]\033[0m"
    WARN = "\033[33m[WARN]\033[0m"

    @staticmethod
    def log_info(message: str) -> None:
        print(f"{Logger.INFO} {message}")

    @staticmethod
    def log_fail(message: str) -> None:
        print(f"{Logger.FAIL} {message}")
        exit(1)

    @staticmethod
    def log_warn(message: str) -> None:
        print(f"{Logger.WARN} {message}")

    @staticmethod
    def log_pass(message: str) -> None:
        print(f"{Logger.PASS} {message}")


cfg = Config()
pm = PathHandler(cfg.dst_root_dir, cfg.src_root_dir, cfg.blog_url)
blog = Blog(cfg)
blog.generate()

Thanks for reading, and I am looking forward for your feedback!

\$\endgroup\$
5
  • 1
    \$\begingroup\$ Can you provide an idea of what this project looks like in the context of the server root filesystem tree? Where does everything live? \$\endgroup\$
    – Reinderien
    Jul 5, 2023 at 15:10
  • \$\begingroup\$ @Reinderien All the files generated by the script are stored in the dst/ directory. Think of the dst/ directory as the web-server root directory. Inside there, you have sub-directories for posts/, assets/, css/ and js. The main pages, such as home, about etc are just in /dst. To get a better idea of the structure, clone the repo I linked above, and spin up a blog instance locally, that should make it easier to see where everything goes. Also the README contains a lot of details. Let me know if you need more info :) \$\endgroup\$
    – 766F6964
    Jul 5, 2023 at 16:24
  • \$\begingroup\$ But /dst where (from the filesystem root)? \$\endgroup\$
    – Reinderien
    Jul 5, 2023 at 17:36
  • \$\begingroup\$ @Reinderien Not entirely sure I understand what you mean. The script creates the dst/ directory in the same directory the script was executed from. And the contents in dst/ should be put on the web-root of the server. I wrote /dst instead of dst/ in my previous comment by accident \$\endgroup\$
    – 766F6964
    Jul 5, 2023 at 18:04
  • 2
    \$\begingroup\$ Alright, well. That's going to be a topic for feedback. \$\endgroup\$
    – Reinderien
    Jul 5, 2023 at 18:05

1 Answer 1

3
+100
\$\begingroup\$

Directories

Your path-formatting such as f"{self.dst_root_dir}posts/" should be replaced with operations from pathlib.Path. Similar for os.sep.join.

You need to shift gears and add argparse configurability.

Configuration

Config has confused responsibilities: it contains configuration for the top-level server (directories) and configuration for the site. In a mature web application, the server and site are configured separately. This makes it much easier to support multiple sites.

Web odds and ends

is_relative_path should not iterate. Instead, do a built-in urlparse to get the scheme. But... this method is a little strange, because it doesn't cover all cases where the path is absolute. The scheme-less /foo/bar is also an absolute path. You say:

By "absolute," I mean providing the full server URL to the file rather than just an absolute path within the web-root directory

That isn't "absolute", and you need to change your terminology. I think you mean "domain-qualified". Again, /foo/bar is an absolute path.

convert_relative_urls doesn't convert anything; it just prints.

make_relative_urls_absolute is terrifying. There is no universe where this is a good idea. If you absolutely had to parse HTML to sanitize links, this is not the way to do it; but - what is the purpose for sanitizing links here? Is the blog content already HTML, and you're doing some kind of processing step? For what purpose? If it's in the name of security, you have vastly bigger problems. You say:

The reason for this workaround is to ensure images etc. are shown correctly in the RSS feed. RSS does not allow relative links.

So then either:

  • Document that the user cannot include relative links; or
  • Throw an error if relative links are detected upon parse of the document by something like BeautifulSoup (not a regex); or
  • Parse by BeautifulSoup and operate on the DOM from a well-defined in-memory representation, substituting out links in elements instead of in text manipulation.

You further ask:

I would also like some more feedback on how to improve the parts, where I generate HTML that gets inserted

If the blog-writing developer is implicitly trusted, just accept Jinja2 templates from them and render them as needed (you can compose user templates with your own program templates if needed). If the blogger is untrusted, parse the DOM with BeautifulSoup and manipulate it with strainer expressions before re-rendering it.

Misc

Helper needs to not be a class. Move it to its own helper.py module and demote all of those methods to globals. You get scope from the module, and don't have to worry about class statics.

Don't write your own Logger. Don't do it. Configure the built-in logging, and add your fancy formats there if you want.

check_pandoc_installed is less than helpful. You should throw an exception from here, or at least return a boolean.

read_file_contents and writefile shouldn't exist. The former hides exceptions from the scope where they actually matter. File I/O is so simple that there's no benefit to writing a helper for this, and you'll be better off writing out open whenever you need it.

Move this:

cfg = Config()
pm = PathHandler(cfg.dst_root_dir, cfg.src_root_dir, cfg.blog_url)
blog = Blog(cfg)
blog.generate()

to a def main(), and call it from if __name__ == '__main__'.

Regexes

Sometimes they're called-for; sometimes they're extremely not (parsing HTML).

When they are called-for, something like

r'^date:\s*([0-9]{4}-(0[1-9]|1[0-2])-(0[1-9]|[1-2][0-9]|3[0-1])$)'

can and should be rewritten for legibility; something like:

r'''(?x)
^date:\s*    # start tag
(?P<year>[0-9]{4})-        # ISO 8601 YYYY
(?P<month>0[1-9]|1[0-2])-  # ISO 8601 MM
(?P<day>(0[1-9]|[1-2][0-9]|3[0-1])  # ISO 8601 DD
$
'''

The x gets you verbose mode, which allows for comments and whitespace trimming. Once the regex is compiled this will not have any impact on performance.

\$\endgroup\$
6
  • \$\begingroup\$ (1/3) Thanks for the feedback. A lot of good advice, but there are also some things, that I don't necessarily agree with, and some I should probably clarify. I would appreciate further elaboration on a few specific points. For directories, using pathlib is good advice. However, I'd appreciate feedback on whether it makes sense to have separate variables for directory names and combined variables for modular paths. \$\endgroup\$
    – 766F6964
    Jul 8, 2023 at 20:06
  • \$\begingroup\$ (2/3) The daemon specific points I don't agree with. This script is intended to be run once, I find it acceptable to create a "dst" directory in the same location as the script. It might be a good idea to configure this through command arguments as you mentioned, or at the very least, inform the user. As for your question about the relative path conversion - The reason for this workaround is to ensure images etc. are shown correctly in the RSS feed. RSS does not allow relative links. \$\endgroup\$
    – 766F6964
    Jul 8, 2023 at 20:06
  • \$\begingroup\$ (3/3) By "absolute," I mean providing the full server URL to the file rather than just an absolute path within the web-root directory. The logger and regex advice is good. I would also like some more feedback on how to improve the parts, where I generate HTML that gets inserted, like explained in my post. \$\endgroup\$
    – 766F6964
    Jul 8, 2023 at 20:07
  • \$\begingroup\$ What do you consider a "modular path"? \$\endgroup\$
    – Reinderien
    Jul 8, 2023 at 23:06
  • 1
    \$\begingroup\$ Typically a library package such as this will define TOP_DIR or PREFIX to carve out a bit of namespace for itself. And then within that, the library will commonly insist that uniform sub-dir names be used. This makes things easier for a support community, so someone reporting or responding to a problem will be clear on which files are playing which role. If a sysad really wants to rock the boat, perhaps because a giant .html file threatens to fill a partition, one can always "fix it with a symlink", which the library can be blissfully ignorant of. \$\endgroup\$
    – J_H
    Jul 9, 2023 at 19:31

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.