sasstools / sass-lint

Pure Node.js Sass linting
MIT License
1.77k stars 532 forks source link

Indentation rule doesn't like alignment in multiline structures #591

Open Lexicality opened 8 years ago

Lexicality commented 8 years ago

In structures such as

#loading-screen {
    // Magic animations
    visibility: hidden;
    transform: translateX(-100%);
    // Delay the visibility until we've drifted out
    transition: transform $in-out-speed,
                visibility 0s $in-out-speed;
    transition-timing-function: linear;
}

or

@font-face {
    font-family: "Freight Sans Pro";
    src: url("../fonts/FreightSansProMedium.woff") format("woff"),
         url("../fonts/FreightSansProMedium.ttf")  format("truetype");
}

scss-lint 1.5.1 complains about the overflow line's indentation:

error-pages/_fonts.scss
  4:10  error  Indentation of 9, expected 4  indentation

The only way to appease it is to reduce the overhanging line to the base indentation of the block

{
    src: url("../fonts/FreightSansProMedium.woff") format("woff"),
    url("../fonts/FreightSansProMedium.ttf")  format("truetype");
}

or put it all on one line

{
    transition: transform $in-out-speed, visibility 0s $in-out-speed;
}

neither of which I am happy with.

bgriffith commented 8 years ago

Thanks for flagging this up. We're aware of quite a few issues with the indentation rule and you can expect them to be addressed soon, now that v1.6 is out of the door.

wbobeirne commented 8 years ago

Also adding to this, you get the Mixed tabs and spaces error if you are set to tabs, and trying to align the rules with spaces. You shouldn't do alignment with tabs (because someone's alignment with 2 space tabs is different from someone else's alignment with 4.)

Just wanted to make sure that was on someone's radar to test when they work on this!

DanPurdy commented 8 years ago

Thanks @wbobeirne definitely a good idea to doc it here as I've just got it in my head at the minute!

natorojr commented 8 years ago

This may or may not be related to this issue, but I'm also seeing indentation issues with multiline nested structures:

src/app/scss/modules/_base-mixins.scss
  53:3  warning  Expected indentation of 6 spaces but found 2  indentation
  54:5  warning  Expected indentation of 8 spaces but found 4  indentation
  55:5  warning  Expected indentation of 8 spaces but found 4  indentation
  56:5  warning  Expected indentation of 8 spaces but found 4  indentation
  57:5  warning  Expected indentation of 8 spaces but found 4  indentation
  58:5  warning  Expected indentation of 8 spaces but found 4  indentation
  59:5  warning  Expected indentation of 8 spaces but found 4  indentation

Caused by what should be considered perfectly nested SCSS:

$header-sizes: (
  small: (
    h1: 16,
    h2: 13,
    h3: 13,
    h4: 13,
    h5: 13,
    h6: 13
  ),
  medium: (
    h1: 16,
    h2: 13,
    h3: 13,
    h4: 13,
    h5: 13,
    h6: 13
  )
);

Important to note: I do not believe this was happening with 1.7.x. I just started seeing it after upgrading to 1.8.x, as it broke my build process (which runs sass-lint first).

DanPurdy commented 8 years ago

That's a separate issue @natorojr could you file a new issue please so we can keep track of it a little easier. Thanks.

eolognt commented 8 years ago

Fixing this would be very appreciated.

pmccloghrylaing commented 8 years ago

There's a few different ways people may want multiline indentation enforced.

// indent one level
{
    transition: transform $in-out-speed,
        visibility 0s $in-out-speed;
}
// perfect alignment
{
    transition: transform $in-out-speed,
                visibility 0s $in-out-speed;
}
// indent one level, enforce new line after ':'
{
    transition:
        transform $in-out-speed,
        visibility 0s $in-out-speed;
}

I've got a fork that enforces indenting one level: https://github.com/pmccloghrylaing/sass-lint/tree/indent-multiline-declaration, but it seems like there'll need to be an option for how multiline properties should be indented.

DaClan008 commented 7 years ago

I have a similar issue. I think is related.

I have @media-screen following another media screen and I get an indentation error


@media screen and (min-width: $bp-start), screen and (min-height: $bp-start)
    // ... some other settings

    .right-center, .featured   // These seems to be correctly indented
        height: 100%
        margin: auto

@media screen and (min-width: $bp-xxs)

    .container    // The place where I get Expected indentation of 8 spaces but found 4

ps I have updated my sass-lint to latest version this morning.

I am using vscode though, but don't think the issue lies there?

riesinger commented 7 years ago

I have exactly the same issue as @DaClan008 describes. This is very annoying....

dominique-mueller commented 7 years ago

Any updates on this? It's quiet annoying not being able to use multi-line indention ...

nicklee commented 7 years ago

We have the same issue here, would be great to get an update on when this will be implemented, single line multi transitions make me sad ;)

DanPurdy commented 7 years ago

@nicklee @dominique-mueller I haven't had any time to even look at this again recently, if someone wants to work on it and fix it then be my guest. Otherwise it unfortunately will have to wait until I get time again.

jhpratt commented 7 years ago

@DanPurdy Not a major issue for me, but now that display: grid is widely supported, multiline rules will be incredibly common.

rudidude86 commented 7 years ago

Just to expand on @jhpratt's comment, it's beneficial to have well-aligned, multiline values for the grid-template-areas property in particular. The arrangement of the values is expressive -- you kind of end up with ASCII-art for how your template is laid out:

grid-template-areas: "head head"
                     "nav  main"
                     "nav  foot";

As CSS Grid becomes more popular, I'm sure a lot of the community would find it beneficial to be able to lint for this situation!

Thanks!

jhpratt commented 7 years ago

That's exactly what I was referring to. Should've spelled it out ─ thanks for that.

mysterycommand commented 6 years ago

Any update on this? I see the epic (#680), but it appears locked and private.

vigcodes commented 6 years ago

Nothing yet? I'm still having the same issue.

miquelferrerllompart commented 6 years ago

Same here, some news?

nwpappas commented 5 years ago

This is a situation that I've been hoping to find a solution for.

image

beatrizsmerino commented 4 years ago

I have the same issue: I have temporarily disabled priority 2 but this is not really a fixed bug

Captura de pantalla 2020-04-23 a las 12 14 19

Captura de pantalla 2020-04-23 a las 12 17 25

lachieh commented 4 years ago

Has anyone said it was fixed? I see a lot of people asking for an update but no one actually investigating a solution except a fork by @pmccloghrylaing from 4 years ago.

Lexicality commented 4 years ago

These days I let prettier handle the alignment rather than trying to deal with this rule.