Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

compilescss does not support jinja2 #49

Closed
AndreasBackx opened this issue Sep 13, 2016 · 10 comments · Fixed by #61
Closed

compilescss does not support jinja2 #49

AndreasBackx opened this issue Sep 13, 2016 · 10 comments · Fixed by #61

Comments

@AndreasBackx
Copy link
Collaborator

AndreasBackx commented Sep 13, 2016 •
edited
Loading

I wanted to add support for the compilescss command because currently the command does not support jinja2. There have been a lot of changes in django-compressor's compress command which added support for jinja2 since the compilescss here was written based on that one. I think we/I should refactor the command and inherit from the compress command. This way we can follow the changes there. The current command on django-compressor is very messy, I think it's best to refactor that on their end first and then inherit it here, but add some changes so instead of compressing it would compile SCSS.

It might also be good to add to the readme that setting SASS_PROCESSOR_ENABLED is the only way to use Jinja2 with DEBUG set to False.
@jrief
Copy link
Owner

jrief commented Sep 13, 2016

But then django-compressor becomes a hard dependency. Currently that's optional.

The most important feature now, are unit tests. When I started, I was in a real hurry and that project was not intended to become what it is now. Normally I add unit tests right from the beginning.

@AndreasBackx
Copy link
Collaborator Author

Currently the readme states:

django-compressor is required only for offline compilation, when using the command manage.py compilescss.

Won't it still be 'optional' in the same way (only required for offline use)? We're currently importing it in compilescss.py anyhow.

@frostbtn
Copy link
Contributor

@AndreasBackx I think I've added --engine jinja2 switch a year ago just "because it was easy". Is it not working anymore? (I'm not using jinja2 at the moment but thinking about it).

And yeah, django-compressor is actually not a very good thing to depend on - they have history of making breaking changes in their code with no good reason (like #45, it's still open with no reaction from their side). Maybe the better idea is right opposite - remove this dependency altogether. Just copy their django template walker and be done with it?

@AndreasBackx
Copy link
Collaborator Author

@frostbtn it wasn't working for me. It's been about 2 weeks since I looked into it so I cannot really remember it. As I'm currently no longer using a Jinja2 environment, but am using django-jinja I cannot check what caused it. But in any case support for django-jinja should perhaps also be added.

I would still say that we should depend on it as most of our users would benefit from using django-compressor and are perhaps already using it like I am. We could always keep the version locked to a specific version in case of a breaking change and make it work later on. I doubt there are gonna be a lot of big breaking changes.

@jrief
Copy link
Owner

jrief commented Feb 6, 2017

can we close this?

@AndreasBackx
Copy link
Collaborator Author

@jrief I'll give it a look the coming weeks. Currently pretty busy.

@AndreasBackx
Copy link
Collaborator Author

It's been a while since I looked at the django-compressor part of the code. But could you explain to me why we need the django-compressor? Can't we simply find all of the scss files and compile them so they're available or am I missing something here?

@jrief
Copy link
Owner

jrief commented Mar 4, 2017

In some sense django-compressor and libsass are complementary indeed, since they both compress the CSS files. In addition to that, django-compressor concatenates all CSS files included by your HTML file.

Here I use Sekizai in combination with Compressor. For example:

{% load sekizai_tags %}
...
<head>
   ...
   {% render_block "css" postprocessor "compressor.contrib.sekizai.compress" %}
</head>

If you use SCSS files exclusively and if you use only one SCSS file per HTML file (you can use @import "..."; to achieve the same effect), then compressor is redundant indeed.

@AndreasBackx
Copy link
Collaborator Author

Doesn't the following work?

{% compress css %}
<link rel="stylesheet" href="{% sass_src 'one.scss' %}" type="text/css" charset="utf-8">
<link rel="stylesheet" href="{% sass_src 'two.scss' %}" type="text/css" charset="utf-8">
{% endcompress %}

@jrief
Copy link
Owner

jrief commented Mar 4, 2017

Maybe? I only use django-compress in combination with django-sekizai, hence I don't know.

@AndreasBackx AndreasBackx mentioned this issue May 7, 2017
Merged
3 tasks
AndreasBackx added a commit to AndreasBackx/django-sass-processor that referenced this issue May 7, 2017
@AndreasBackx
Added tests for compilescss management command.
60483ba 
* Resolves jrief#49.
* Fixes failing Python 2.7 Jinja2 tests.
* Multiple engines are now supported in the compilescss command.
* Added more info to README regarding Jinja2 compilescss support.
@jrief jrief closed this as completed in #61 May 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added tests for Jinja2. AndreasBackx/django-sass-processor
3 participants
@jrief @AndreasBackx @frostbtn