sasstools / sass-lint

Pure Node.js Sass linting
MIT License
1.77k stars 532 forks source link

New Rule: Mergeable Selector #18

Closed Snugug closed 9 years ago

Snugug commented 9 years ago

Check to see if selectors can be merged together

DanPurdy commented 9 years ago

Nearly, done with this.

Few interesting points I was reading within the scss-lint issues

First off - one I'm intending to support mergeable-selector whitelist

And a few cases we need to be careful with.. Was adding lots of test cases last night. avoid this

DanPurdy commented 9 years ago

@Snugug @bgriffith Wondering if you guys have opinions on the following?

The force nesting option within the mergeable selectors rule of scss-lint allows you to force element nesting as such

div p {
  color: red;
}

// to this

div {
  p {
    color: red;
  }
}

But it doesn't enforce pseudo class nesting like the following

div:hover {
  color: red;
}

// Not enforced

div {
  &:hover {
    color: red;
  }
}

I think it's a valid type of nesting to flag up and so I'm thinking as well as the whitelist option I already have working we should perhaps support both a force nesting option and a force pseudo nesting option.

I think a separate rule for each may be a little overboard but it makes mergeable-selector fairly flexible..

I'm just trying to cover this in tests for all types of selectors too

DanPurdy commented 9 years ago

Had to back to the drawing board a bit here and rebuild what I had as there was no easy way to see if a rule was a child of another rule without keeping reference to the number of child rules a parent rule contained and dynamically building the selector structure as gonzales-pe quite rightly strips some of the important characters.

I've just got force nesting left to do now and then a bit of tidying up from my fairly procedural approach so hopefully will have this ready very soon!