peterbe / premailer

Turns CSS blocks into style attributes
https://premailer.io
BSD 3-Clause "New" or "Revised" License
1.06k stars 188 forks source link

Add preserve_handlebar_syntax option #252

Closed CraigRobertWhite closed 3 years ago

CraigRobertWhite commented 3 years ago

Resolves #248

Description

Added

Motivations

Additional Context

I really enjoy working with this library and would appreciate any feedback because I am using this library in other projects.

coveralls commented 3 years ago

Coverage Status

Coverage remained the same at 0.0% when pulling 8426f81e1e8200ac7ac80584036ea3f8c46fa799 on CraigRobertWhite:handlebar-preservation into 4dc9e63bb5180faf4cfa7ed3d07d8b9eb18dd010 on peterbe:master.

peterbe commented 3 years ago

The TravisCI testing has stopped working. We need to switch to GitHub Actions. I started https://github.com/peterbe/premailer/pull/253

CraigRobertWhite commented 3 years ago

@peterbe https://github.com/peterbe/premailer/pull/252/commits/b470e16821954a2326c47bcf73f7e7ce354bbfc8

CraigRobertWhite commented 3 years ago

I forgot that this package supported Python versions pre-walrus. Going to fix asap. Sorry about that.

peterbe commented 3 years ago

Yeah, I knew that was going to happen :) The walrus syntax is neat and this new feature you're adding is opt-in but from a quick skim, if you can just rewrite in a "old school" way it would alleviate the problem for another couple of years.

CraigRobertWhite commented 3 years ago

@peterbe I'm not crazy about this implementation without the walrus operator but it works. If you have any room for improvement, I'd definitely be willing to implement it. Also, I'm also totally cool with squash merging this PR, haha.

CraigRobertWhite commented 3 years ago

Sorry about the multiple commits for fixing formatting.

peterbe commented 3 years ago

Hey, as a general comment the tests look great (apart from the note about making the 2 unit tests into 1). I don't know if I understand the unquote and unescape and all that but the tests clearly prove that what comes out of the other end is sane and desired. I expected it to be a bit weird because HTML + Handlebar isn't HTML so hacking around it is the only sensible option. I've seen it many times with Django Templates and Jinja templates. For example, some IDEs try to syntax highlight it as HTML when it's actually something entirely different.

If you can just heed the refactoring nit about the unit tests and write some why-comments about the stuff before and after .tostring() then I'll approve and make a release.