ruricolist / spinneret

Common Lisp HTML5 generator
MIT License
369 stars 26 forks source link

Too eager HTML printing for `deftag`ged tags #85

Closed aartaka closed 1 year ago

aartaka commented 1 year ago

I have this smart macro, :ul*, which makes strings (or any forms) inside its body to be wrapped into their own <li>s. But then, if one wants to force a <li>, :ul* should just skip the wrapping and insert it literally. Here's how I've implemented it:

(deftag :ul* (body attrs &key &allow-other-keys)
  "<ul> with every form (except <li>) in BODY auto-wrapped into a <li>."
  `(:ul ,@attrs ,@(loop for form in body
                        when (and (listp form)
                                  (eq :li (first form)))
                          collect form
                        else
                          collect `(:li ,form))))

Which seems absolutely valid. But when I run

(with-html
  (:ul*
   "Item 1"
   "Item 2"
   (:b "Bold item 3")
   (:li "Proper <li> item 4")
   "Item 5"))

it prints

<ul>
 <li>Item 1
 <li>Item 2
 <li><b>Bold item 3</b>
 <li> <!-- Notice the extra <li> here! -->
  <li>Proper &lt;li&gt; item 4
 <li>Item 5
</ul>

So, it seems, the literal :li is somehow printed before the tag macro is expanded. Which seems wrong.

Is it possible to force deftagged macro expansion to happen before the tag printing?

ruricolist commented 1 year ago

What's happening is that with-html is walking the form before deftag gets expanded, so when deftag receives its arguments they've already been wrapped in with-tag forms;

(macroexpand-1 
 '(with-html
   (:ul*
    "Item 1"
    "Item 2"
    (:b "Bold item 3")
    (:li "Proper <li> item 4")
    "Item 5"))'
=>
(let ((*html* (spinneret::ensure-html-stream *html*)))
  (progn
   (:ul*
     "Item 1"
     "Item 2"
     (spinneret::with-tag (:b)
       "Bold item 3")
     (spinneret::with-tag (:li)
       "Proper <li> item 4")
     "Item 5")))

I think the right thing to do here is to have the walker detect that :ul* is defined as a deftag and not descend into it. I've pushed a branch that does this, dont-descend-deftag. It doesn't break any tests but I want to do some more experimentation before I merge it to master.

aartaka commented 1 year ago

Thanks for working on it, you've made my code and website much simpler :black_heart: