Skip to content

Commit d8e5da3

Browse files
committed
Fixes bug where --ignore-sass-mixins isn't respected in @imports
There is a `@children.empty?` edgecase that I can't recreate in the tests but I'm seeing on a private codebase. It would be nice to red/green test that bit of the conditional. refs: zmoazeni#62
1 parent 17c8c68 commit d8e5da3

File tree

4 files changed

+44
-20
lines changed

4 files changed

+44
-20
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 1.3.1 - 4/20/2013 ##
2+
3+
* Fixes --ignore-sass-mixins bug with @importing
4+
15
## 1.3.0 - 4/20/2013 ##
26

37
* Adds --require switch for user configuration

lib/csscss/cli.rb

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -172,16 +172,12 @@ def gem_installed?(gem_name)
172172

173173
def load_sass_file(filename)
174174
abort 'Must install the "sass" gem before parsing sass/scss files' unless gem_installed?("sass")
175+
require "csscss/sass_include_extensions" if @ignore_sass_mixins
175176

176177
sass_options = {cache:false}
177178
sass_options[:load_paths] = Compass.configuration.sass_load_paths if @compass
178179
begin
179-
tree = Sass::Engine.for_file(filename, sass_options).to_tree
180-
if @ignore_sass_mixins
181-
require "csscss/sass_include_extensions"
182-
Csscss::SassMixinVisitor.visit(tree)
183-
end
184-
tree.render
180+
Sass::Engine.for_file(filename, sass_options).render
185181
rescue Sass::SyntaxError => e
186182
if e.message =~ /compass/ && !@compass
187183
puts "Enable --compass option to use compass's extensions"
Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
require "sass"
22

3-
module Csscss
4-
class SassMixinVisitor < Sass::Tree::Visitors::Base
5-
def self.visit(root)
6-
new.send(:visit, root)
7-
end
3+
Sass::Tree::MixinDefNode.class_eval do
4+
def children
5+
first_child = @children.first
86

9-
def visit_mixindef(node)
10-
begin_comment = Sass::Tree::CommentNode.new(["/* CSSCSS START MIXIN: #{node.name} */"], :normal)
11-
end_comment = Sass::Tree::CommentNode.new(["/* CSSCSS END MIXIN: #{node.name} */"], :normal)
7+
# not sure why/how we can get here with empty children, but it
8+
# causes issues
9+
unless @children.empty? || (first_child.is_a?(Sass::Tree::CommentNode) && first_child.value.first =~ /CSSCSS START/)
10+
begin_comment = Sass::Tree::CommentNode.new(["/* CSSCSS START MIXIN: #{name} */"], :normal)
11+
end_comment = Sass::Tree::CommentNode.new(["/* CSSCSS END MIXIN: #{name} */"], :normal)
1212

1313
begin_comment.options = end_comment.options = {}
14-
15-
node.children.unshift(begin_comment)
16-
node.children.push(end_comment)
14+
@children.unshift(begin_comment)
15+
@children.push(end_comment)
1716
end
17+
18+
@children
1819
end
1920
end

test/csscss/sass_include_extensions_test.rb

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require "test_helper"
2+
require "tempfile"
23
require "csscss/sass_include_extensions"
34

45
module Csscss
@@ -37,9 +38,31 @@ module Csscss
3738
/* CSSCSS END MIXIN: bar */ }
3839
CSS
3940

40-
tree = Sass::Engine.new(scss, syntax: :scss).to_tree
41-
Csscss::SassMixinVisitor.visit(tree)
42-
tree.render.must_equal(css)
41+
Sass::Engine.new(scss, syntax: :scss, cache: false).render.must_equal(css)
42+
end
43+
44+
it "should insert comments even with imported stylesheets" do
45+
Tempfile.open(['foo', '.scss']) do |f|
46+
f << <<-SCSS
47+
@mixin foo {
48+
outline: 1px;
49+
}
50+
51+
h1 {
52+
@include foo;
53+
}
54+
SCSS
55+
f.close
56+
57+
css =<<-CSS
58+
h1 {
59+
/* CSSCSS START MIXIN: foo */
60+
outline: 1px;
61+
/* CSSCSS END MIXIN: foo */ }
62+
CSS
63+
64+
Sass::Engine.new("@import '#{f.path}'", syntax: :scss, cache: false).render.must_equal(css)
65+
end
4366
end
4467
end
4568
end

0 commit comments

Comments
 (0)