coleslaw-org / coleslaw

Flexible Lisp Blogware
BSD 2-Clause "Simplified" License
553 stars 82 forks source link

Introduce the theme-engine protocol #116

Closed PuercoPop closed 4 years ago

PuercoPop commented 7 years ago

The goal of this protocol is to allow users to customize what templates their themes use. Introduce a new generic function GET-THEME-FN and make COMPILE-THEME a generic function as well. Move all the cl-closure template specific code to closure-template-engine

A thing I'm unsure of is if using keywords for template engines is good enough or if we should use the 'Class method' approach used by the document protocol? I'm inclined to go with the simpler approach for the time being.

@libre-man I would appreciate your feedback on the API, which is a subset of your PR. Is the documentation understandable? Do you think something is missing?

Note that I intend to merge against the 0.9.8 branch, The idea is to get your PR and #115 merged and do some light testing before merging on the master branch and release a new version of Coleslaw

PuercoPop commented 7 years ago

Ok, after adapting the djula theme engine to this protocol I found a couple of issues

Ferada commented 7 years ago

Regarding the package argument, the closure code also has it again in the called theme-fn as an optional argument - can't it be removed for good from the generic function then?

PuercoPop commented 7 years ago

@Ferada I think you are right, I'll look into it.

Btw, I've fixed the types noted in #120 thanks for catching them

kingcons commented 7 years ago

I imagine class methods aren't necessary here. Keyword specializers seem a bit more appropriate to me.

libre-man commented 7 years ago

Sorry that I was gone for sometime again. Is there something I can do to get this pull request further to acceptance?

PuercoPop commented 7 years ago

@libre-man I could merge this PR right away but I wanted to try out the suitability of the API by implementing djula as a plugin first. That is where I ran into trouble. The plugin mostly works but in order to make it work I would have to do away with djula's base template feature, one of its main selling points.

The blocker with the djula plugin is described in further detail here. The gist of it is that the rendering strategy of Coleslaw makes the assumption of 'nesting' templates (ie. having templates call other templates) instead of the djula approach of having a base layout and having templates extend it (which is a cleaner design imho fwiw). We need to find a way to abstract over that.