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

No line break insertions in short CSS selectors #5086

Open
janosh opened this issue Sep 12, 2018 · 36 comments · May be fixed by #11783
Open

No line break insertions in short CSS selectors #5086

janosh opened this issue Sep 12, 2018 · 36 comments · May be fixed by #11783
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS

Comments

@janosh
Copy link

janosh commented Sep 12, 2018

Prettier 1.14.2
Playground link

--parser babylon
--no-semi

I suggest reconfiguring the changes made in #1962 so as to only insert line breaks in CSS selectors when they exceed a given length. Otherwise, you end up with unnecessarily long files in cases such as the one below.

Input:

import styled from 'styled-components'

import mediaQuery from '../../utils/mediaQuery'

const Participants = styled.section`
  padding: 2rem;
  background: ${props => props.theme.mainWhite};
  display: grid;
  grid-gap: 2rem;
  h1, h2, h3, h4 {
    width: 100%;
    padding: 0.5rem 2rem;
    background: ${props => props.theme.lightGreen};
  }
  ${mediaQuery.phone} {
    h1, h2, h3, h4 {
      font-size: 1rem;
    }
  }
  ${mediaQuery.tablet} {
    grid-gap: 1rem;
    h1, h2, h3, h4 {
      grid-column: 1/-1;
    }
  }
  ${mediaQuery.minTablet} {
    h1, h2, h3, h4 {
      white-space: nowrap;
      grid-row: 2;
    }
  }
`

export default Participants

Output:

import styled from "styled-components"

import mediaQuery from "../../utils/mediaQuery"

const Participants = styled.section`
  padding: 2rem;
  background: ${props => props.theme.mainWhite};
  display: grid;
  grid-gap: 2rem;
  h1,
  h2,
  h3,
  h4 {
    width: 100%;
    padding: 0.5rem 2rem;
    background: ${props => props.theme.lightGreen};
  }
  ${mediaQuery.phone} {
    h1,
    h2,
    h3,
    h4 {
      font-size: 1rem;
    }
  }
  ${mediaQuery.tablet} {
    grid-gap: 1rem;
    h1,
    h2,
    h3,
    h4 {
      grid-column: 1/-1;
    }
  }
  ${mediaQuery.minTablet} {
    h1,
    h2,
    h3,
    h4 {
      white-space: nowrap;
      grid-row: 2;
    }
  }
`

export default Participants

Expected behavior:

No insertion of line breaks in CSS selectors of length less than, say, 80.

@j-f1 j-f1 added lang:css/scss/less Issues affecting CSS, Less or SCSS status:needs discussion Issues needing discussion and a decision to be made before action can be taken labels Sep 13, 2018
@j-f1
Copy link
Member

j-f1 commented Sep 13, 2018

Perhaps we shouldn’t break if all of the sectors are bare element selectors.

@lydell
Copy link
Member

lydell commented Sep 13, 2018

@j-f1 Sounds reasonable. Thinking back of the previous issues, I think that’s the primary use case.

@janosh
Copy link
Author

janosh commented Jan 20, 2019

Just out of curiosity, was this issue addressed by any PR?

@lydell
Copy link
Member

lydell commented Jan 20, 2019

@janosh No PR for this so far!

@j-f1 j-f1 added help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! and removed status:needs discussion Issues needing discussion and a decision to be made before action can be taken labels Mar 5, 2019
@j-f1
Copy link
Member

j-f1 commented Mar 5, 2019

Let’s go with the heuristic of not automatically breaking selectors that consist of just element selectors.

@syntag
Copy link

syntag commented Mar 5, 2019

Let’s go with the heuristic of not automatically breaking selectors that consist of just element selectors.

This would undeniably be better default behavior than the current implementation. And since this is a relevant topic, I still think it's worth considering offering a config for this, whether it be toggling multiline for style selectors or setting some kind of max-len as referenced here #5937

It can sometimes be more preferable to read/write:

.heading, h1, h2 {
   margin: 0;
}

as opposed to:

.heading
h1,
h2 {
  margin: 0;
}

Prettier has handled this for Javascript, so what I am proposing is something similar for CSS too. For example, it's generally more preferable to read/write the following:

const { Header, Footer, Nav } = require('./layout');

as opposed to:

const {
  Header,
  Footer,
  Nav,
} = require('./layout');

@lydell
Copy link
Member

lydell commented Mar 5, 2019

I think we should keep things as they are. Keep it simple! As of this writing, there are no 👍s on this issue.

@lydell lydell removed the help wanted We're a small group who can't get to every issue promptly. We’d appreciate help fixing this issue! label Mar 5, 2019
@alexander-akait
Copy link
Member

I think we can close issue, printing in one line unreadable

@syntag
Copy link

syntag commented Mar 5, 2019

I think we can close issue, printing in one line unreadable

To say the one-line code examples given above are unreadable is a bit of an over exaggeration.

I think @j-f1 idea would be a great solution to this.

@lydell
Copy link
Member

lydell commented Mar 6, 2019

Feel free to provide more examples! 👍

@syntag
Copy link

syntag commented Mar 7, 2019

Here are some more real examples of scenarios where it's nicer to have one line:

*, *:before, *:after {
  box-sizing: border-box;
}
ul, ol {
    margin: 0;
    padding: 0;
    list-style: none;
}
h1, h2, h3, h4, h5, h6 {
  margin: 0;
}

There are a lot of cases like this where it's simpler to keep things compact. Putting each selector on its own line is nice, but only really necessary when you start selecting more things.

@janosh
Copy link
Author

janosh commented Mar 7, 2019

@lydell I'm a little surprised that this issue is even debated. I was under the impression that the point of prettier is not to be simple in and of itself but to produce ideally formatted code. I can't imagine anyone would consider this to be ideally formatted,

h1,
h2,
h3,
h4,
h5, 
h6 {
  margin: 0;
}

especially when compared to this

h1, h2, h3, h4, h5, h6 {
  margin: 0;
}

Breaking code onto multiple lines only once it exceeds a certain length is such a common practice. I don't see why it would be more difficult or less useful with CSS selectors than anywhere else.

@lydell
Copy link
Member

lydell commented Mar 8, 2019

I think Prettier already has too many heuristics that break out of the simple rules. For example, #5939. What usually happens when we add stuff like this is that people open a new issue wondering why their code is/isn't broken across several lines in some cases.

I see what you mean with the h1, h2, h3, h4, h5, h6 and ul, ol examples. I've experienced those too in basically all projects I've worked on. But I only write those selectors one or two times in the entire project, so it didn't bother me much.

But maybe this is fine to add. I guess I'm just a little more hesitant to changes since the recent ternary change backlash: #5814 (for example). Basically every time we change something new people come and want it changed back. And this issue only has 2 👍s so far.

@janosh
Copy link
Author

janosh commented Mar 8, 2019

@lydell I certainly understand the reluctance to introduce additional heuristics. Just to be clear though, I wasn't advocating for one. I'm suggesting to only break CSS selectors onto multiple lines if they exceed a certain length in general. Is there a reason why this isn't the current behavior?

@lydell
Copy link
Member

lydell commented Mar 8, 2019

Ah, I see. It was decided early on to print every selector on its own line: #2057

@janosh
Copy link
Author

janosh commented Mar 8, 2019

Thanks for digging that up! Interestingly, that issue also didn't receive any upvotes. Seems like there is low interest in CSS selectors in general. FWIW, I believe making that change was a misstep.

@lydell
Copy link
Member

lydell commented Mar 8, 2019

Regarding upvotes – that issue is from when Prettier was not even half a year old, and the CSS support was even younger than that. Things were moving quickly, and since then the bar for changes has grown increasingly higher.

@sublimeye
Copy link

So is it possible to reconfigure this behavior?

@lydell
Copy link
Member

lydell commented Aug 20, 2019

@sublimeye There's no option for this currently.

@janosh
Copy link
Author

janosh commented Aug 20, 2019

I think @sublimeye may have been referring to reverting #2057, i.e. only breaking CSS selectors onto multiple lines if they exceed a certain line length.

@lydell
Copy link
Member

lydell commented Aug 20, 2019

There are no plans to change this currently, either.

@syntag
Copy link

syntag commented Aug 20, 2019

@lydell It's a shame really. I understand that you're looking for an overwhelming consensus before taking any action since you probably have more important things to focus on.

But with Preitter being an already opinionated formatter, I was at least hoping that even though this thread lacks popularity, that the change request shares similar values that were already made on other languages for virtually the same scenario in Javascript. I don't understand why SCSS is being treated differently.

I'd least hope that we can look at this change request objectively and see that implementing it would not only make sense for Preitter, but add consistency across different languages.

@alexander-akait
Copy link
Member

@syntag instead says shame you can help, we help you on a voluntary basis, and it’s very unpleasant to hear it

@syntag
Copy link

syntag commented Aug 21, 2019

@evilebottnawi I didn't mean to come off rude or disrespectful. I appreciate all that you do, and I understand how it is to work on a volunteer basis. I would certainly help if I had the capabilities, but I'm still fairly to new Javascript.

I'm just very passionate about using Prettier for my projects and was hoping to guide this change request in a direction where it might be considered for a future update, despite its lack of popularity.

@fisker
Copy link
Sponsor Member

fisker commented Aug 22, 2019

Code Guide suggest:

When grouping selectors, keep individual selectors to a single line.

https://codeguide.co/#css

airbnb/css

When using multiple selectors in a rule declaration, give each selector its own line.

https://github.com/airbnb/css#selectors

stylelint-config-standard has

"selector-list-comma-newline-after": "always",

https://github.com/stylelint/stylelint-config-standard/blob/5d1531025526324abdfc6d1fbf40b5fb9f2d7c2f/index.js#L95

normalize.css follows the same style

https://github.com/necolas/normalize.css/blob/fc091cce1534909334c1911709a39c22d406977b/normalize.css#L125-L126

bootstrap

https://github.com/twbs/bootstrap/blob/66e9ab3e4dd0a5fe501e8499e45bbd7488b80fd0/scss/_reboot.scss#L14-L16

https://github.com/twbs/bootstrap/blob/ecaefed92043fa889a0fb4c6100450ca6c29e9ef/site/static/docs/4.3/assets/scss/_component-examples.scss#L140-L145

@aravindvnair99
Copy link

I feel it makes better sense to split to multiple lines when it goes beyond a certain number of characters as pointed out by @syntag at #5086 (comment), @janosh at #5086 (comment), @j-f1 at #5086 (comment)

Also, as per the official Prettier - Options page, it says

For readability we recommend against using more than 80 characters:

In code styleguides, maximum line length rules are often set to 100 or 120. However, when humans write code, they don't strive to reach the maximum number of columns on every line. Developers often use whitespace to break up long lines for readability. In practice, the average line length often ends up well below the maximum.

Prettier, on the other hand, strives to fit the most code into every line. With the print width set to 120, prettier may produce overly compact, or otherwise undesirable code.

So why not apply the same logic to CSS as well? Put as many as 80 characters and only split when the code is untidy instead of splitting for each selector which makes it tough to scroll or decreases readability in the case of long CSS files. I feel an option to choose between the two styles would be nice to have.

@Jason-2020
Copy link

Is there any workaround?

@nickbclifford
Copy link

Hello from 2021! Have there been any updates or workarounds for this since the last post?

@janosh
Copy link
Author

janosh commented Mar 8, 2021

@nickbclifford It seems unlikely that Prettier will change its behavior here. But a simple workaround is to use the CSS pseudo-classes :is and :where (they only differ in specificity).

Prettier will not break inside these selectors:

:is(h1, h2, h3, h4, h5, h6) {
  margin: 0;
}

If your selectors have a common parent or child, I wouldn't even consider this a workaround but the new best practice:

Old CSS

section h1,
section h2,
section h3,
section h4,
section h5,
section h6 {
  margin: 0;
}

New CSS

section :is(h1, h2, h3, h4, h5, h6) {
  margin: 0;
}

Browser support is 90% for :is and 81% for :where so unless you still care about IE, you can start using at least :is right away. 😎

@shirobachi
Copy link

It looks like I've the same problem..
I've

      p, h3, ul {
        margin: 0;
        padding: 0;
      }

Prettier make:

      p,
      h3,
      ul {
        margin: 0;
        padding: 0;
      }

I want to have:

      p, h3, ul {
        margin: 0;
        padding: 0;
      }

Is it possible?

@AlexandroPerez
Copy link

@Jason-2020 I was having the same issue, and was about to have all my .css files be ignored by prettier, when I stumbled upon the ignore comment, when I was looking for a way to do it.

@shirobachi So we can have the following in css:

/* prettier-ignore */
p, h3, ul {
  margin: 0;
  padding: 0;
}

@janosh and the following in javascript:

import styled from "styled-components";

import mediaQuery from "../../utils/mediaQuery";

// prettier-ignore
const Participants = styled.section`
  padding: 2rem;
  background: ${props => props.theme.mainWhite};
  display: grid;
  grid-gap: 2rem;
  h1, h2, h3, h4 {
    width: 100%;
    padding: 0.5rem 2rem;
    background: ${props => props.theme.lightGreen};
  }
  ${mediaQuery.phone} {
    h1, h2, h3, h4 {
      font-size: 1rem;
    }
  }
  ${mediaQuery.tablet} {
    grid-gap: 1rem;
    h1, h2, h3, h4 {
      grid-column: 1/-1;
    }
  }
  ${mediaQuery.minTablet} {
    h1, h2, h3, h4 {
      white-space: nowrap;
      grid-row: 2;
    }
  }
`

export default Participants;

Is a bit annoying to have to add a comment, and it has to be added before every block of code, or abstract syntax tree as referred to by prettier docs, that is to be excluded from formatting.

@bdlowery
Copy link

@Jason-2020 I was having the same issue, and was about to have all my .css files be ignored by prettier, when I stumbled upon the ignore comment, when I was looking for a way to do it.

THANK YOU. Now my meyer-reset doesn't look like complete shit every-time I add it to a project.

before (disgusting)
Pasted_Image_7_23_21__11_54_PM

after (better)
Pasted_Image_7_23_21__11_52_PM

@arimgibson
Copy link

Just a thought, has anyone tried to PR this idea? I wasn't able to find anything and 10000% understand the want to make sure a PR will be accepted before writing the code for it, but I wonder if that's the main thing holding this up. Clearly there's support for this being a setting, which also means those who are fans of the current use could still have their way. Even keep it as the default.

@ShahriarKh
Copy link

Any updates?

@uzbeki
Copy link

uzbeki commented Mar 28, 2022

any updates? Beautify extension already does this.

@heppokofrontend
Copy link

Similar problems
#15070

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang:css/scss/less Issues affecting CSS, Less or SCSS
Projects
None yet
Development

Successfully merging a pull request may close this issue.