Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New options: Spaces before and after selector delimiters #196

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions config/csscomb.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
"space-after-colon": " ",
"space-after-combinator": " ",
"space-after-opening-brace": "\n",
"space-after-selector-delimiter": "\n",
Copy link
Member

Choose a reason for hiding this comment

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

@mishanga, I need your opinion on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tonyganch, it might be

    "space-after-selector-delimiter": " ",

I just followed my company styleguide.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's leave it as it already is.

"space-before-colon": "",
"space-before-combinator": " ",
"space-before-opening-brace": "\n",
"space-before-selector-delimiter": "",
"strip-spaces": true,
"unitless-zero": true,
"vendor-prefix-align": true,
Expand Down
76 changes: 76 additions & 0 deletions doc/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,44 @@ a{
color: panda;}
```

## space-after-selector-delimiter

Set space after selector delimiter.

Acceptable values:

* `{Number}` — number of whitespaces;
* `{String}` — string with whitespaces, tabs or line breaks.

Example: `{ 'space-after-selector-delimiter': 1 }`

```scss
// Before:
a,b{
color: panda;
}

// After:
a, b {
color: panda;
}
```

Example: `{ 'space-before-selector-delimiter': '\n' }`
Copy link
Member

Choose a reason for hiding this comment

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

@vecmezoni, space-before... –> space-after...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tonyganch, actually space-after... –> space-before....
Fixed.


```scss
// Before:
a, b{
color: panda;
}

// After:
a,
b{
color: panda;
}
```

## space-before-colon

Set space before `:` in declarations.
Expand Down Expand Up @@ -536,6 +574,44 @@ a
}
```

## space-before-selector-delimiter

Set space before selector delimiter.

Acceptable values:

* `{Number}` — number of whitespaces;
* `{String}` — string with whitespaces, tabs or line breaks.

Example: `{ 'space-before-selector-delimiter': 0 }`

```scss
// Before:
a , b{
color: panda;
}

// After:
a, b {
color: panda;
}
```

Example: `{ 'space-before-selector-delimiter': '\n' }`

```scss
// Before:
a, b{
color: panda;
}

// After:
a
,b{
color: panda;
}
```

## strip-spaces

Whether to trim trailing spaces.
Expand Down
2 changes: 2 additions & 0 deletions lib/csscomb.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ var OPTIONS = [
'space-after-colon',
'space-before-opening-brace',
'space-after-opening-brace',
'space-before-selector-delimiter',
'space-after-selector-delimiter',
'sort-order',
'block-indent',
'tab-size',
Expand Down
54 changes: 54 additions & 0 deletions lib/options/space-after-selector-delimiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
module.exports = {
name: 'space-after-selector-delimiter',

accepts: {
number: true,
string: /^[ \t\n]*$/
},

/**
* Processes tree node.
*
* @param {String} nodeType
* @param {node} node
*/
process: function(nodeType, node) {
if (nodeType !== 'selector') return;

var value = this.getValue('space-after-selector-delimiter');

for (var i = node.length; i--;) {
if (node[i][0] === 'delim') {
Copy link
Member

Choose a reason for hiding this comment

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

@vecmezoni, the same thing with if (node[i][0] !== 'delim') continue; here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

if (node[i + 1][1][0] === 's') {
node[i + 1][1][1] = value;
} else {
node[i + 1].splice(1, 0, ['s', value]);
}
}
}
},

/**
* Detects the value of an option at the tree node.
*
* @param {String} nodeType
* @param {node} node
*/
detect: function(nodeType, node) {
if (nodeType !== 'selector') return;

var variants = [];

for (var i = node.length; i--;) {
if (node[i][0] !== 'delim') continue;

if (node[i + 1][1][0] === 's') {
variants.push(node[i + 1][1][1]);
} else {
variants.push('');
}
}

return variants;
}
};
54 changes: 54 additions & 0 deletions lib/options/space-before-selector-delimiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
module.exports = {
name: 'space-before-selector-delimiter',

accepts: {
number: true,
string: /^[ \t\n]*$/
},

/**
* Processes tree node.
*
* @param {String} nodeType
* @param {node} node
*/
process: function(nodeType, node) {
if (nodeType !== 'selector') return;

var value = this.getValue('space-before-selector-delimiter');

for (var i = node.length; i--;) {
if (node[i][0] === 'delim') {
Copy link
Member

Choose a reason for hiding this comment

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

@vecmezoni, I like if (node[i][0] !== 'delim') continue; from line 43 better, because this way we have less indentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

if (node[i - 1][node[i - 1].length - 1][0] === 's') {
node[i - 1][node[i - 1].length - 1][1] = value;
} else {
node[i - 1].push(['s', value]);
}
Copy link
Member

Choose a reason for hiding this comment

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

@vecmezoni, I think it's a good idea to cache node[i - 1] value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a variable for previous node.

}
}
},

/**
* Detects the value of an option at the tree node.
*
* @param {String} nodeType
* @param {node} node
*/
detect: function(nodeType, node) {
if (nodeType !== 'selector') return;

var variants = [];

for (var i = node.length; i--;) {
if (node[i][0] !== 'delim') continue;

if (node[i - 1][node[i - 1].length - 1][0] === 's') {
variants.push(node[i - 1][node[i - 1].length - 1][1]);
} else {
variants.push('');
}
}

return variants;
}
};
83 changes: 83 additions & 0 deletions test/options/space-after-selector-delimiter.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
describe('options/space-after-selector-delimiter:', function() {
beforeEach(function() {
this.filename = __filename;
});

it('Array value => should not change anything', function() {
this.comb.configure({ 'space-after-selector-delimiter': ['', ' '] });
this.shouldBeEqual('test.css');
});

it('Invalid string value => should not change anything', function() {
this.comb.configure({ 'space-after-selector-delimiter': ' nani ' });
this.shouldBeEqual('test.css');
});

it('Float number value => should not change anything', function() {
this.comb.configure({ 'space-after-selector-delimiter': 3.5 });
this.shouldBeEqual('test.css');
});

it('Integer value => should set proper space after selector delimiter', function() {
this.comb.configure({ 'space-after-selector-delimiter': 0 });
this.shouldBeEqual('test.css', 'test.expected.css');
});

it('Valid string value (spaces only) => should set proper space after selector delimiter', function() {
this.comb.configure({ 'space-after-selector-delimiter': ' ' });
this.shouldBeEqual('test.css', 'test-2.expected.css');
});

it('Valid string value (spaces and newlines) => should set proper space after selector delimiter', function() {
this.comb.configure({ 'space-after-selector-delimiter': '\n ' });
this.shouldBeEqual('test.css', 'test-3.expected.css');
});

it('Should detect no whitespace', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a,b{top:0}',
{ 'space-after-selector-delimiter': '' }
);
});

it('Should detect whitespace', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a, \n b {top:0}',
{ 'space-after-selector-delimiter': ' \n ' }
);
});

it('Should detect no whitespace (2 blocks)', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a,b{top:0} a, b{left:0}',
{ 'space-after-selector-delimiter': '' }
);
});

it('Should detect whitespace (2 blocks)', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a, b {top:0} b,a{left:0}',
{ 'space-after-selector-delimiter': ' ' }
);
});

it('Should detect no whitespace (3 blocks)', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a, b{top:0} b,c{left:0} c,d{right:0}',
{ 'space-after-selector-delimiter': '' }
);
});

it('Should detect whitespace (3 blocks)', function() {
this.shouldDetect(
['space-after-selector-delimiter'],
'a,b{top:0} b, c{left:0} c, sd{right:0}',
{ 'space-after-selector-delimiter': ' ' }
);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
a, b { color: red }
a, b { color: red }
a , b { color: red }
a, b { color: red }
a+b, c>d, e{ color: red }
11 changes: 11 additions & 0 deletions test/options/space-after-selector-delimiter/test-3.expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
a,
b { color: red }
a,
b { color: red }
a ,
b { color: red }
a,
b { color: red }
a+b,
c>d,
e{ color: red }
6 changes: 6 additions & 0 deletions test/options/space-after-selector-delimiter/test.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
a,b { color: red }
a, b { color: red }
a ,b { color: red }
a,
b { color: red }
a+b,c>d,e{ color: red }
5 changes: 5 additions & 0 deletions test/options/space-after-selector-delimiter/test.expected.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
a,b { color: red }
a,b { color: red }
a ,b { color: red }
a,b { color: red }
a+b,c>d,e{ color: red }
Loading