Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .csscomb.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"element-case": "lower",
"eof-newline": true,
"leading-zero": false,
"remove-empty-rulesets": true,
"rule-indent": true,
"stick-brace": "\n",
"strip-spaces": true,
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ node_modules

.idea
*.iml

.DS_Store
17 changes: 17 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Example configuration:
"exclude": ["node_modules/**"],
"verbose": true,

"remove-empty-rulesets": true,
"always-semicolon": true,
"block-indent": true,
"colon-space": true,
Expand All @@ -59,6 +60,14 @@ Example configuration:

## Options

### exclude

Available values: `{String[]}`

Array of [Ant path patterns](http://ant.apache.org/manual/dirtasks.html#patterns) to exclude.

Example: `{ "exclude": ["node_modules/**"] }` - exclude all files and directories under `node_modules` dir.

### verbose

Available value: `{Boolean}` `true`
Expand All @@ -80,6 +89,14 @@ $ ./bin/csscomb ./test --verbose
$ ./bin/csscomb ./test -v
```

### remove-empty-rulesets

Available values: `{Boolean}` `true`

Example: `{ "remove-empty-rulesets": true }` - remove rulesets that have no declarations or comments.

`a { color: red; } p { /* hey */ } b { }` → `a { color: red; } p { /* hey */ } `

### always-semicolon

Available value: `{Boolean}` `true`
Expand Down
13 changes: 8 additions & 5 deletions lib/csscomb.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var vfs = require('vow-fs');
*/
var Comb = function() {
this._options = [
'remove-empty-rulesets',
'always-semicolon',
'color-case',
'color-shorthand',
Expand Down Expand Up @@ -76,23 +77,25 @@ Comb.prototype = {
* @param {Number} level Indent level
*/
processNode: function(node, level) {
var _this = this;
node.forEach(function(node) {
if (!Array.isArray(node)) return;

var nodeType = node.shift();
_this._handlers.forEach(function(handler) {
this._handlers.forEach(function(handler) {
handler.process(nodeType, node, level);
});
node.unshift(nodeType);

if (nodeType === 'atrulers') level++;
_this.processNode(node, level);
});

this.processNode(node, level);
}, this);
},

/**
* Process file provided with a string.
* @param {String} text
* @param {String} filename
* @param {String} [filename]
*/
processString: function(text, filename) {
if (!text) return text;
Expand Down
85 changes: 85 additions & 0 deletions lib/options/remove-empty-rulesets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
module.exports = {

/**
* Sets handler value.
*
* @param {String} value Option value
* @returns {Object|undefined}
*/
setValue: function(value) {
if (value === true) {
this._value = value;
return this;
}
},

/**
* Remove rulesets with no declarations.
*
* @param {String} nodeType
* @param {Array} nodeContent
*/
process: function(nodeType, nodeContent) {
if (nodeType === 'stylesheet') {
this._processStylesheetContent(nodeContent);
}
},

_processStylesheetContent: function(nodeContent) {
this._removeEmptyRulesets(nodeContent);
this._mergeAdjacentWhitespace(nodeContent);
},

_removeEmptyRulesets: function(nodeContent) {
var i = nodeContent.length;
while (i--) {
if (this._isRuleset(nodeContent[i]) && this._isEmptyRuleset(nodeContent[i])) {
nodeContent.splice(i, 1);
}
}
},

/**
* Removing ruleset nodes from tree may result in two adjacent whitespace nodes which is not correct AST:
* [space, ruleset, space] => [space, space]
* To ensure correctness of further processing we should merge such nodes into one.
* [space, space] => [space]
*/
_mergeAdjacentWhitespace: function(nodeContent) {
var i = nodeContent.length - 1;
while (i-- > 0) {
if (this._isWhitespace(nodeContent[i]) && this._isWhitespace(nodeContent[i + 1])) {
nodeContent[i][1] += nodeContent[i + 1][1];
nodeContent.splice(i + 1, 1);
}
}
},

_isEmptyRuleset: function(ruleset) {
return ruleset.filter(this._isBlock).every(this._isEmptyBlock, this);
},

/**
* Block considered empty when it has no declarations or comments.
*/
_isEmptyBlock: function(node) {
return !node.some(this._isDeclarationOrComment);
},

_isDeclarationOrComment: function(node) {
return node[0] === 'declaration' || node[0] === 'comment';
},

_isRuleset: function(node) {
return node[0] === 'ruleset';
},

_isBlock: function(node) {
return node[0] === 'block';
},

_isWhitespace: function(node) {
return node[0] === 's';
}

};
4 changes: 2 additions & 2 deletions test/integral.origin.css
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ a, b, i /* foobar */ {
outline: 0;
padding: 0.4em 0;
}
}
}.empty-rule{}

/* Фигурные скобки. Вариант 2 */
div
Expand Down Expand Up @@ -120,7 +120,7 @@ top: 0;/* ololo */margin :0;}
b
{
top :0/* trololo */;margin : 0}

.empty-rule{}



Expand Down
99 changes: 99 additions & 0 deletions test/remove-empty-rulesets.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
var Comb = require('../lib/csscomb');
var assert = require('assert');

describe('options/remove-empty-rulesets', function() {
var comb;

beforeEach(function() {
comb = new Comb();
});

describe('configured with invalid value', function() {
beforeEach(function() {
comb.configure({ 'remove-empty-rulesets': 'foobar' });
});

it('should not remove empty ruleset', function() {
assert.equal(comb.processString('a { width: 10px; } b {}'), 'a { width: 10px; } b {}');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add some comments into a tested string.

});
});

describe('configured with Boolean "true" value', function() {
beforeEach(function() {
comb.configure({ 'remove-empty-rulesets': true });
});

it('should remove empty ruleset', 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 ');
});

it('should leave ruleset with comments', function() {
assert.equal(comb.processString('a { /* comment */ }\nb {} '), 'a { /* comment */ }\n ');
});
});
});

describe('options/remove-empty-rulesets AST manipulation', function() {
var rule;
var nodeContent;

beforeEach(function() {
rule = require('../lib/options/remove-empty-rulesets.js');
});

describe('merge adjacent whitespace', function() {
it('should do nothing with empty content', function() {
nodeContent = [];
rule._mergeAdjacentWhitespace(nodeContent);
assert.deepEqual(nodeContent, []);
});

it('should do nothing with only one whitespace', function() {
nodeContent = [['s', ' ']];
rule._mergeAdjacentWhitespace(nodeContent);
assert.deepEqual(nodeContent, [['s', ' ']]);
});

it('should merge two adjacent whitespaces', function() {
nodeContent = [['s', ' '], ['s', ' \n']];
rule._mergeAdjacentWhitespace(nodeContent);
assert.deepEqual(nodeContent, [['s', ' \n']]);
});

it('should merge three adjacent whitespaces', function() {
nodeContent = [['s', ' '], ['s', ' \n'], ['s', ' \n']];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

synthetic test, can't be in real life

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And previous too.
But this kind of tree may be produced on first step in processing and i must ensure that second step will work correctly. We should have both integration and unit tests. This is a unit test.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, accepted.

rule._mergeAdjacentWhitespace(nodeContent);
assert.deepEqual(nodeContent, [['s', ' \n \n']]);
});
});

describe('remove empty rulesets', function() {
it('should do nothing with empty content', function() {
nodeContent = [];
rule._removeEmptyRulesets(nodeContent);
assert.deepEqual(nodeContent, []);
});

it('should do nothing with no rulesets', function() {
nodeContent = [['s', ' ']];
rule._removeEmptyRulesets(nodeContent);
assert.deepEqual(nodeContent, [['s', ' ']]);
});

it('should remove empty ruleset', function() {
nodeContent = [['ruleset', []]];
rule._removeEmptyRulesets(nodeContent);
assert.deepEqual(nodeContent, []);
});

it('should remove two empty rulesets', function() {
nodeContent = [['s', ' '], ['ruleset', []], ['s', ' \n'], ['ruleset', []]];
rule._removeEmptyRulesets(nodeContent);
assert.deepEqual(nodeContent, [['s', ' '], ['s', ' \n']]);
});
});
});