magento / magento-coding-standard

Magento Coding Standard
Open Software License 3.0
343 stars 153 forks source link

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

Open hostep opened 1 year ago

hostep commented 1 year ago

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
    );
    }
  2. 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);
    }
  3. 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](https://github.com/magento/magento2/blob/8ea62ab9374a9e14a9ffe615aceedff745d80077/app/design/adminhtml/Magento/backend/web/css/source/components/_file-uploader.less#L274-L280) and is thus violating its own coding standards ...

This was already reported before, but just [in a comment](https://github.com/magento/magento-coding-standard/issues/395#issuecomment-1503527962) on another issue, now it has its own proper issue which might make it faster to get picked up?
m2-assistant[bot] commented 1 year ago

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.