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

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

Open
Lexicality opened this issue Mar 30, 2016 · 22 comments · May be fixed by #849
Open

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

Lexicality opened this issue Mar 30, 2016 · 22 comments · May be fixed by #849
Assignees

Comments

@Lexicality
Copy link

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
Copy link
Member

bgriffith commented Apr 21, 2016

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
Copy link

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
Copy link
Member

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

@natorojr
Copy link

natorojr commented Jun 25, 2016

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
Copy link
Member

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
Copy link

eolognt commented Jun 30, 2016

Fixing this would be very appreciated.

@pmccloghrylaing
Copy link

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
Copy link

DaClan008 commented Oct 12, 2016

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
Copy link

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

@dominique-mueller
Copy link

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

@nicklee
Copy link

nicklee commented Mar 17, 2017

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
Copy link
Member

@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
Copy link

jhpratt commented Jul 7, 2017

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

@rudidude86
Copy link

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
Copy link

jhpratt commented Jul 12, 2017

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

@mysterycommand
Copy link

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

@vigcodes
Copy link

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

@miquelferrerllompart
Copy link

Same here, some news?

@nwpappas
Copy link

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

image

mmzoo pushed a commit to bukowskis/style-guide that referenced this issue Feb 5, 2019
mmzoo added a commit to bukowskis/style-guide that referenced this issue Feb 5, 2019
@beatrizsmerino
Copy link

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
Copy link

lachieh commented Apr 23, 2020

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.