v2.0.1

lynette1022 · Dec 23, 2013 · cac347b · cac347b
2 parents 83112bd + 89a7f8a
commit cac347b
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 5 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,8 @@
 # Changelog
 
+## 2.0.1 - 2013-12-23
+- Fix for `remove-empty-rulesets` option (#133)
+
 ## 2.0.0 - 2013-12-18
 **Great thanks for @tonyganch and @kizu!**
 - Use Gonzales PE to parse *.scss and *.less files

diff --git a/README.md b/README.md
@@ -522,7 +522,7 @@ p[href^='https://']:before { content: 'secure' }
 
 Acceptable values: `{Boolean}` `true`
 
-Example: `{ "remove-empty-rulesets": true }` - remove rulesets that have no declarations or comments.
+Example: `{ "remove-empty-rulesets": true }` - remove rulesets that have nothing but spaces.
 
 `a { color: red; } p { /* hey */ } b { }` &rarr; `a { color: red; } p { /* hey */ } `
 

diff --git a/lib/options/remove-empty-rulesets.js b/lib/options/remove-empty-rulesets.js
@@ -32,8 +32,26 @@ module.exports = {
 
     _removeEmptyRulesets: function(nodeContent) {
         var i = nodeContent.length;
+        // Loop through node and try to find a ruleset:
         while (i--) {
-            if (this._isRuleset(nodeContent[i]) && this._isEmptyRuleset(nodeContent[i])) {
+            var node = nodeContent[i];
+            if (!this._isRuleset(node)) continue;
+            // If a ruleset is found, try to find its nested rulesets and remove
+            // all empty ones:
+            var j = node.length;
+            while (j--) {
+                // Nested rulesets are located inside blocks, that's why look
+                // for blocks only:
+                var blockNode = node[j];
+                if (blockNode[0] !== 'block') continue;
+                blockNode.shift();
+                this._processStylesheetContent(blockNode);
+                blockNode.unshift('block');
+                node[j] = blockNode;
+            }
+            // If after removing all empty nested rulesets the parent has also
+            // become empty, remove it too:
+            if (this._isEmptyRuleset(node)) {
                 nodeContent.splice(i, 1);
             }
         }
@@ -60,10 +78,10 @@ module.exports = {
     },
 
     /**
-     * Block considered empty when it has no declarations or comments.
+     * Block is considered empty when it has nothing but spaces.
      */
     _isEmptyBlock: function(node) {
-        return !node.some(this._isDeclarationOrComment);
+        return node.length === 1 || !node.some(this._isNotWhitespace);
     },
 
     _isDeclarationOrComment: function(node) {
@@ -82,6 +100,10 @@ module.exports = {
         return node[0] === 's';
     },
 
+    _isNotWhitespace: function(node) {
+        return typeof node === 'object' && node[0] !== 's';
+    },
+
     /**
      * Detects the value of an option at the tree node.
      * This option is treated as `true` by default, but any trailing space would invalidate it.

diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
     "name": "csscomb",
     "description": "CSS coding style formatter",
-    "version": "2.0.0",
+    "version": "2.0.1",
     "homepage": "http://csscomb.com/",
     "author": "Mikhail Troshev <[email protected]>",
     "repository": "https://github.com/csscomb/csscomb.js",

diff --git a/test/remove-empty-rulesets-scss.js b/test/remove-empty-rulesets-scss.js
@@ -0,0 +1,35 @@
+var Comb = require('../lib/csscomb');
+var assert = require('assert');
+var fs = require('fs');
+
+describe('options/remove-empty-rulesets (scss)', function() {
+    var comb = new Comb({ 'remove-empty-rulesets': true });
+    var input;
+    var expected;
+
+    function readFile(path) {
+        return fs.readFileSync('test/remove-empty-rulesets-scss/' + path, 'utf8');
+    }
+
+    it('Should not remove rulesets which contain only includes', function() {
+        input = readFile('include.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), input);
+    });
+
+    it('Should remove rulesets with contain only empty nested rules', function() {
+        input = readFile('empty-nested-rule.scss', 'utf-8');
+        expected = readFile('empty-nested-rule.expected.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), expected);
+    });
+
+    it('Should not remove rulesets with non-empty nested rules. Test 1', function() {
+        input = readFile('nested-rule-1.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), input);
+    });
+
+    it('Should not remove rulesets with non-empty nested rules. Test 2', function() {
+        input = readFile('nested-rule-2.scss', 'utf-8');
+        expected = readFile('nested-rule-2.expected.scss', 'utf-8');
+        assert.equal(comb.processString(input, 'scss'), expected);
+    });
+});
diff --git a/test/remove-empty-rulesets-scss/empty-nested-rule.expected.scss b/test/remove-empty-rulesets-scss/empty-nested-rule.expected.scss
@@ -0,0 +1 @@
+
diff --git a/test/remove-empty-rulesets-scss/empty-nested-rule.scss b/test/remove-empty-rulesets-scss/empty-nested-rule.scss
@@ -0,0 +1,5 @@
+.parent {
+    .child {
+        .grandchild { }
+    }
+}
diff --git a/test/remove-empty-rulesets-scss/include.scss b/test/remove-empty-rulesets-scss/include.scss
@@ -0,0 +1,4 @@
+.parent {
+    @include mix-all;
+    @include mix-top;
+}
diff --git a/test/remove-empty-rulesets-scss/nested-rule-1.scss b/test/remove-empty-rulesets-scss/nested-rule-1.scss
@@ -0,0 +1,6 @@
+.parent {
+    .child {
+        @include mix-all;
+        @include mix-top;
+    }
+}
diff --git a/test/remove-empty-rulesets-scss/nested-rule-2.expected.scss b/test/remove-empty-rulesets-scss/nested-rule-2.expected.scss
@@ -0,0 +1,7 @@
+.parent {
+    .child1 {
+        @include mix-all;
+        @include mix-top;
+    }
+
+}
diff --git a/test/remove-empty-rulesets-scss/nested-rule-2.scss b/test/remove-empty-rulesets-scss/nested-rule-2.scss
@@ -0,0 +1,9 @@
+.parent {
+    .child1 {
+        @include mix-all;
+        @include mix-top;
+    }
+    .child2 {
+        .grandchild { }
+    }
+}
diff --git a/test/remove-empty-rulesets.js b/test/remove-empty-rulesets.js
@@ -27,6 +27,10 @@ describe('options/remove-empty-rulesets', function() {
             assert.equal(comb.processString(' b {} '), '  ');
         });
 
+        it('should remove ruleset with spaces', function() {
+            assert.equal(comb.processString(' b {   } '), '  ');
+        });
+
         it('should leave ruleset with declarations', function() {
             assert.equal(comb.processString('a { width: 10px; }\nb {} '), 'a { width: 10px; }\n ');
         });