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

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

Merged

Conversation

mhnaeem
Copy link
Contributor

@mhnaeem mhnaeem commented Apr 7, 2022

Description

Fix issues with spacing in LESS code when using object literals inside mixins. The changes in this PR could be considered breaking since but I believe they are correct.

Fixes Issue:

Fixes #722
Fixes #2016

' }',
' );',
' @{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.

test/data/css/tests.js Outdated Show resolved Hide resolved
@bitwiseman bitwiseman added this to the v1.15.x milestone Apr 7, 2022
test/data/css/tests.js Outdated Show resolved Hide resolved
test/data/css/tests.js Outdated Show resolved Hide resolved
test/data/css/tests.js Outdated Show resolved Hide resolved
Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

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

Nicely done!

@bitwiseman bitwiseman merged commit 51a283a into beautifier:main Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LESS mixin unexpected code formatting LESS mixins gets formatted strangely
2 participants