sass / libsass

A C/C++ implementation of a Sass compiler
https://sass-lang.com/libsass
Other
4.33k stars 462 forks source link

Indented syntax loud comment formatting is different than Dart Sass #2566

Open nex3 opened 6 years ago

nex3 commented 6 years ago

There are a few cases where LibSass's formatting of loud comments in the indented syntax differs from Dart Sass's (and Ruby Sass's):

I'm adding specs for this in sass/sass-spec#1215.

nex3 commented 6 years ago

It's worth noting that these are aesthetic rather than semantic concerns, so if you feel like your output is better you're allowed to leave it as-is. But it's probably better to align the implementations where possible.

mgreter commented 6 years ago

Might be related to https://github.com/sass/libsass/issues/1026

mgreter commented 6 years ago

Can you elaborate a bit how the formatting works? I fail to see the logic behind it!

no intial indentation

div { a {
/*************
       * a
       * multiline
       * comment
       */
      top: 10px;
} }

three spaces as indentation

div { a {
   /*************
       * a
       * multiline
       * comment
       */
      top: 10px;
} }

five spaces indentation

div { a {
     /*************
       * a
       * multiline
       * comment
       */
      top: 10px;
} }

seven spaces indentation

div { a {
       /*************
       * a
       * multiline
       * comment
       */
      top: 10px;
} }

As you can see the lines following the initial comment have different indentation every time! But I have no clue what logic this follows!

mgreter commented 6 years ago

BTW. the test were made with scss syntax! sass2scss preserves original input as much as possible!

mgreter commented 6 years ago

I tested with dart-sass and ruby sass and it mostly seem to correspond: One case where it differs is with ten spaces initial indentation:

div { a {
          /*************
       * a
       * multiline
       * comment
       */
      top: 10px;
} }
mgreter commented 6 years ago

AFAIR LibSass only adjusts the initial line to indent according to the scope and keeps the rest as is.

nex3 commented 6 years ago

For loud comments that come from SCSS, the relative indentation of each line of the comment should be preserved. In your examples in https://github.com/sass/libsass/issues/2566#issuecomment-373906449, the subsequent lines of the comment are indented seven, four, two, and zero levels deeper than the opening line, and that relative depth is preserved in the output when the entire comment is re-indented so that the shallowest line appears at the current indentation level.

In cases like https://github.com/sass/libsass/issues/2566#issuecomment-373906863, where the first line is deeper than the subsequent lines, it looks like neither Ruby nor Dart Sass handle that ideally. I'd expect this output:

div a {
     /*************
  * a
  * multiline
  * comment
  */
  top: 10px; }

which preserves the relative indentation and places the shallowest line at the current indentation level. But I think these types of comments are unlikely to appear in practice, so I wouldn't sweat their formatting too much.