plone / diazo

Diazo applies a static HTML theme to a dynamic website
http://diazo.org
Other
41 stars 26 forks source link

Absolute prefix support for srcset attributes #38

Closed huubbouma closed 9 years ago

huubbouma commented 9 years ago

Hi,

I had an issue with srcset attributes in my theme which had relative URLs. I needed to have them converted to absolute URLs like normal image src attributes.

Cheers, Huub

seocam commented 9 years ago

Hello @huubbouma! I'm not sure if the re.sub inside the loop. RE's are usually heavy and they are not compiled in that case.

Isn't it possible to do all the replaces using just one pre-compiled RE? If not I would rather do a split, store everything into a list and join it all in the end. Perhaps you could do some benchmarks. ;)

huubbouma commented 9 years ago

Fair enough. I don't wanna go through the effort of doing the benchmarks, but the simplified regex is ok. will fix it soon

huubbouma commented 9 years ago

Can someone please consider merging this pull request? I've used it in production, added the unit tests, and merged with upstream so it's ready to be merged.

lrowe commented 9 years ago

This feature looks fine, but the history has got a bit overcomplicated now. Please submit the unrelated documentation changes separately and squash and rebase just the srcset changes here.

huubbouma commented 9 years ago

Ik rebased so the pull request is cleaned up again @lrowe