magento / magento/magento-coding-standard
Allow multiline css declarations in less files for increased readability
Nobody has claimed this yet.
- Dominant language
- PHP
- Stars
- 375
- Forks
- 165
- PR merge metrics
- No merged PRs in 30d
Description
### Preconditions
1. Have a file `test1.less` like this:
```less
& 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:
```less
& 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?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Run vendor/bin/phpcs -s --standard=Magento2 against the two test*.less examples and inspect the Magento2.Less.ColonSpacing.AfterNewline rule. Add regression coverage for multiline Less declarations, using the existing Magento2 standard tests or the rule's test entry point if present. Done means both equivalent files produce no warnings.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100