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

Jump to bottom

Avoid combining disable comments with a blank line #4913

Merged
merged 1 commit into from Sep 1, 2020
Merged

Avoid combining disable comments with a blank line #4913

merged 1 commit into from Sep 1, 2020

Conversation

jathak
Copy link
Contributor

@jathak jathak commented Aug 31, 2020

Which issue, if any, is this issue related to?

Fixes #4911.

Is there anything in the PR that needs further explanation?

This checks line numbers before combining adjacent // comments.

@jathak jathak force-pushed the blank-line-between-comments branch from bb54948 to eb3b04a Compare August 31, 2020 18:34
hudochenkov
hudochenkov approved these changes Aug 31, 2020
View reviewed changes
Copy link
Member

@hudochenkov hudochenkov left a comment

Choose a reason for hiding this comment

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

Looks good!

Thank you for a quick fix!

@jeddy3 jeddy3 mentioned this pull request Sep 1, 2020
Closed
6 tasks
@jeddy3 jeddy3 merged commit c287bd8 into master Sep 1, 2020
@jeddy3 jeddy3 deleted the blank-line-between-comments branch September 1, 2020 19:53
@jeddy3
Copy link
Member

jeddy3 commented Sep 1, 2020

Thanks for the fix!

Changelog entry added:

  • Fixed: double-slash disable comments when followed by another comment (#4913).

@sebastianhaeni sebastianhaeni mentioned this pull request Sep 2, 2020
Closed
m-allanson added a commit that referenced this pull request Sep 3, 2020
@m-allanson
Merge branch 'master' into browser-bundle
5599148 
* master: (34 commits)
  Update CHANGELOG.md
  Fix double-slash disable comments when followed by another comment (#4913)
  Update CHANGELOG.md (#4916)
  13.7.0
  Prepare 13.7.0
  Prepare changelog
  Update dependencies
  Update CHANGELOG.md
  Deprecate *-blacklist/*-requirelist/*-whitelist (#4892)
  Fix some path / glob problems (#4867)
  Update CHANGELOG.md
  Add a reportDescriptionlessDisables flag (#4907)
  Fix CHANGELOG.md format via Prettier (#4910)
  Fix callbacks in tests (#4903)
  Update CHANGELOG.md
  Fix false positives for trailing combinator in selector-combinator-space-after (#4878)
  Add coc-stylelint (#4901)
  Update CHANGELOG.md
  Add support for *.cjs config files (#4905)
  Add a reportDisables secondary option (#4897)
  ...
@ThiefMaster
Copy link

Could this be released as 13.7.1? :)

ThiefMaster added a commit to indico/indico that referenced this pull request Sep 9, 2020
@ThiefMaster
Downgrade stylelint
3f79b60 
until the fix from stylelint/stylelint#4913 is released...
@XhmikosR
Copy link
Member

It seems we still get failures even with this patch: https://github.com/twbs/bootstrap/runs/1110881505.

I'll try to pinpoint the cause, but going back to 13.6.1 works fine again.

@stof
Copy link
Contributor

stof commented Sep 14, 2020

This PR fixed it for cases where the stylelint-disable double-slash comment is on the first line of a group of commented lines, by avoiding merging it with previous lines. But Bootstrap has cases where such comment happens after other comment but without empty lines between them: https://github.com/twbs/bootstrap/blob/5c25ea647bdc463df5a322c8248460a78305a1f4/scss/bootstrap-grid.scss#L24-L27

Adding an empty line between the 2 comments should fix it with 13.7.1.

@XhmikosR
Copy link
Member

Thanks for the reply. That does seem to be the case. Although, ideally, this should be handled too because it is a regression.

@stof stof mentioned this pull request Sep 14, 2020
Closed
@stof
Copy link
Contributor

stof commented Sep 14, 2020

@XhmikosR I created an new issue for the remaining regression

@chalkygames123 chalkygames123 mentioned this pull request Sep 28, 2020
Closed
This was referenced Mar 8, 2021
Closed
Closed
@dependabot dependabot bot mentioned this pull request Mar 15, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hudochenkov hudochenkov hudochenkov approved these changes

@jeddy3 jeddy3 jeddy3 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix double-slash disable comments followed by another comment
6 participants
@jathak @jeddy3 @ThiefMaster @XhmikosR @stof @hudochenkov