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

Allow multiline css declarations in less files for increased readability #471

Open
hostep opened this issue Sep 26, 2023 · 1 comment
Open
Labels

Comments

@hostep
Copy link
Contributor

hostep commented Sep 26, 2023

Preconditions

  1. Have a file test1.less like this:
& when (@media-common = true) {
    .lib-icon-font(
        @icon-camera__content,
        @_icon-font: @icons-admin__font-name,
        @_icon-font-size: @image-gallery-placeholder-icon__size,
        @_icon-font-color: @image-gallery-placeholder-icon__color,
        @_icon-font-text-hide: true
    );
}
  1. Have a file test2.less like this:
& when (@media-common = true) {
    .lib-icon-font(@icon-camera__content, @_icon-font: @icons-admin__font-name, @_icon-font-size: @image-gallery-placeholder-icon__size, @_icon-font-color: @image-gallery-placeholder-icon__color, @_icon-font-text-hide: true);
}
  1. See that the contents of these 2 files are the same, it's just the formatting of the code that's different

Steps to reproduce

  1. Run vendor/bin/phpcs -s --standard=Magento2 test*.less

Expected result

  1. Both files should show no errors

Actual result

  1. Getting this output:
FILE: test1.less
-------------------------------------------------------------------------------------------------------------------------
FOUND 0 ERRORS AND 4 WARNINGS AFFECTING 4 LINES
-------------------------------------------------------------------------------------------------------------------------
 4 | WARNING | Expected 1 space after colon in style definition; newline found
   |         | (Magento2.Less.ColonSpacing.AfterNewline)
 5 | WARNING | Expected 1 space after colon in style definition; newline found
   |         | (Magento2.Less.ColonSpacing.AfterNewline)
 6 | WARNING | Expected 1 space after colon in style definition; newline found
   |         | (Magento2.Less.ColonSpacing.AfterNewline)
 7 | WARNING | Expected 1 space after colon in style definition; newline found
   |         | (Magento2.Less.ColonSpacing.AfterNewline)
-------------------------------------------------------------------------------------------------------------------------

Time: 78ms; Memory: 6MB

Discussion

In my opinion, the formatting of the code in file test1.less is a lot more readable then in file test2.less. Coding standards shouldn't recommend to make code less readable.
Also, Magento uses this syntax as well in its own code and is thus violating its own coding standards ...

This was already reported before, but just in a comment on another issue, now it has its own proper issue which might make it faster to get picked up?

@hostep hostep added the bug Something isn't working label Sep 26, 2023
@m2-assistant
Copy link

m2-assistant bot commented Sep 26, 2023

Hi @hostep. Thank you for your report.
To speed up processing of this issue, make sure that you provided sufficient information.
Add a comment to assign the issue: @magento I am working on this


Join Magento Community Engineering Slack and ask your questions in #github channel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Ready for Grooming
Development

No branches or pull requests

1 participant