• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-0.10.x (829a19c)