Skip to content
This repository has been archived by the owner on Aug 17, 2023. It is now read-only.

Add less, sass and escaped filters #55

Merged
merged 4 commits into from
Feb 7, 2017
Merged

Conversation

rowanseymour
Copy link
Member

Also makes some simplifications to filter code and brings us closer to the output from the Ruby Haml implementation

@rowanseymour rowanseymour self-assigned this Feb 6, 2017
# ----------------------------------------------------------------------------------

def plain(text, options):
return text
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently each filter handles indentation separately but now, like the Ruby implementation, indentation is handled outside of the filter.

@@ -9,14 +9,14 @@

should } not be interpreted as a multiline string
:css
.test {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is mostly replacing tabs with spaces

@@ -2,78 +2,77 @@
These { { braces

should } not be interpreted as a multiline string
These { { braces

 should } not be interpreted as a multiline string
These { { braces

should } not be interpreted as a multiline string
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is consistent with how the Haml filter works

// <![CDATA[
// ]]>
//<![CDATA[
\x20\x20\x20\x20
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are just spaces - need to use escapes because otherwise PyCharm removes trailing whitespace

@coveralls
Copy link

coveralls commented Feb 6, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 8e2ec68 on filter_improvements into 75aee7c on master.

@@ -14,6 +14,7 @@ class Compiler:
'django_inline_style': False, # support both #{...} and ={...}
'tag_config': 'django', # Django vs Jinja2 tags
'custom_self_closing_tags': {}, # additional self-closing tags
'cdata': True, # wrap CSS, Javascript etc content in CDATA section
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CDATA usage isn't required in HTML5 so making it optional here like in Ruby Haml

The implementation of these should match https://github.com/haml/haml/blob/master/lib/haml/filters.rb as closely as
possible. Where we differ is that we don't compile Stylus, Coffeescript etc into CSS or Javascript - but place the
content into suitable <style> and <script> that can be transformed later by something like django-compressor.
"""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should consider this.. that is turning :sass blocks into native CSS.

Copy link
Member Author

@rowanseymour rowanseymour Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like I only just about understand how a :coffee section currently ends up as compressed js, but wouldn't that mean that the coffee→js / sass→css compilation has to happen for every request? Is it time we tried using cached templates?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We of course need the compress tag to have sass/less/coffee converted during offline compression, e.g.

- compress js
  :coffee
    foo=bar
- compress css
  :sass
    .mystyle {...}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, I think what I was thinking of was closer to:

-compress css inline
  :sass
    .mystyle {...}

In that you want it converted and inserted directly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure and that means the sass compilation happens every time that template is requested? Maybe that's not a big deal and/or caching of templates works?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well Simpla will def be compressing offline.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant just template caching - to avoid haml→html compilation for every request

Copy link

@nicpottier nicpottier Feb 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well now I'm confused, won't the offline compression do that? Or is that not converting haml to html at that point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that just extracted the js and css? Each request still requires compiling the Haml into html. Now I'm confused

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right.. well sheesh, that seems like something we should fix. I don't even like that happening with per-process caching.

@rowanseymour rowanseymour merged commit 3a951d2 into master Feb 7, 2017
@rowanseymour rowanseymour deleted the filter_improvements branch February 7, 2017 09:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants