Add custom LessCSS compiler.

Review Request #7982 — Created Feb. 19, 2016 and submitted

Information

Djblets
release-0.10.x

Reviewers

The built-in Less compiler works fairly well, but has a couple issues that were
annoying me.

First of all, in order to cope with an old bug in lessc that no longer exists,
it redirected the output of lessc rather than passed the output filename on the
command line. This made it impossible to use the --source-map argument, which
makes devtools a much nicer experience. I've reimplemented compile_file to
pass in the outfile name on the command-line, and raised the question with the
django-pipeline maintainer to see if we can't get that upstream.

Second, the compiler was completely ignoring the outdated check and always
recompiling every file on every load. This made sense because there was no
dependency analysis, so if an @imported file was changed, the dependent file
would not be recompiled. This was slowing down reloads a fair bit (but it was
still faster than recompiling in the browser).

To fix this second part, I've written a helper, less-imports.js, which uses
the less module's parser to get a deep list of the file imports. These are then
each compared against the target output file's timestamp to see if anything is
out of date. While this is still some overhead (since it still has to parse
the files), it's quite a bit faster than actually recompiling.

  • Loaded and reloaded things a bunch.
  • Made a change to defs.less and saw in the log that it was recompiling all the
    files that imported it.
Description From Last Updated

This file would benefit from doc comments.

brenniebrennie

Quote filenames?

brenniebrennie

Quote filenames?

brenniebrennie

Blank line between these?

brenniebrennie

Can we just build it in place?

brenniebrennie

This should be: os.path.join(os.path.dirname(__file__), '..', '..', '..', 'contrib', 'internal', 'less-imports.js') Otherwise it might explode on Windows.

brenniebrennie

Blank line between these.

brenniebrennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
  2. 
      
david
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
  2. 
      
brennie
  1. 
      
  2. contrib/internal/less-imports.js (Diff revision 2)
     
     
    Show all issues

    This file would benefit from doc comments.

  3. contrib/internal/less-imports.js (Diff revision 2)
     
     
    Show all issues

    Quote filenames?

  4. contrib/internal/less-imports.js (Diff revision 2)
     
     
    Show all issues

    Quote filenames?

  5. contrib/internal/less-imports.js (Diff revision 2)
     
     
     
    Show all issues

    Blank line between these?

  6. djblets/pipeline/compilers/less.py (Diff revision 2)
     
     
     
     
     
     
     
     
    Show all issues

    Can we just build it in place?

  7. djblets/pipeline/compilers/less.py (Diff revision 2)
     
     
     
    Show all issues

    This should be:

    os.path.join(os.path.dirname(__file__),
                 '..', '..', '..', 'contrib', 'internal',
                 'less-imports.js')
    

    Otherwise it might explode on Windows.

  8. djblets/pipeline/compilers/less.py (Diff revision 2)
     
     
     
    Show all issues

    Blank line between these.

  9. 
      
david
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        djblets/pipeline/compilers/less.py
    
    Ignored Files:
        contrib/internal/less-imports.js
    
    
  2. 
      
brennie
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-0.10.x (829a19c)
Loading...