ben-eb / perfectionist

Beautify CSS files.
MIT License
229 stars 14 forks source link

Removing comments #11

Closed Mottie closed 8 years ago

Mottie commented 8 years ago

Sorry to be a pain, because this is probably an edge case... but we do have this situation.

I tried to provide a reduced case and found that if there are more than 3 selectors in a row followed by at least 3 comments, the comments get removed. Then I found some other odd behavior... so starting with the following css and using default settings:

 /* 2 selectors + 3 comments + 3 selectors */
 .selectorA, .selectorB,
 /* comment 1 */
 /* comment 2 */
 /* comment 3 */
 .selectorD, .selectorE, .selectorF {
   color: white;
 }
 /* 3 selectors + 2 comments + 3 selectors */
 .selectorA, .selectorB, .selectorC,
 /* comment 1 */
 /* comment 2 */
 .selectorD, .selectorE, .selectorF {
   color: white;
 }
 /* 3 selectors + 3 comments + 3 selectors */
 .selectorA, .selectorB, .selectorC,
 /* comment 1 */
 /* comment 2 */
 /* comment 3 */
 .selectorD, .selectorE, .selectorF {
   color: white;
 }
 /* 4 selectors + 2 comments + 3 selectors */
 .selectorA, .selectorB, .selectorC, .selectorX,
 /* comment 1 */
 /* comment 2 */
 .selectorD, .selectorE, .selectorF {
   color: white;
 }
 /* 4 selectors + 1 comment + 3 selectors */
 .selectorA, .selectorB, .selectorC, .selectorX,
 /* comment 1 */
 .selectorD, .selectorE, .selectorF {
   color: white;
 }
 /* 4 selectors + 1 comment + 2 selectors */
 .selectorA, .selectorB, .selectorC, .selectorX,
 /* comment 1 */
 .selectorD, .selectorE {
   color: white;
 }

This is the result:

/* 2 selectors + 3 comments + 3 selectors */

.selectorA, .selectorB,
 /* comment 1 */
 /* comment 2 */
 /* comment 3 */
 .selectorD, .selectorE, .selectorF {
    color: white;
}

/* 3 selectors + 2 comments + 3 selectors */

.selectorA, .selectorB, .selectorC,
 /* comment 1 */
 /* comment 2 */
 .selectorD, .selectorE, .selectorF {
    color: white;
}

/* 3 selectors + 3 comments + 3 selectors */

.selectorA, .selectorB, .selectorC, .selectorD, .selectorE, .selectorF {
    color: white;
}

/* 4 selectors + 2 comments + 3 selectors */

.selectorA, .selectorB, .selectorC, .selectorX, .selectorD, .selectorE,
.selectorF {
    color: white;
}

/* 4 selectors + 1 comment + 3 selectors */

.selectorA, .selectorB, .selectorC, .selectorX, .selectorD, .selectorE,
.selectorF {
    color: white;
}

/* 4 selectors + 1 comment + 2 selectors */

.selectorA, .selectorB, .selectorC, .selectorX,
 /* comment 1 */
 .selectorD, .selectorE {
    color: white;
}

The last case is an especially odd one... I think it might have something to do with the maxSelectorLength setting.

ben-eb commented 8 years ago

I'll see if I can get to this tomorrow. :+1:

ben-eb commented 8 years ago

I can stop perfectionist removing the comments, however the formatting is kind of wonky on this, for example:

/* 3 selectors + 3 comments + 3 selectors */

.selectorA, .selectorB, .selectorC,
/* comment 1 */
 /* comment 2 */
 /* comment 3 */
 .selectorD, .selectorE,
.selectorF {
    color: white;
}

But I'm not sure if I want to start parsing selector comments for newlines, especially as you say it's kind of an edge case as it is. I would appreciate any help regarding this if it's important to people. :+1:

Mottie commented 8 years ago

It's actually all working fine now. I think the weird indenting that you see is because the original input all starts out with a one space indention, which isn't typical. If you remove it, the output lines up perfectly.

I would still like to see an option to not add the extra carriage return after the comment, but I have the grunt build set up to use string replacement to remove them all, so I'm not worried about it.

Thanks again for your hard work!