clo / cl-site public

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-27.

All preprocess-* functions are now called "around" the original renderer, instead of in separate passes. This approach facilitates passing information between each of the processing steps. The major driver for this change is the need to pass around the original file name, which we need to generate the Edit this page button.

Introduce a way to declare dependencies between pages. That way, we can convert generate-news into a post-processor for news.md, delaying the rendering of index.html until news.md has been generated.

And last but not least: actually add the 'Edit this page' button with a link to our GitLab project!

@dcooper, @mmontone, @vdardel, @tplotnikov: your reviews are highly appreciated! This is quite a big change and I think it's good that we agree on this path forward.

Closes #8.

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-28.

With these additional commits (simplifications), I'd say this branch is ready to be merged. (As ready as it'll ever be...)

Dave Cooper (@dcooper) on GitLab, 2018-10-28.

I have trouble following those huge loops, but that's just because I never use loop myself.

I guess I'd better get used to it because it seems people around here are quite fond of them.

I missed how the context (i.e. file-name) is actually being passed around. Previously I was thinking the computed-page-content dynamic variable could be used for this purpose, by pre-binding it with some info before loading the LHTML pages in preprocess-lisp-pages.

Finaly some nit-picky stuff:

In make-site, why not do:

(defun make-site (&optional (output-dir *output-dir*)) ... )

instead of binding it again with an inner let variable?

In processing.lisp why use setf when setting the value of a symbol, when setq will do and is more targeted/surgical for the purpose? (I always use setq when possible and only use setf when necessary for setting an actual place).

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-28.

Thanks for your review!

Regarding *output-dir* vs output-dir, it was there when we started this recent flurry of activity. I'll change it, but you got a more fundamental point: we seem to ignore the output directory passed in when processing statics and announcing the output directory...

Fix coming up.

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-28.

As for setq vs setf, I've grown used to using setf: it's the generalized version of setq. I wonder: what happens when you setq a place bound by symbol-macrolet?

Anyway, given that we control this code base and we don't use trickery like that, I'll agree with you that setq is fine and probably preferred.

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-28.

Regarding your comment

I missed how the context (i.e. file-name) is actually being passed around.

I think one of my next MRs will be to add (more) documentation about the general structure of what we're doing here. Hoping that will lower the barrier of entry.

Erik Huelsmann (@ehuelsmann) on GitLab, 2018-10-28.

After my latest adjustments of the code (without replacing loop with do), do you have further comments?

My reason to want to use loop over do here is that the init form is also the step form for loop. In case of the rendering driver, we want to initialize with the same value as updating. although I guess dolist with a let* could have been appropriate. For the inner loops, I'm not sure.

Note that I started out with multiple iteration variables, which is hard with dolist (not so hard with do), but very easy with loop. So, that's why it started out that way. As I see the code now, it uses only a single iteration variable on the outer list.