cryogen-project / cryogen-core

Cryogen's core
Eclipse Public License 1.0
69 stars 62 forks source link

Proposal: Replace :content with :content-dom #131

Closed holyjak closed 4 years ago

holyjak commented 4 years ago

Currently articles have their text under :content, which we parse repeatedly in create-preview and add-description.

It would be more efficient to parse the content into an enlive tree once in page-content and include it under :content-nodes (or perhaps better :content-dom ?) and use that at all the other places. Then we would emit* + str it at the very end, right before we call render-file on it.

This is a breaking change but until 0.2.1 and its extend-params-fn there was no official way to access it and even this was not primarily intended to give access to the content (contrary to the upcoming #130).

(We would ideally rewrite content-until-more-marker to use the dom instead.)

What do you think, @lacarmen ?

lacarmen commented 4 years ago

Just rephrasing so I can make sure I understand what you're saying:

Instead of parsing the html string :content in a bunch of places, let's pass around an enlive tree :content-dom. Then we can use the enlive tree to do the previews/description injection stuff and whatever else we want to derive from the page content (using the new :update-article-fn).

Finally, just before we render the templates, we convert :content-dom into :content by emitting the html str. Is that correct?

This is a breaking change

Is it really a breaking change though? We'd be changing up the internals but as far as I understand, nothing should be changing at the API level.

holyjak commented 4 years ago

Exactly!

lacarmen commented 4 years ago

Ok, sounds good! This will give more power to :update-article-fn as well :)