@libre-man Hey thanks for the swift PR, I was having trouble seeing how one would configure the template used but didn't want to discourage you. This is great!. Besides the notes in the commits, there shouldn't be anything of djula related in the main code base, everything should be in a plugin. In the main code base there should be only extension points.

Also, why change the signature of render to &rest instead of adding a new keyword parameter?

@kingcons This is a rather big change, please let me know if I've missed anything

Love the idea here and this initial effort certainly demonstrates that it is feasible!

2 things I would like to see changed if possible:

1. Actually have Djula enabled as a plugin and store related variables in the config objects are we do in other plugins. Note that this would move the djula dependency to an eval-when in the plugins/djula.lisp file.

2. No case expressions, use method dispatch! I would strongly prefer that we use some sort of indirection here. My first thought is storing :djula or :cl-closure-template in the config object and using EQL-specialized methods to handle those cases. The functions in question are: render-page, theme-fn, and compile-theme. The first two could be fixed in a straightforward manner, e.g.

• (defmethod render-page (content (template-engine (eql :djula)) &optional theme-fn &rest render-args) ...
• (defmethod theme-fn (name (template-engine (eql :djula)) &optional (package (theme *config*))

Finally, I'm still pondering the variety of places that we do intern is being used. It seems that is mostly done for interoperation between Djula themes and our current scheme of referring to themes by symbol. I think the real question here is how to refactor compile-theme and theme-fn.

I do not really like the idea of using a hashmap for this, I think we would implementing a module system that way. However we could simply use the existing module system and make sure theme-engine should provide some methods, such as

(precompile theme) -> void
(theme-fn theme name &optional package) -> (lambda &rest rest) -> string
(render-page theme content &optional theme-fn &rest render-args) -> string

This way we can also separate cl-closure from the main base, and we would could both remove them as dependencies.

The only problem I see with this is that each template-engine should implement it's own atom, rss, and sitemaps files.

I think the usage of intern would also be reduced using this kind of setup.

By the way: I found a bug with my current implementation because rss, atom and sitemaps won't be generated correctly.

I'm not sure I'm understand your argument against using a hash table. Are you saying we would be reimplementing a module system poorly? If so I don't follow.

The reason why I suggested we use a hash table instead of a case is to move the dispatch 'logic' from compile time to run-time. One of Coleslaw's design principles is to be easy to hack and extend, having a fixed set of acceptable values goes against this. Say someone wants to add a cl-mustache plugin? They'll have to modify coleslaw instead of just adding a lambda to a hash table in their plugin. Is my motiviation for the change clearer? Btw I'm not deadset on using a hash table, if there is another alternative so the logic is kept extensible I'll love to hear it.

Sorry, English is not my first language, so I'm might not be able to express myself coherently.

I agree that in hindsight (for me) using case statements was a bad idea. However using a hash table would be implementing a module system, or implementing a part of CLOS.

So I would like to move all template engines to plugins, and load them on the fly when we need them. I would prefer using module that should have defined and specified the methods I described in my previous post.

This way the code is easier to read for lisp programmers, as it uses techniques that are commonly used for dispatching. This would still make it possible to add modules at runtime, and it might be somewhat faster than a hash-table (however I don't think that should be a real reason to use CLOS for this).

implementing a part of CLOS

Yes, it is mimicking method dispatch.

I saw @kingcons suggested to use generic functions instead. A more principled approach. Seems a better idea.

This took a bit longer than expected, some other things were somewhat more important; sorry. I think this should be a good implementation. I only think config.lisp requires some cleanup, we're still interning and template engines are still seen as plugins, but they are handled different. Does anybody have some suggestions?

What is the current status of this merge request. What should I change to get it merged?

@libre-man Sorry for lack of communication. Thanks a lot your work to allow Coleslaw to use djula as the template engine for the themes. I'll re-review the whole PR and try to give you detailed feedback on Sunday. Some quick feedback:

• *template-engine* should be moved to the blog object
• Moving the closure theme engine to a plugin should not be part of this PR. Or at least be done in a way that doesn't require users to modify their current config file (ie not having to add :closure to the plugins).

No problem, I myself was also very busy the last month so I wouldn't have had the time to change anything. Thanks for the feedback. A few remarks:

• *templating-engine* (I think you mean this variable) should have been deleted by me. Everything should be using the blog object now.
• The default value of the template-engine is 'CL-CLOSURE so this PR should be backwards compatible for all users.

I'm looking forward to your detailed feedback and thank you very much for your help.

Note: To see the consolidated changes of all the commits I used the Files changed tab view w/o whitespace

• The new 'theme backend protocol' should be documented in docs/hacking.md.
• The loading of plugins should stay in load-config. Automatically loading the plugin of the templating-engine is not a good idea. This would be mean that closure-template should stay in the main codebase for backwards compatibility.
• djula is still listed as a dependency in ASDF definition. It should be removed.
• In coleslaw.lisp there is still *djula-post-template* and *djula-index-template*. They should be removed. *templating-engine* should be removed as well.
• In themes.lisp get-djula-theme should be moved to the plugin

Due to the size of this PR, I think that the best path forward is to split this PR in at least two. One PR to allow the themes backend to be used to be configured and the second one that adds the djula theme backend plugin. A third PR could move closure-template to a plugin and assert that at least one of the theme backend plugins are in the config options.

@libre-man I want to thank you once again for taking the time to write this PR and apologize for the our belated responses.

One thing that I left out was that the get-theme-fn in the djula plugin has a (break) statement in the code.

While working on extracting the template engine API from this PR I've been thinking, is it necessary for render-page to be a generic function and a method provided by each template engine? Couldn't it be written in terms of get-theme-fn?

Hey thanks for your feedback. I have a few remarks:

• I would prefer that closure-template would remain (or become) a plugin and therefore a optional dependency. I don't see the need to give it special rights and I would think the code would be cleaner and easier to extend when we treat all the template engines the same. We need to load a templating-engine anyway and by just giving it a default value there is no breaking behavior.
• The reason I made render-page a generic function is that it would be easier to extend it by providing auxiliary methods that change or extend the behavior.

I will remove all the useless code and variables this week and will split the PR in two parts if that is your preference.

I would prefer that closure-template would remain (or become) a plugin and therefore a optional dependency

I certainly agree that that should be the end result. I just want to split to have a transition phase. I've been thinking that a reasonable upgrade path would be the check if the template-engine slot is unbound then emit a warning and load the cl-closure plugin. But that would be part of the 3rd PR.

The reason I made render-page a generic function is that it would be easier to extend it by providing auxiliary methods that change or extend the behavior.

We can evaluate the need when it presents itself, as it stands both render-page methods are identical. Btw, we probably should provide a template-engine base class to specialize default methods on.

will split the PR in two parts if that is your preference.
Thank you for this, smaller changes are always easier to apply.

Hi, what happened with this? Why was it not merged?

Will it be possible to now easily add other templating engines? I was thinking that if we do, we may be able to just access all of the themes available in other static generators like Hugo, and that would be very nice

Hi, what happened with this? Why was it not merged?

AFAICS:

will split the PR in two parts