Syntax: Multiline comments break for unsupported features

[am11]

While testing this issue: madskristensen/WebEssentials2013#1162, I came across a bug with multiline comments.

This code:

footer {
    color: red;
}

// Ampersand in SassScript:
/*.button {
    &-primary {
        background: orange;
    }

    &-secondary {
        background: blue;
    }
}*/

// Output:
.button-primary {
    background: orange;
}

.button-secondary {
    background: blue;
}

compiles to:

footer {
  color: red; }

/*.button {
    &-primary {
        background: orange;
    }

    &-secondary {
        background: blue;
    }
}*/
.button-primary {
  background: orange; }

.button-secondary {
  background: blue; }

But this:

footer {
    color: red;
}

/*@each $header, $size in (h1: 2em, h2: 1.5em, h3: 1.2em) {
    #{$header} {
        font-size: $size;
    }
}*/

// Ampersand in SassScript:
/*.button {
    &-primary {
        background: orange;
    }

    &-secondary {
        background: blue;
    }
}*/

// Output:
.button-primary {
    background: orange;
}

.button-secondary {
    background: blue;
}

throws on compile (as maps with multi-vars are not supported yet: #394).

But with single-line comments:

footer {
    color: red;
}

///*@each $header, $size in (h1: 2em, h2: 1.5em, h3: 1.2em) {
//    #{$header} {
//        font-size: $size;
//    }
//}*/

// Ampersand in SassScript:
/*.button {
    &-primary {
        background: orange;
    }

    &-secondary {
        background: blue;
    }
}*/

// Output:
.button-primary {
    background: orange;
}

.button-secondary {
    background: blue;
}

it compiles to:

footer {
  color: red; }

/*.button {
    &-primary {
        background: orange;
    }

    &-secondary {
        background: blue;
    }
}*/
.button-primary {
  background: orange; }

.button-secondary {
  background: blue; }

(Also note: multi-line comments are retained in the output, while single-line comments are not. I think they can be transformed to multi-line comments syntax for CSS valid output)

[am11]

@mgreter is this issue also fixed as part of #433?

[mgreter]

@am11: I dont think so! #433 is about custom functions; and the sass2scss update only matters if you feed .sass (indented syntax) files to libsass, which IMO is not the case here?

[am11]

Ops, I must have misread it. I thought the fixes are related to the SASS 3.x syntax functions, which (I guess) is causing this multi-line comments issue. :)

[akhleung]

Hmm, the text inside multi-line comments is interpreted by LibSass because things like variable substitutions are allowed, but it looks like it needs to treat the comment as raw text when interpretation would result in errors.

[am11]

@akhleung, thanks!

I tried changing it here:

libsass/parser.cpp

Line 47 in 97e5346

String* contents = parse_interpolated_chunk(lexed);

(and all the occurrences of this line in parser.cpp) like:

String*  contents;
try {
  contents = parse_interpolated_chunk(lexed);
}
catch (Error& err) {
  contents = new (ctx.mem) String_Schema(lexed, source_position);
}

and few other varieties in catch-block, but it still throws.

Any pointers on where to look to make it parse the comment as text (in case comment-code result in error)?

[HamptonMakes]

We definitely need a simpler test case here...

[malrase]

So I was playing around with this to find a working test case, but Sass 3.4 seems to throw an error for this, too.

Sass errors because there's no variable "$header" – meaning that it's seeing and trying to interpret the variable, but doesn't pay attention to the values given to it by the @each line.

http://sassmeister.com/gist/0277dd12d32a99cd97fd

It looks like libsass (at least in Sassmeister) gives a similar error about unbound variables. Could this be a Ruby Sass bug too?

[malrase]

(Note: if I set the variable "$header: h1" in either example before the comment, the code works fine.)

[malrase]

So if you're following the Sass issue, it looks like this is intended behaviour. See the comment by lolmaus here: sass/sass#1465 (comment)

[HamptonMakes]

Soooooooo, yeah, it now dawns on me what's going on here. This exact same sample causes the same error, because Sass interpolates inside of comments. The @each was a red-herring

/* #{$header} */

If $header is defined, it works just fine. If it's not... it breaks. This is the intended Sass behaviour and I can't believe I didn't think of comment interpolation.

[am11]

@malrase, thanks for looking into it. But wouldn't @akhleung's approach (above) makes sense? If it throws error in multi-line comments, wouldn't it be nice to raise a warning instead of syntax error? Something like:

Warning: Unbound variable in multi-line comment at line X, column Y in <file>.scss. 
         Did you mean to use single-line comment?

[HamptonMakes]

I think that might be nice, but we have to stick with Sass' choices right now. Maybe open an issue there, as I agree with you.

On Tue, Oct 7, 2014 at 1:11 PM, Adeel Mujahid notifications@github.com
wrote:

@malrase, thanks for looking into it. But wouldn't @akhleung's approach (above) makes sense? If it throws error in multi-line comments, wouldn't it be nice to raise a warning instead of syntax error. Something like:

Warning: Unbound variable in multi-line comment at line X, column Y in <file>.scss. 
         Did you mean to use single-line comment?

---

Reply to this email directly or view it on GitHub:
#424 (comment)