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.

Sign up for GitHub

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

[LESS] Fixing issues with spacing when an object literal lives inside a mixin #2017

Merged
merged 6 commits into from
Apr 7, 2022
Merged

[LESS] Fixing issues with spacing when an object literal lives inside a mixin #2017

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
784bd1e
Fixing issues with spacing when an object literal lives inside a mixi…
mhnaeem Apr 7, 2022
15e0bbf
Update test/data/css/tests.js
bitwiseman Apr 7, 2022
a3fc4b6
Update test/data/css/tests.js
bitwiseman Apr 7, 2022
d768b06
Update test/data/css/tests.js
bitwiseman Apr 7, 2022
f6e0936
Update test/data/css/tests.js
bitwiseman Apr 7, 2022
395b773
Update test/data/css/tests.js
bitwiseman Apr 7, 2022
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
13 changes: 12 additions & 1 deletion js/src/css/beautifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,12 @@ Beautifier.prototype.beautify = function() {
this.indent();
this._output.set_indent(this._indentLevel);
} else {
this.indent();
// inside mixin and first param is object
if (previous_ch === '(') {
this._output.space_before_token = false;
} else if (previous_ch !== ',') {
this.indent();
}
this.print_string(this._ch);
}

Expand Down Expand Up @@ -346,6 +351,12 @@ Beautifier.prototype.beautify = function() {
this._output.add_new_line(true);
}
}
if (this._input.peek() === ')') {
this._output.trim(true);
if (this._options.brace_style === "expand") {
this._output.add_new_line(true);
}
}
} else if (this._ch === ":") {
if ((insideRule || enteringConditionalGroup) && !(this._input.lookBack("&") || this.foundNestedPseudoClass()) && !this._input.lookBack("(") && !insideAtExtend && parenLevel === 0) {
// 'property: value' delimiter
Expand Down
10 changes: 9 additions & 1 deletion python/cssbeautifier/css/beautifier.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,11 @@ def beautify(self):
self.indent()
self._output.set_indent(self._indentLevel)
else:
self.indent()
# inside mixin and first param is object
if previous_ch == "(":
self._output.space_before_token = False
elif previous_ch != ",":
self.indent()
self.print_string(self._ch)

self.eatWhitespace(True)
Expand Down Expand Up @@ -372,6 +376,10 @@ def beautify(self):
):
if self._input.peek() != "}":
self._output.add_new_line(True)
if self._input.peek() == ")":
self._output.trim(True)
if self._options.brace_style == "expand":
self._output.add_new_line(True)
elif self._ch == ":":
if (
(insideRule or enteringConditionalGroup)
Expand Down
51 changes: 48 additions & 3 deletions test/data/css/tests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1360,9 +1360,8 @@ exports.test_data = {
'}',
'.set {',
' each(@set, {',
' @{key}-@{index}: @value;',
' }',
' );',
' @{key}-@{index}: @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.

Had to change this test because I believe it is incorrect. It adds extra indents to properties which is unexpected, this test also conflicting with my fixes. I have added example of what it would look like before my changes and after.

Before PR:

@set: {
    one: blue;
    two: green;
    three: red;
}
.set {
    each(@set, {
            @{key}-@{index}: @value;
        }
    );
}

After PR:

@set: {
    one: blue;
    two: green;
    three: red;
}
.set {
    each(@set, {
        @{key}-@{index}: @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.

Please refer to the example on the LESS's website which also uses the same formatting as after my changes
https://lesscss.org/functions/#list-functions-each

Copy link
Member

Choose a reason for hiding this comment

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

You are correct. Sometimes the tests document existing wrong behavior so that we'll know when something is fixed.

'}'
]
},
Expand Down Expand Up @@ -1732,6 +1731,20 @@ exports.test_data = {
'{{empty_line_indent}} width: auto;',
'}'
]
}, {
comment: 'mixins call with object notation, and brace_style="expand"',
input: [
'.example({',
' color:red;',
'});'
],
output: [
'.example(',
' {',
' color:red;',
' }',
');'
]
}, {
comment: 'integration test of newline_between_rules, imports, and brace_style="expand"',
input: '.a{} @import "custom.css";.rule{}',
Expand All @@ -1758,6 +1771,38 @@ exports.test_data = {
' &:extend(a:hover);',
'}'
]
}, {
unchanged: [
'.test {',
' .example({',
' color:red;',
' });',
'}'
]
}, {
unchanged: [
'.example2({',
' display:none;',
'});'
]
}, {
unchanged: [
'.aa {',
' .mq-medium(a, {',
' background: red;',
' });',
'}'
]
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
}, {
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
unchanged: [
'@selectors: blue, green, red;',
'',
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
'each(@selectors, {',
' .sel-@{value} {',
' a: b;',
' }',
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
'});'
]
bitwiseman marked this conversation as resolved.
Show resolved Hide resolved
}, {
comment: 'Ensure simple closing parens do not break behavior',
unchanged: [
Expand Down