CSS Hierarchy Indent option (off by default)

[holloway]

First some stats about the level of popularity of this formatting:

• SitePoint surveys over two years of 5000 devs indicate about ~25% of CSS is written this way. A Twitter poll of 725 devs indicates that 10% of CSS is written this way, and because these are probably Prettier users this may indicate that 10% of devs disable Prettier CSS printing to maintain this style. So it's probably 10% hard support ranging to 25% soft support for this feature.
• It's very similar to the default output of Sass (both libsass and rubysass) and their devs say indenting hierarchy helps with debugging and readability.
• Recommended best practice by CSSGuidelin.es / Harry Roberts who's fairly well known in the CSS community

---

As discussed in #3012 this indents adjacent styles based on implied or explicit hierarchies in the CSS selector. It can be enabled with the --css-hierarchy-indent flag (formerly the --indent-styles).

As this is my first PR to Prettier I'd appreciate as much feedback as you can give!

In terms of approach, the code makes indent wrappers around CSS Rules without disturbing the rest of the codebase (so when indent-styles is false there's minimal differences in the code path)

Previous formatting

.reposition {
  display: flex;
  align-items: center;
}

.reposition button[disabled] {
  opacity: 0.25;
}

.maps {
  display: block;
  overflow: auto;
}

.maps--disabled {
  opacity: 0.5;
  pointer-events: none;
}

New formatting

.reposition {
  display: flex;
  align-items: center;
}

  .reposition button[disabled] {
    opacity: 0.25;
  }

.maps {
  display: block;
  overflow: auto;
}

  .maps--disabled {
    opacity: 0.5;
    pointer-events: none;
  }

Previous formatting:

table {
  background: papayawhip;
}

table td.inactive {
  background: palegoldenrod;
}

table tr {
  background: palegreen;
}

table tr.active {
  background: palegreen;
}

div {
  border-color: firebrick;
}

New formatting

table {
  background: papayawhip;
}

  table td.inactive {
    background: palegoldenrod;
  }

  table tr {
    background: palegreen;
  }

    table tr.active {
      background: lime;
    }

div {
  border-color: firebrick;
}

[azz]

Why would

table tr.active

be a different indentation level to

table td.inactive

?

[holloway]

(edit: sorry I misunderstand, and I've edited my comment @azz )

The answer is that table tr.active is indented because it's preceded by table tr.

The flattened list is,

table
table td.inactive
table tr
table tr.active
div

which is turned into

table
  table td.inactive
  table tr
    table tr.active
div

table td.inactive is 1 deep because follows table , whereas table tr.active follows both table tr and table so it's 2 deep.

This is the approach that I think is a good balance of algo complexity without having to favour any particular CSS naming conventions. Of course I'm open to feedback on that if we can think of a better rule.

While we know (as programmers) that tr is implied in an HTML <table> I don't think the CSS indentation algo could (or should?) indent by inferring Elements.

[azz]

So it's supposed to approximate DOM hierarchy? That seems difficult to define programatically.

[holloway]

No, and I agree that would be too complicated. Instead it's (almost!) just treating selectors like strings and comparing preceding ones to infer hierarchy / indentation.

If we just look two levels deep into the hierarchy and ignore classes (just use elements) it looks like,

table
  table tr
  table td

Both table tr and table td are preceded with table which causes this hierarchy.

So it's not approximating HTML DOM hierarchy.

This means that it works with common CSS naming conventions, such as:

.col {
  display:block;
}

  .col.col1 {
    width: 8.33%;
  }

  .col.col3 {
    width: 25%;
  }

  .col.col6 {
    width: 50%;
  }

    .col.col6--disabled {
      opacity: 0.5;
    }

  .col.col9 {
    width: 75%;
  }

  .col.col12 {
    width: 100%;
  }

or

.container {
  display:block;
}

  .container__cell {
    border-style: dashed;
  }

  .container--inactive {
    opacity: 0.5;
  }

[holloway]

@azz sorry, I just realised that I misunderstand your question and I've updated my 2nd comment

[holloway]

Just to give a better example

I understand that Prettier's purpose is to create more uniformity in codebases by removing personal style (except for a few cases like ' vs " etc.), and so CSS indenting could be viewed as a personal style...

...however I'd say that primarily it has a function (that 24% of people use, about 47% of those without Sass nesting use) and that is to group CSS and to help catch typos early by using naming conventions that devs already have, and as an example of that here's how this PR looks on a larger codebase: here's all of Bootstrap (the compiled output as plain CSS),

https://gist.github.com/holloway/66cb1585cc11ac5c96848dfcca5d1c5c#file-bootstrap-with-indenting-css-L1414

Now of course this is just some arbitrary CSS, but on a codebase that wasn't organised with CSS Indenting in mind you can see tables starting on line 1414 have hierarchies that are made clearer, and .form-controls also adhere to a useful grouping.

(by the way -- I should say that I'm sorry if I'm repeating myself @azz but I'm not quite sure how to progress this. E.g. with your emoji response on the issue thread I'm not sure if I should explain the PR code in detail, or give high-level examples like this, or wait until after Prettier 1.8 is out, or promise to maintain the code for years (i will!) etc.)

[lipis]

Can we pass on this one?!

table {
  background: papayawhip;
}

  table td.inactive {
    background: palegoldenrod;
  }

  table tr {
    background: palegreen;
  }

    table tr.active {
      background: lime;
    }

div {
  border-color: firebrick;
}

and usually with SASS, or Less you would nest those and in those cases nobody cares about the output as it's usually not in the codebase..

[holloway]

@lipis Yep and in the issues thread you can see some stats around preprocessor nesting usage which indicates that nesting in Sass/Less is slightly less popular than not nesting or using manual CSS indenting (52% vs. 44%), which probably means that about half of all Sass/Less codebases don't nest much, and as you rightly imply there's always plain CSS which can't nest.

Personally I think that managing large JavaScript codebases gets a lot of discussion in the community (older yet essential books like JavaScript: The Good Parts saying to use a subset of the language so don't use with, etc), but CSS is a simpler and yet also problematic language with it's global (non-scoped) selectors and !important and such, so managing larger Sass/Less/CSS codebases is also often about removing problematic features. There are parts of the CSS community who consider nesting to be an anti-pattern as it encourages overly specific selectors (e.g. some people use a subset of Sass/Less, perhaps just variables and mixins).

Originally I just wanted this issue considered so I'm happy either way on how you judge this feature request. Again, thanks for your time.

[lydell]

Were adding these equal signs here a mistake?

[holloway]

Sorry that was a leftover from a merge conflict, now removed!

(there are linting errors too that I can't reproduce on my local machine... I'll try to clean up this PR over the next day)

[lydell]

Interesting related reading: stylelint used to have this feature (stylelint/stylelint#278, stylelint/stylelint#305), but it was removed (stylelint/stylelint#1309, stylelint/stylelint#1385).

[holloway]

Thanks, that is interesting! I wonder where they were getting their stats from and if they agree with SitePoint survey.

From reading through the Stylelint code they seem to have a very sophisticated (and comparatively complicated) approach to hierarchy. The comparable code for indenting in this PR is in getIndentTree, and regarding the code complexity (in general) I was trying to minimise the amount of codepath differences with a pass-through function so hopefully that issue doesn't apply, but obviously that's your call about whether it's justified in this case.

[lipis]

I agree with Stylelint, it looks weird to me to indent CSS (or SASS, Less) files. My point was that if the style get's too big, usually it's broken to multiple files to organize it better, so there is no need to indent to group the related classes.

[holloway]

Upgraded PR to Prettier 1.8.0 and now all tests pass.

[ryami333]

@lipis to me this type of indentation is essentially just a more explicit alternative to nesting with a pre-processor, which is wildly popular. Using an ampersand makes sense to some people (and this keeps out of their way, because this formatting is behind an option/flag), but to others it just doesn't. To those other people, the ampersand just represents an unnecessary abstraction which results in selectors that you can't 'search' your codebase for.

I agree that stylesheets should be split when they get too big, but I don't really see how that factors into this conversation at all. Using that logic you could argue that we wouldn't need prettier at all if we would just refactor all of our code into small enough modules.

[holloway]

Just adding more credence to indenting, Sass itself (both libsass and rubysass) have an output formatting option called nested which indents the output CSS in a very similar way.

Their docs describe it as:

Nested style is the default Sass style, because it reflects the structure of the CSS styles and the HTML document they're styling. Each property has its own line, but the indentation isn't constant. Each rule is indented based on how deeply it's nested.

Nested style is very useful when looking at large CSS files: it allows you to easily grasp the structure of the file without actually reading anything.

(I'm not so sure about the 'without actually reading anything' part but you get the idea!)

The source file needs to be formatted using Sass nesting and then the output CSS is indented like this,

.reposition {
  display: flex;
  align-items: center; }
  .reposition button[disabled] {
    opacity: 0.25; }

.maps {
  display: block;
  overflow: auto; }
  .maps .maps--disabled {
    opacity: 0.5;
    pointer-events: none; }

Personally I think a few more linebreaks between the CSS Rules would help readability, but regardless this seems to be a major example of a similar indenting style done for similar reasons of readability and seeing hierarchy at a glance.

Finally, a LibSass dev says of nesting,

we felt that nested and compressed were the most important ones (since nested is most likely to be used for debugging, and compressed is most likely to be used in production).

...which supports the idea that this is a bit more than a style; it's a useful way of visualising the css hierarchy to see whether it's as expected.

[lydell]

I feel like this indentation style is popular enough for inclusion. And it is not just a "I prefer once space before that" type of thing, but something that can be argued to actually improve the overall readability of the entire style sheet.

Now that we have the markdown-only --no-prose-wrap option, I don’t think it will be any problems adding a CSS-only option.

When it comes to the implementation, I haven’t taken the time to read it carefully enough to understand how it works fully, but it is pretty short and only affects printing of "css-rule", and the difficult part seems to be contained in the getIndentTree function. So maintenance cost should not be a problem, I think.

What I’d like to see now are non-happy-path tests.

Also, what’s performance like? Just want to make sure this is crazy much slower than without the option.

So, I’m 👍 to this now (even though I won’t use it myself).

[lydell]

Perhaps a real-word example is clearer?

// Element "section" preceded by element "s" shouldn’t be indented.

[lydell]

Typo: SEPERATOR → SEPARATOR

const SEPARATOR = "\0"; // NUL is invalid in CSS, so it's a safe separator.

[lydell]

I would like to see crazy SCSS and Less tests as well.

• What happens when an indented rule has nested rules?
• What happens when SCSS/Less/PostCSS interpolations are used in selectors?
• What about pseudo-classes (:hover) and pseudo-elements (::after)?
• What about selectors with commas? table th, table td?

And some case insensitiviy stuff. I guess TABLE TR should be indented under table?

[holloway]

Awesome, thanks @lydell! I'll try to address your feedback over the next few days. Commas are already supported, and the Bootstrap output CSS formatting example has some (and of course that's just some arbitrary CSS I chose, I don't imagine that people would normally format their output).

Just to add another question to the list:

• What happens with code comments?

[holloway]

For performance testing is there a tool that Prettier devs usually use? If there's nothing currently I could use my whats-the-damage package if that's ok... it can measure execution times, CPU, and memory usage.

[lydell]

Don't focus to much on the performance thing. It's enough to just run the CLI on a big piece of CSS with and without the option to see if there's a huge difference. For example, a mistake in the selector-checking loop could cause a massive slowdown, but if it's all correct we should be fine.

Also not that my questions in the bullet points in my above comments were intended more to be requests for additions to the tests, rather than just questions. Sorry for being unclear.

[lydell]

We need to add this to the CLI code as well.

[azz]

Gotta admit, I'm still completely confused as to how it doesn't cause massive git diffs.

From your example:

table {
  background: papayawhip;
}

  table tr {
    background: palegreen;
  }

    table tr.active {
      background: lime;
    }

If I delete the table tr block:

-  table tr {
-    background: palegreen;
-  }
-

Then format, and the final diff will be:

-  table tr {
-    background: palegreen;
+  table tr.active {
+    background: lime;
   }
-
-    table tr.active {
-      background: lime;
-    }

Similarly, the reverse has the same problems. Start with:

table {
  background: papayawhip;
}

  table tr.active {
    background: lime;
  }

Then add in between the two rules:

+  table tr {
+    background: palegreen;
+  }
+

Format, then the final diff is:

-  table tr.active {
-    background: lime;
+  table tr {
+    background: palegreen;
   }
+
+    table tr.active {
+      background: lime;
+    }

Both of those diffs are incredibly unfriendly for such a small change.

[lydell]

@azz That’s a really good point. A withdraw half of my +1. :+0.5:

[holloway]

Good point @azz, and it certainly has that ramification to diffs. There are relationships between CSS Rules that are being shown and changed, and there's no avoiding the need to reformat the 'child' branch now that 'parent' is gone. The flow-on effect of indenting (and diffs) would only affect 'child' CSS rules, and any other adjacent rules would be left as-is. To some degree it's like nesting in almost any language (XML, JS, Sass, SAX output, etc) when removing an intermediary level, however specific to CSS is that the hierarchy hasn't changed it's just our visualisation of it. If we transpose your example of deletion to nesting with a preprocessor there are similar issues, and we'd also manually replace & with tr, and likewise the hierarchy hasn't changed but we still need to update adjacent lines and indentation levels:

table {
-  tr {
-    background: papayawhip;
-    &.active {
+  tr.active {
-      background: lime;
+    background: lime;
-    } 
  }
}

Ultimately I think your diff, and the Sass diff, is what people who use those would expect. In the opposite scenario of a file that contains table and table tr.active then inserting a table tr between them then indenting the table tr.active would also be expected (to show that table tr.active is more specific than table tr). Affecting adjacent lines is a feature, not a bug!

[suchipi]

I'm okay with including this as long as @lydell is okay with including this, because he's most familiar with the CSS printer, and my main concern in regard to adding options is maintenance cost. Based on what he said, the maintenance cost shouldn't be too high, because this is relatively isolated- that said, I also wonder about performance.

[lipis]

In case this one will make it to a release, I guess there will be an option to enable it and by default it will be disabled.

[lydell]

Regarding the gotcha around causing large-y diffs, I think @holloway's example makes sense. When writing SCSS, it happens every now and then that I split up or join nested rules due and therefor cause indentation-only changes of some blocks of code, just like in the example. So this option is not much worse than using SCSS.

[holloway]

@lipis Yes the proposal is for it to be an option that's disabled by default (see vjeux's feedback and 4.)

[holloway]

(just copying my comment here for visibility because I didn't realise that I posted it on a hidden outdated diff...sorry for the Github notifications spam)

I've just pushed a new version with CLI mode, comment indenting, nested styles, and more tests. The indenting or passthrough code is now a single line of code too.

For nested styles the code skips indentation changes for that block, but it retains any indentation from the outer selector. It's hard to explain but I think it looks right,

before:

div {
  background: papayawhip;
}

div s {
  background: palegreen;
}

div s.ghost {
  background: palegoldenrod;

  .coast {
      background-color: gray;
  }
}

div s.dancer {
  // this should be indented with previous nested style
  background: palevioletred;
}

div section {
  // this shouldn't be indented with "div s" because although it starts with the same characters
  // we treat element name as atomic.
  background: papayawhip;
}

after

div {
  background: papayawhip;
}

  div s {
    background: palegreen;
  }

    div s.ghost {
      background: palegoldenrod;

      .coast {
        background-color: gray;
      }
    }

    div s.dancer {
      // this should be indented with previous nested style
      background: palevioletred;
    }

  div section {
    // this shouldn't be indented with "div s" because although it starts with the same characters
    // we treat element name as atomic.
    background: papayawhip;
  }

So for div s.ghost that outer selector for the nested style is used for indenting, but the inner ones are ignored and they just use the existing Prettier formatting. If it didn't skip the .coast then I'm not sure what would be expected...the code might have to resolve the full selector and perhaps resolve & and so on, which (I think) would make the code far too complex.

The comment indenting looks like this,

before

a {
    text-decoration: none;
}
// a comment that should be indented at same column as following selector
a:hover {
    text-decoration: underline overline; 
}

after

a {
  text-decoration: none;
}
  // a comment that should be indented at same column as following selector
  a:hover {
    text-decoration: underline overline;
  }

This way if there's an indentation shape any comments will adhere to it. This example also shows pseudo classes, and pseudo elements work the same way.

For performance just using time on the command line it's averaging 0.254s without indent-styles and 0.267s with.

And some case insensitiviy stuff. I guess TABLE TR should be indented under table

This makes sense per-spec for HTML, but I suppose there are extremely niche cases like XML CSS that afaik are case-sensitive. More popular would be Attribute values which seem to be case-sensitive (a[data-amiga="cOmMoDoRe"] !== a[data-amiga="CoMmOdOrE"]) unless there's an i flag. These per-spec rules are somewhat complex, and I wonder if people writing TABLE TR followed by table tr td is also a niche case to support. Maybe this aspect of the code should be left as-is to keep the code simpler in getSelectorPattern, even if it is wrong per-spec. I don't have a strong opinion here though so I'll make elements insensitive with the next PR revision unless you suggest otherwise.

TODO:

• More tests for every CSS node type appearing in the file.
• Confirm whether any of these nodes should be indented too. I'm hoping not because I wouldn't want to affect much more of the main code-paths. The current test file has a Sass variable declaration between two selectors
• "What happens when SCSS/Less/PostCSS interpolations are used in selectors?"
• Anything else?

I'll try to get a new version in the next week.

Thanks so much Lydell!

[lydell]

Well, then I'll have to move my comment as well.

Re. .coast: That's the expected output. I wonder if it is possible to trick the indenter into indenting too much if you use something like this?

div {
  div.foo {}
}
div.foo.bar {}

The expected output is:

div {
  div.foo {}
}
  div.foo.bar {}

and not:

div {
  div.foo {}
}
    div.foo.bar {} // A mistake in the logic could cause double indentation here. 

Re. performance: Great, nothing to worry about there, I guess.

Re. case insensitivity: I totally understand that it is an edge case, but Prettier already lowercases stuff, so we have to do it in order to make the formatting stable. We don't want this to happen:

TABLE {}
table.zebra {}

→

table {}
table.zebra {}

→

table {}
  table.zebra {}

[holloway]

Thanks, so I'll add that indenter trick as a test (currently the indenter passes that test).

For case insensitivity.. that makes a lot of sense, so I'll definitely do it and adhere to the same normalization rules that Prettier already has.

[lipis]

Point taken :)

[holloway]

Cool, thanks dude :)

[thibaudcolas]

I just made a repo with some real-world code using this formatting if anyone wants to further play with it: https://github.com/thibaudcolas/prettier-indent-styles.

Formatting code like this is useful in a codebase with a preprocessor (and BEM/ITCSS), to discourage nesting and keep specificity in check. Preprocessor nesting and & interpolation are easily abused, and make individual nested selectors appear deceptively short compared to their actual specificity in the resulting CSS. This formatting makes it easier to break up styles into separate blocks, while perserving the visual aid of nesting.

Here is a with/without example, shorter than what real-world styles would look like but still demonstrates what problem this helps address:

// The pyramid of doom in CSS land. Menus are a classic example.
.nav-list {
  li {
    display: inline;

    a {
      text-transform: none;

      // This will become `.nav-list li a.active`.
      &.active {
        text-transform: underline;
      }
    }
  }
}

// Breaking up the pyramid.
.nav-list {
  li {
    display: inline;
  }
}

  // This makes it clear the `li` is actually not needed.
  .nav-list li a {
    text-transform: none;

    // We could go further and also de-nest this to have a selector with 2 classes only.
    &.active {
      text-transform: underline;
    }
  }

And the BEM equivalent:

// The BEM pyramid of doom. Specificity is low but it gets hard to see what is going on.
.nav-list {
  &__item {
    display: inline;

    &__link {
      text-transform: none;

      &--active {
        text-transform: underline;
      }
    }
  }
}

// Breaking up the pyramid. Still using reasonable selector interpolation.
.nav-list {
  &__item {
    display: inline;
  }
}

  .nav-list__link {
    text-transform: none;
    &--active {
      text-transform: underline;
    }
  }

---

For diffs, I haven't suffered much from this. The changes caused by this are only whitespace, which diff tools often ignore. Preprocessor nesting has the same drawback, except you also need to update the & interpolations.

The main drawback of this formatting is that doing it manually takes lots of tedious indenting and de-indenting to maintain the visual nesting as you refactor selectors. But that's exactly why it would be good to have this automated.

[azz]

I wanted to get a (relatively) unbiased sampling of opinions on Twitter. I have a suspicion that the SitePoint survey suffers from people reading "Do you indent your CSS to match HTML hierarchy" and clicking "Yes" (because they use a postprocessor) without reading the responses carefully. People often whiz through those surveys really quickly, and it didn't have an image to describe what people were voting for.

I'm worried that the first couple of comments on the survey are just going to set the tone for all of the remaining commentary

I don't think that really happens - people are always keen to argue things on the internet.

---

I suppose my main concern with this is that it's an alternative style, when Prettier historically has only supported one "opinionated" style. The options introduced are for highly controversial things (tabs vs spaces, quotemarks, etc), and none of them significantly affect formatting (tabWidth excluded).

Every now and again, someone will ask to format vars like this:

var name     = 1;
var longName = 2;

Even though that style is perfectly valid and somewhat popular (perhaps more popular than this CSS style), we haven't implemented it to encourage consistency. The variable alignment style also suffers git diff problems when automated, same as this PR.

I think this code really belongs in stylelint (or a stylelint plugin), then prettier-stylelint could do this formatting. But it was removed due to "It [being] a seldom-used feature that was complex and difficult to maintain in core."

[azz]

Twitter results are in:

If you take out the 14% who voted for (show results), 90% don't indent CSS.

Not going to make any claims about the veracity of a Twitter poll, but it confirms my suspicion that this isn't a hugely popular style.

[holloway]

Yep, the Stylelint example was mentioned already and so I had a good look at their code. Their technical approach certainly was very complex (and, more to the point, very different) whereas Prettier has been fairly straight-forward to adapt. This PR is essentially using the existing CSS printer while wrapping some indent()s if preceding selectors are substrings of the current selector. As I'm sure you can appreciate implementations of features can vary greatly. (hey just for my knowledge is there a way of replacing my path.pushCall() with a normal path.call()? Would I set values on the path and use .call() to put them on the stack by name? that was one bit of my code that I wasn't too happy with).

I suppose my main concern with this is that it's an alternative style

Now this is a point that I think is quite reasonable, and yeah that's the main point for me too. I guess that's why Lydell responded saying:

And it is not just a "I prefer once space before that" type of thing, but something that can be argued to actually improve the overall readability of the entire style sheet.

For me that's the main point. It's a bit more than tabs vs spaces and (imo) tedious arguments like that... instead it's like auto-generating headings in your CSS. And I guess you've read the Sass devs talk about debugging stylesheets with this, and so on?

Anyway... is this your call, and you're killing the PR? Or is it something where all the existing contributors vote ... what's next?

I'll accept the decision of course, and no hard feelings. Obviously I think this PR warrants inclusion because it's more like a feature than a mere style, but I'll wait for whatever happens next. I didn't want to talk about your 'survey suspicions' because, well, obviously there's no way to prove or disprove hunches so I'm not sure if that would be very productive. Cheers :)

[azz]

There isn't really a procedure in place for these decisions, usually there is some consensus among collaborators. If not, I guess @vjeux has the final word.

[vjeux]

Our guidelines around options is that the default stance is that we should not add any. If there’s a huge demand for a new option and adding it would unblock a sizeable amount of people that couldn’t use it otherwise, then, and only then, should we consider adding it.

This particular option doesn’t seem like it falls in the second category I’m afraid.

[holloway]

@vjeux Ok. It's good to finally know.

I really hope you're not basing it on a twitter poll of ~750 though, when there are much larger datapoints around.

[impressivewebs]

Hi everyone,

I'm the one that organized the SitePoint surveys over the past two years. It's possible that @azz is correct, that many people clicked "yes" without thinking it through.

But, that being said, what were they answering "yes" to? I don't buy the answer that they were equating "indenting" to using a pre- or post-processor. Nesting isn't the same as indenting (if that's what was assumed). I'd like to think the respondents were smart enough to understand that difference. (But then again, in the first survey, tons of respondents said they used CSS Hacks for IE11, when in reality, they were saying they supported IE11).

I think it's safe to assume in that case that most understood the question. The numbers still might be skewed because of those who didn't understand it, but that would still leave a decent number, IMO. Also, there was an option for "I don't know what that means", to cover those who didn't get it. So again, I feel like it was pretty clear and didn't encourage false positives.

To be honest, the Typeform survey app is nice, but it would be better if I could include images and code examples with questions, without just linking to stuff. If I do this survey again, I'll definitely try to improve on the wording of that question.

In my opinion, I think it would be fine if Prettier had an option for this, but I'm not necessarily married to the feature. I do it in my own code, but not consistently; only in areas where I feel it will help with maintenance.

[duailibe]

I originally was against this feature, now I'm neutral and wouldn't mind having it. That said, IMO, this would be better living in a separate tool that can run after prettier (prettier-eslint, prettier-standard, etc.). But I think the same about almost all of our other options (the exception being tabWidth).

I think this falls in the same category of the other options we started to support. I missed the discussion of when they were initially supported so I don't know how much demand there was, but I think this brings more value than something like noBracketSpacing or jsxBracketSameLine (no wonder people use it so much as an argument "why don't you support X option if you support Y?").

[xiaoyu0519]

good

[aaronkahlhamer]

An associate was dissuaded from using Prettier because of it removing css hierarchy indent. The person heavily invests in nesting to get one's bearings. I appreciate hierarchy indent, but prefer Prettier over it. A fear is, I'll be the only person using Prettier and will accidentally overwrite their formatting when providing an assist. All members of the team have to be on board to make Prettier the authoritative code formatter.

A baker's dozen, many who are freshman coders who recently started learning how to code this year! and have no clue about the 2016 SitePoint survey, will be using Prettier once this feature is enabled. Looking forward to it!

[j-f1]

@holloway Can you merge (or rebase) your PR so it’s mergeable again?

@lydell Does your ❌ review still stand?

[holloway]

@j-f1 done!

Initially I didn't know why the snapshot tests were different, but then I realized that since v1.9.0 (#3317) the CSS normalization rules had changed. So it's a positive sign that whatever the normalization rules Prettier has this code will follow 😊

[lipis]

So this is going in? :|

I don't remember what changes I requested anymore

[holloway]

@lydell Cheers! I think I'd responded to all the specific things that you wanted changed (and everyone else's like @duailibe and @ikatyang too).

I've updated the first comment at the top of this PR with a summary of the polls so far.

One thing I thought might make sense to decrease the maybeIndentStyle calls in the CSS comment code so I've just done that...

before:

if (text.indexOf(rawText) === -1) {
  if (n.raws.inline) {
    return maybeIndentStyle(
      concat([path.indentStyleAsWhitespace || "", "// ", rawText])
    );
  }
  maybeIndentStyle(
    concat([path.indentStyleAsWhitespace || "", "/* ", rawText, " */"])
  );
}
return maybeIndentStyle(
  concat([path.indentStyleAsWhitespace || "", text])
);

after:

let concatComment;
if (text.indexOf(rawText) === -1) {
  if (n.raws.inline) {
    concatComment = ["// ", rawText];
  } else {
    concatComment = ["/* ", rawText, " */"];
  }
} else {
  concatComment = [text];
}
concatComment.unshift(path.indentStyleAsWhitespace || "");
return maybeIndentStyle(concat(concatComment));

@lipis No idea, sorry.

[azz]

I'm doing a clean up of stale PRs, closing this one. As @vjeux #3038 (comment) mentioned there isn't significant demand for this feature, and it would mean Prettier could produce significantly different code (where most options are fairly small/low maintenance).

@holloway I'd recommend adding this option to a fork of Prettier, or adding it as a stylelint plugin used with prettier-stylelint.