From e59c593bbc49b9b318c4ee7c837ee95dbc787694 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Fri, 25 May 2012 16:24:42 -0700 Subject: [PATCH 1/7] Properly indent nested empty directives. --- lib/sass/tree/visitors/to_css.rb | 7 ++++--- test/sass/engine_test.rb | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/lib/sass/tree/visitors/to_css.rb b/lib/sass/tree/visitors/to_css.rb index 6778b5b049..13364b591a 100644 --- a/lib/sass/tree/visitors/to_css.rb +++ b/lib/sass/tree/visitors/to_css.rb @@ -68,13 +68,14 @@ def visit_comment(node) def visit_directive(node) was_in_directive = @in_directive - return node.value + ";" unless node.has_children - return node.value + " {}" if node.children.empty? + tab_str = ' ' * @tabs + return tab_str + node.value + ";" unless node.has_children + return tab_str + node.value + " {}" if node.children.empty? @in_directive = @in_directive || !node.is_a?(Sass::Tree::MediaNode) result = if node.style == :compressed "#{node.value}{" else - "#{' ' * @tabs}#{node.value} {" + (node.style == :compact ? ' ' : "\n") + "#{tab_str}#{node.value} {" + (node.style == :compact ? ' ' : "\n") end was_prop = false first = true diff --git a/test/sass/engine_test.rb b/test/sass/engine_test.rb index aad0df0ba7..71dfc253a6 100755 --- a/test/sass/engine_test.rb +++ b/test/sass/engine_test.rb @@ -2492,6 +2492,22 @@ def test_comment_like_selector SASS end + def test_nested_empty_directive + assert_equal < Date: Fri, 18 May 2012 17:34:43 -0700 Subject: [PATCH 2/7] Add tests for extending within a directive. --- test/sass/extend_test.rb | 198 ++++++++++++++++++++++++++++++++++++--- 1 file changed, 187 insertions(+), 11 deletions(-) diff --git a/test/sass/extend_test.rb b/test/sass/extend_test.rb index f3edbefcba..1a6cdf850f 100755 --- a/test/sass/extend_test.rb +++ b/test/sass/extend_test.rb @@ -1369,13 +1369,13 @@ def test_control_flow_while SCSS end - - def test_extend_in_media + def test_extend_out_of_media assert_warning(< Date: Fri, 25 May 2012 15:48:50 -0700 Subject: [PATCH 3/7] Add a "subsequence?" helper to Util. --- lib/sass/util.rb | 17 +++++++++++++++++ test/sass/util_test.rb | 10 ++++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/sass/util.rb b/lib/sass/util.rb index ae1293f1f2..bc436ab7cd 100644 --- a/lib/sass/util.rb +++ b/lib/sass/util.rb @@ -253,6 +253,23 @@ def check_range(name, range, value, unit='') "#{name} #{str} must be between #{range.first}#{unit} and #{range.last}#{unit}") end + # Returns whether or not `seq1` is a subsequence of `seq2`. That is, whether + # or not `seq2` contains every element in `seq1` in the same order (and + # possibly more elements besides). + # + # @param seq1 [Array] + # @param seq2 [Array] + # @return [Boolean] + def subsequence?(seq1, seq2) + i = j = 0 + loop do + return true if i == seq1.size + return false if j == seq2.size + i += 1 if seq1[i] == seq2[j] + j += 1 + end + end + # Returns information about the caller of the previous method. # # @param entry [String] An entry in the `#caller` list, or a similarly formatted string diff --git a/test/sass/util_test.rb b/test/sass/util_test.rb index 0b86a29c68..954fc01f27 100755 --- a/test/sass/util_test.rb +++ b/test/sass/util_test.rb @@ -120,6 +120,16 @@ def test_lcs_with_block lcs([-5, 3, 2, 8], [-4, 1, 8]) {|a, b| (a - b).abs <= 1 && [a, b].max}) end + def test_subsequence + assert(subsequence?([1, 2, 3], [1, 2, 3])) + assert(subsequence?([1, 2, 3], [1, :a, 2, :b, 3])) + assert(subsequence?([1, 2, 3], [:a, 1, :b, :c, 2, :d, 3, :e, :f])) + + assert(!subsequence?([1, 2, 3], [1, 2])) + assert(!subsequence?([1, 2, 3], [1, 3, 2])) + assert(!subsequence?([1, 2, 3], [3, 2, 1])) + end + def test_silence_warnings old_stderr, $stderr = $stderr, StringIO.new warn "Out" From d6e8cd47f5151a3d292f2e350bb86501d80ea070 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Fri, 25 May 2012 16:15:51 -0700 Subject: [PATCH 4/7] Get rid of the old @extend-within-directive warnings. --- lib/sass/tree/extend_node.rb | 12 ------------ lib/sass/tree/visitors/check_nesting.rb | 12 ------------ lib/sass/tree/visitors/cssize.rb | 1 - 3 files changed, 25 deletions(-) diff --git a/lib/sass/tree/extend_node.rb b/lib/sass/tree/extend_node.rb index ea1affc2f8..104f7660b3 100644 --- a/lib/sass/tree/extend_node.rb +++ b/lib/sass/tree/extend_node.rb @@ -25,17 +25,5 @@ def initialize(selector) @selector = selector super() end - - # Disables this `@extend` due to it being inside a directive. - def disable! - @disabled = true - end - - # Whether this `@extend` is disabled due to it being inside a directive. - # - # @return [Boolean] - def disabled? - @disabled - end end end diff --git a/lib/sass/tree/visitors/check_nesting.rb b/lib/sass/tree/visitors/check_nesting.rb index 3f177f606b..08483b47b0 100644 --- a/lib/sass/tree/visitors/check_nesting.rb +++ b/lib/sass/tree/visitors/check_nesting.rb @@ -54,18 +54,6 @@ def invalid_extend_parent?(parent, child) unless is_any_of?(parent, VALID_EXTEND_PARENTS) return "Extend directives may only be used within rules." end - - if !child.disabled? && - directive = @parents.find {|p| p.is_a?(Sass::Tree::DirectiveNode)} - child.disable! - Sass::Util.sass_warn < 1 raise Sass::SyntaxError.new("Can't extend #{seq.to_a.join}: can't extend nested selectors") From 5abebb27656661e50b00f10497d96e1cea6a0f98 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Fri, 25 May 2012 16:16:39 -0700 Subject: [PATCH 5/7] Restrict @extend within directives. --- lib/sass/engine.rb | 1 + lib/sass/selector/comma_sequence.rb | 8 ++++-- lib/sass/selector/sequence.rb | 6 ++-- lib/sass/selector/simple_sequence.rb | 25 +++++++++++++++-- lib/sass/tree/node.rb | 22 --------------- lib/sass/tree/root_node.rb | 2 +- lib/sass/tree/rule_node.rb | 9 ------ lib/sass/tree/visitors/cssize.rb | 9 ++++-- lib/sass/tree/visitors/extend.rb | 42 ++++++++++++++++++++++++++++ 9 files changed, 83 insertions(+), 41 deletions(-) create mode 100644 lib/sass/tree/visitors/extend.rb diff --git a/lib/sass/engine.rb b/lib/sass/engine.rb index ce40aed06d..87d50c467a 100644 --- a/lib/sass/engine.rb +++ b/lib/sass/engine.rb @@ -25,6 +25,7 @@ require 'sass/tree/visitors/base' require 'sass/tree/visitors/perform' require 'sass/tree/visitors/cssize' +require 'sass/tree/visitors/extend' require 'sass/tree/visitors/convert' require 'sass/tree/visitors/to_css' require 'sass/tree/visitors/deep_copy' diff --git a/lib/sass/selector/comma_sequence.rb b/lib/sass/selector/comma_sequence.rb index c7a548eecb..08229bbc65 100644 --- a/lib/sass/selector/comma_sequence.rb +++ b/lib/sass/selector/comma_sequence.rb @@ -47,10 +47,14 @@ def resolve_parent_refs(super_cseq) # @param extends [Sass::Util::SubsetMap{Selector::Simple => # Sass::Tree::Visitors::Cssize::Extend}] # The extensions to perform on this selector + # @param parent_directives [Array] + # The directives containing this selector. # @return [CommaSequence] A copy of this selector, # with extensions made according to `extends` - def do_extend(extends) - CommaSequence.new(members.map {|seq| seq.do_extend(extends)}.flatten) + def do_extend(extends, parent_directives) + CommaSequence.new(members.map do |seq| + seq.do_extend(extends, parent_directives) + end.flatten) end # Returns a string representation of the sequence. diff --git a/lib/sass/selector/sequence.rb b/lib/sass/selector/sequence.rb index 8f226a1fc7..ddf6cf3f98 100644 --- a/lib/sass/selector/sequence.rb +++ b/lib/sass/selector/sequence.rb @@ -72,14 +72,16 @@ def resolve_parent_refs(super_seq) # @param extends [Sass::Util::SubsetMap{Selector::Simple => # Sass::Tree::Visitors::Cssize::Extend}] # The extensions to perform on this selector + # @param parent_directives [Array] + # The directives containing this selector. # @return [Array] A list of selectors generated # by extending this selector with `extends`. # These correspond to a {CommaSequence}'s {CommaSequence#members members array}. # @see CommaSequence#do_extend - def do_extend(extends, seen = Set.new) + def do_extend(extends, parent_directives, seen = Set.new) paths = Sass::Util.paths(members.map do |sseq_or_op| next [[sseq_or_op]] unless sseq_or_op.is_a?(SimpleSequence) - extended = sseq_or_op.do_extend(extends, seen) + extended = sseq_or_op.do_extend(extends, parent_directives, seen) choices = extended.map {|seq| seq.members} choices.unshift([sseq_or_op]) unless extended.any? {|seq| seq.superselector?(sseq_or_op)} choices diff --git a/lib/sass/selector/simple_sequence.rb b/lib/sass/selector/simple_sequence.rb index 63007cc7f2..03b9407647 100644 --- a/lib/sass/selector/simple_sequence.rb +++ b/lib/sass/selector/simple_sequence.rb @@ -54,24 +54,28 @@ def resolve_parent_refs(super_seq) # Non-destrucively extends this selector with the extensions specified in a hash # (which should come from {Sass::Tree::Visitors::Cssize}). # - # @overload def do_extend(extends) + # @overload def do_extend(extends, parent_directives) # @param extends [{Selector::Simple => # Sass::Tree::Visitors::Cssize::Extend}] # The extensions to perform on this selector + # @param parent_directives [Array] + # The directives containing this selector. # @return [Array] A list of selectors generated # by extending this selector with `extends`. # @see CommaSequence#do_extend - def do_extend(extends, seen = Set.new) + def do_extend(extends, parent_directives, seen = Set.new) extends.get(members.to_set).map do |ex, sels| # If A {@extend B} and C {...}, # ex.extender is A, sels is B, and self is C self_without_sel = self.members - sels next unless unified = ex.extender.members.last.unify(self_without_sel) + next unless check_directives_match!(ex, parent_directives) [sels, ex.extender.members[0...-1] + [unified]] end.compact.map do |sels, seq| seq = Sequence.new(seq) - seen.include?(sels) ? [] : seq.do_extend(extends, seen + [sels]) + next [] if seen.include?(sels) + seq.do_extend(extends, parent_directives, seen + [sels]) end.flatten.uniq end @@ -123,6 +127,21 @@ def inspect private + def check_directives_match!(extend, parent_directives) + dirs1 = extend.directives.map {|d| d.value} + dirs2 = parent_directives.map {|d| d.value} + return true if Sass::Util.subsequence?(dirs1, dirs2) + + Sass::Util.sass_warn < - # Sass::Tree::Visitors::Cssize::Extend}] - # The extensions to perform on this tree - # @return [Tree::Node] The resulting tree of static CSS nodes. - # @raise [Sass::SyntaxError] Only if there's a programmer error - # and this is not a static CSS tree - def do_extend(extends) - node = dup - node.children = children.map {|c| c.do_extend(extends)} - node - rescue Sass::SyntaxError => e - e.modify_backtrace(:filename => filename, :line => line) - raise e - end - # Iterates through each node in the tree rooted at this node # in a pre-order walk. # diff --git a/lib/sass/tree/root_node.rb b/lib/sass/tree/root_node.rb index f50fd1b42f..9a04ad2de4 100644 --- a/lib/sass/tree/root_node.rb +++ b/lib/sass/tree/root_node.rb @@ -20,7 +20,7 @@ def render result = Visitors::Perform.visit(self) Visitors::CheckNesting.visit(result) # Check again to validate mixins result, extends = Visitors::Cssize.visit(result) - result = result.do_extend(extends) unless extends.empty? + Visitors::Extend.visit(result, extends) result.to_s end end diff --git a/lib/sass/tree/rule_node.rb b/lib/sass/tree/rule_node.rb index d7245401a9..740524a3fc 100644 --- a/lib/sass/tree/rule_node.rb +++ b/lib/sass/tree/rule_node.rb @@ -103,15 +103,6 @@ def continued? last.is_a?(String) && last[-1] == ?, end - # Extends this Rule's selector with the given `extends`. - # - # @see Node#do_extend - def do_extend(extends) - node = dup - node.resolved_rules = resolved_rules.do_extend(extends) - node - end - # A hash that will be associated with this rule in the CSS document # if the {file:SASS_REFERENCE.md#debug_info-option `:debug_info` option} is enabled. # This data is used by e.g. [the FireSass Firebug extension](https://addons.mozilla.org/en-US/firefox/addon/103988). diff --git a/lib/sass/tree/visitors/cssize.rb b/lib/sass/tree/visitors/cssize.rb index 5a29921763..69e8d662c8 100644 --- a/lib/sass/tree/visitors/cssize.rb +++ b/lib/sass/tree/visitors/cssize.rb @@ -12,6 +12,7 @@ def self.visit(root); super; end attr_reader :parent def initialize + @parent_directives = [] @extends = Sass::Util::SubsetMap.new end @@ -38,9 +39,11 @@ def visit_children(parent) # @yield A block in which the parent is set to `parent`. # @return [Object] The return value of the block. def with_parent(parent) + @parent_directives.push parent if parent.is_a?(Sass::Tree::DirectiveNode) old_parent, @parent = @parent, parent yield ensure + @parent_directives.pop if parent.is_a?(Sass::Tree::DirectiveNode) @parent = old_parent end @@ -88,7 +91,9 @@ def visit_root(node) # The selector of the CSS rule containing the `@extend`. # @attr target [Array] The selector being `@extend`ed. # @attr node [Sass::Tree::ExtendNode] The node that produced this extend. - Extend = Struct.new(:extender, :target, :node) + # @attr directives [Array] + # The directives containing the `@extend`. + Extend = Struct.new(:extender, :target, :node, :directives) # Registers an extension in the `@extends` subset map. def visit_extend(node) @@ -108,7 +113,7 @@ def visit_extend(node) raise Sass::SyntaxError.new("#{seq} can't extend: invalid selector") end - @extends[sel] = Extend.new(seq, sel, node) + @extends[sel] = Extend.new(seq, sel, node, @parent_directives.dup) end end diff --git a/lib/sass/tree/visitors/extend.rb b/lib/sass/tree/visitors/extend.rb new file mode 100644 index 0000000000..8829054067 --- /dev/null +++ b/lib/sass/tree/visitors/extend.rb @@ -0,0 +1,42 @@ +# A visitor for performing selector inheritance on a static CSS tree. +# +# Destructively modifies the tree. +class Sass::Tree::Visitors::Extend < Sass::Tree::Visitors::Base + # @param root [Tree::Node] The root node of the tree to visit. + # @param extends [Sass::Util::SubsetMap{Selector::Simple => + # Sass::Tree::Visitors::Cssize::Extend}] + # The extensions to perform on this tree. + # @return [Object] The return value of \{#visit} for the root node. + def self.visit(root, extends) + return if extends.empty? + new(extends).send(:visit, root) + end + + protected + + def initialize(extends) + @parent_directives = [] + @extends = extends + end + + # If an exception is raised, this adds proper metadata to the backtrace. + def visit(node) + super(node) + rescue Sass::SyntaxError => e + e.modify_backtrace(:filename => node.filename, :line => node.line) + raise e + end + + # Keeps track of the current parent directives. + def visit_children(parent) + @parent_directives.push parent if parent.is_a?(Sass::Tree::DirectiveNode) + super + ensure + @parent_directives.pop if parent.is_a?(Sass::Tree::DirectiveNode) + end + + # Applies the extend to a single rule's selector. + def visit_rule(node) + node.resolved_rules = node.resolved_rules.do_extend(@extends, @parent_directives) + end +end From 7608f2ba5879e1ad840f4c59c700b7cb549abaa1 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Fri, 25 May 2012 16:41:35 -0700 Subject: [PATCH 6/7] Document new @extend-in-directive semantics. --- doc-src/SASS_CHANGELOG.md | 2 ++ doc-src/SASS_REFERENCE.md | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/doc-src/SASS_CHANGELOG.md b/doc-src/SASS_CHANGELOG.md index 1749d67db6..cf8097eb0b 100644 --- a/doc-src/SASS_CHANGELOG.md +++ b/doc-src/SASS_CHANGELOG.md @@ -7,6 +7,8 @@ * Fix an `uninitialized constant Sass::Exec::Sass::Util` error when using the command-line tool. +* Allow `@extend` within directives such as `@media` as long as it only extends + selectors that are within the same directive. ## 3.1.18 diff --git a/doc-src/SASS_REFERENCE.md b/doc-src/SASS_REFERENCE.md index 341892bdde..4c6ddd6b6f 100644 --- a/doc-src/SASS_REFERENCE.md +++ b/doc-src/SASS_REFERENCE.md @@ -1461,10 +1461,40 @@ This is compiled to: #### `@extend` in Directives -Unfortunately, `@extend` cannot be used within directives such as `@media`. Sass -is unable to make CSS rules outside of the `@media` block apply to selectors -inside it without creating a huge amount of stylesheet bloat by copying styles -all over the place. +There are some restrictions on the use of `@extend` within directives such as +`@media`. Sass is unable to make CSS rules outside of the `@media` block apply +to selectors inside it without creating a huge amount of stylesheet bloat by +copying styles all over the place. This means that if you use `@extend` within +`@media` (or other CSS directives), you may only extend selectors that appear +within the same directive block. + +For example, the following works fine: + + @media print { + .error { + border: 1px #f00; + background-color: #fdd; + } + .seriousError { + @extend .error; + border-width: 3px; + } + } + +But this is an error: + + .error { + border: 1px #f00; + background-color: #fdd; + } + + @media print { + .seriousError { + // INVALID EXTEND: .error is used outside of the "@media print" directive + @extend .error; + border-width: 3px; + } + } Someday we hope to have `@extend` supported natively in the browser, which will allow it to be used within `@media` and other directives. From 95c279cfe24aed101b84f7a5a1f324f7f45740c4 Mon Sep 17 00:00:00 2001 From: Nathan Weizenbaum Date: Fri, 25 May 2012 16:43:00 -0700 Subject: [PATCH 7/7] Plan to make @extend-within-directive an error in 3.3, not 3.2. 3.2 is too close to release to break backwards-compat. --- lib/sass/selector/simple_sequence.rb | 2 +- test/sass/extend_test.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/sass/selector/simple_sequence.rb b/lib/sass/selector/simple_sequence.rb index 03b9407647..ab69adad52 100644 --- a/lib/sass/selector/simple_sequence.rb +++ b/lib/sass/selector/simple_sequence.rb @@ -136,7 +136,7 @@ def check_directives_match!(extend, parent_directives) DEPRECATION WARNING on line #{extend.node.line}#{" of #{extend.node.filename}" if extend.node.filename}: @extending an outer selector from within #{extend.directives.last.name} is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARNING return false diff --git a/test/sass/extend_test.rb b/test/sass/extend_test.rb index 1a6cdf850f..b926efa897 100755 --- a/test/sass/extend_test.rb +++ b/test/sass/extend_test.rb @@ -1374,7 +1374,7 @@ def test_extend_out_of_media DEPRECATION WARNING on line 3 of test_extend_out_of_media_inline.sass: @extending an outer selector from within @media is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN .foo { @@ -1392,7 +1392,7 @@ def test_extend_out_of_unknown_directive DEPRECATION WARNING on line 3 of test_extend_out_of_unknown_directive_inline.sass: @extending an outer selector from within @flooblehoof is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN .foo { @@ -1412,7 +1412,7 @@ def test_extend_out_of_nested_directives DEPRECATION WARNING on line 4 of test_extend_out_of_nested_directives_inline.sass: @extending an outer selector from within @flooblehoof is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN @media screen { @@ -1515,7 +1515,7 @@ def test_extend_within_and_without_media DEPRECATION WARNING on line 4 of test_extend_within_and_without_media_inline.sass: @extending an outer selector from within @media is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN .foo { @@ -1538,7 +1538,7 @@ def test_extend_within_and_without_unknown_directive DEPRECATION WARNING on line 4 of test_extend_within_and_without_unknown_directive_inline.sass: @extending an outer selector from within @flooblehoof is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN .foo { @@ -1561,7 +1561,7 @@ def test_extend_within_and_without_nested_directives DEPRECATION WARNING on line 5 of test_extend_within_and_without_nested_directives_inline.sass: @extending an outer selector from within @flooblehoof is deprecated. You may only @extend selectors within the same directive. - This will be an error in Sass 3.2. + This will be an error in Sass 3.3. It can only work once @extend is supported natively in the browser. WARN @media screen {