Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Follow-up to #2865971: Use stylelint as opposed to csslint in core. Configure the declaration-block-trailing-semicolon to be consistent with https://www.drupal.org/docs/develop/standards/css/css-coding-standards
From the CSS Formatting Guidelines:
Include a semi-colon at the end of all declarations, including the last declaration in a declaration block.
Proposed resolution
Brief instructions on running stylelint - you'll need npm...
All the commands below take place in DRUPAL_ROOT/core
To install stylelint
npm install
This will install Drupal 8's npm dependencies of which stylelint is one.
To run it on all core css files. Apply this issue's patch and do the following command from DRUPAL_ROOT/core
npm run lint:css
Remove the stylelint rule from the core/.stylelintrc file, since the rule now matches the stylelint-config-standard that is extended, see https://github.com/stylelint/stylelint-config-standard/blob/master/index.js
Remaining tasks
User interface changes
None
API changes
None
Comments
Comment #2
alexpottComment #3
BrightBoldComment #4
BrightBoldSet
declaration-block-trailing-semicolonto "always" and added missing semicolons in css.This patch is dependent on #2865971-41: Use stylelint as opposed to csslint in core so will fail tests until that's committed.
Comment #5
BrightBoldThis patch includes the same changes as #4 but also incorporates the contents of #2865971-41: Use stylelint as opposed to csslint in core and thus can be used to run tests.
Comment #6
BrightBoldComment #7
idebr CreditAttribution: idebr at ezCompany commentedUpdated the issue summary to reflect the rule can be removed from the .stylelintrc, since it matches the stylelint-config-standard that is extended.
Added the Novice tag, since there is a clear issue summary and proposed resolution.
Comment #8
idebr CreditAttribution: idebr at ezCompany commentedComment #9
pk188 CreditAttribution: pk188 at OpenSense Labs commentedRe rolled.
Comment #11
pk188 CreditAttribution: pk188 at OpenSense Labs commentedRe rolled.
Comment #13
pk188 CreditAttribution: pk188 at OpenSense Labs commentedComment #14
idebr CreditAttribution: idebr at ezCompany commentedHi pk188,
Thanks for working on this issue!
Your patch did not apply correctly, since the target branch was misconfigured to 8.3.x instead of 8.4.x. I have updated the issue accordingly.
Comment #15
idebr CreditAttribution: idebr at ezCompany commentedComment #17
BrightBoldComment #18
BrightBoldComment #19
BrightBoldThe only differences I see between the patches in #9/#11 and the patch in #4 are changes not covered by this issue: the deletion of a commented out property and a change in the minimum browser versions. While those both may be changes we want to make, we shouldn't be making them here. So I've fix the patch from #4 to take @idebr's comment into account.
Comment #21
joelpittetThank you @BrightBold and @Alexpott. Ran the patch with the lint command and it has no errors, reverted one file to see the error show and it did.
Comment #22
alexpottCommitted 5616f9b and pushed to 8.5.x. Thanks!