rtts / djhtml

Django/Jinja template indenter
GNU General Public License v3.0
582 stars 32 forks source link

Broken indention for scss files #81

Closed GitRon closed 1 year ago

GitRon commented 1 year ago

Hi @JaapJoris

I have a scss file with multiple declarations beneith each other:

.icon-x-alarm:before { content: '\e800'; } /* '' */
.icon-x-align-center:before { content: '\e801'; } /* '' */
.icon-x-align-justify:before { content: '\e802'; } /* '' */
.icon-x-align-left:before { content: '\e803'; } /* '' */
.icon-x-align-right:before { content: '\e804'; } /* '' */
.icon-x-anchor:before { content: '\e805'; } /* '' */

What the linter does is this:

grafik

But that's the only bug I found so far 💪

Best from Cologne Ronny

JaapJoris commented 1 year ago

Hi Ronny, and thank you for reporting this. This bug is actually worse than it seems: string detection in CSS mode is done "greedily", so that the following:

.icon1:before { content:'foo'} .icon2:before { content:'bar'}

incorrectly leads to the substring 'foo'} .icon2:before { content:'bar' being detected as an entire string (because it is technically the largest substring surrounded by quotes).

This was already fixed for DjJS mode, and I'll now also fix it for DjCSS mode and create a new release. Thanks again for noticing this so quickly!

GitRon commented 1 year ago

Thx for fixing it that quickly! When will you release the bugfix?

GitRon commented 1 year ago

It works 💪

DmytroLitvinov commented 1 year ago

I am using main branch in pre-commit and we are having also strangle indentation in scss files.

Initial scss code:

$line-height:           1.5 !default; // calculating line-height: round($font-size * 1.5)
$line-space:            10px !default;

And how it indented:

$line-height:           1.5 !default; // calculating line-height: round($font-size * 1.5)
                                                                  $line-space:            10px !default;

Another one:

  // START: Quick dirty fix for Boostrap alert
  .alert {
    position: relative;
    padding: 0.75rem 1.25rem;
    margin-bottom: 1rem;
    border: 0 solid transparent;
    border-radius: 0;

And result:

  // START: Quick dirty fix for Boostrap alert
            .alert {
    position: relative;
    padding: 0.75rem 1.25rem;
    margin-bottom: 1rem;
    border: 0 solid transparent;
    border-radius: 0;

Seems like the issue somewhere with comments

DmytroLitvinov commented 1 year ago

Adn this one is also strange:

image
DmytroLitvinov commented 1 year ago

here is more information how it affects.

image image image image
JaapJoris commented 1 year ago

Thanks for reporting these bugs. They all stem from the problem that my understanding of SCSS and, by extension, DjCSS's understanding is limited. The first bug you reported is caused by DjCSS not being aware that // starts a comment. The following:

div {
    foo: bar; // comment: ignore
    font-weight: bold;
}

therefore gets indented as this:

div {
    foo: bar; // comment: ignore
                          font-weight: bold;
}

Because DjCSS thinks that comment: is a CSS attribute without the trailing ;. Adding one fixes it:

div {
    foo: bar; // comment: ignore;
    font-weight: bold;
}

The second problem is that DjCSS treats text between ( and ) no different than ordinary attribute/value pairs in CSS. Note that the following:

@font-face {
    src: url('Arial.woff2') format('woff2'),
         url('Arial.woff') format('woff');
}

is consistent with the following, from DjCSS's perspective:

@include button-outline-variant(
    $color: $white,
            $hover-background;
)

This is a harder problem, but I think it can be fixed by special-casing stuff between ( and ).

Are there any further SCSS peculiarities that I'm not yet aware of?

DmytroLitvinov commented 1 year ago

Thank you for explaining @JaapJoris .

Here are more examples. Can you check whatever is related to the issue?

  1. Not sure that closing parenthesis

    image
  2. I would expect it under screen.

    image
  3. Maybe this one would be interested also

    image
oselz commented 1 year ago

Here is another simple example I just hit:

$grid-breakpoints: (
    xs: 0,
    sm: 576px,
    md: 768px,
    lg: 992px,
    xl: 1200px
) !default;
$grid-breakpoints: (
                     xs: 0,
      sm: 576px,
      md: 768px,
      lg: 992px,
      xl: 1200px
    ) !default;
JaapJoris commented 1 year ago

I believe all these problems are now fixed in the latest release :partying_face:

If you find any further problems, please open a new issue with examples of both the expected and actual output. And a special request: no screenshots please, it was really annoying having to type out the information from a picture instead of being able to copy the text :pray:

GitRon commented 1 year ago

Hi @JaapJoris

I updated to the latest version and now get this odd behaviour 😅

grafik

Best from Cologne
Ronny

GitRon commented 1 year ago

Ooops, sorry, will create a new issue.