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

Multiline strings with backslashes are silently truncated #942

Closed
anlutro opened this issue Mar 13, 2015 · 10 comments · Fixed by #956
Closed

Multiline strings with backslashes are silently truncated #942

anlutro opened this issue Mar 13, 2015 · 10 comments · Fixed by #956

Comments

@anlutro
Copy link
Contributor

anlutro commented Mar 13, 2015

I have helpers like this in some of my code. These used to work fine, but are now silently truncated (no errors) so that anything after the slash and newline isn't included as part of the variable.

$media-medium: "screen and (min-width: #{$break-medium-min}) \
and (max-width: #{$break-medium-max})";

I can provide a sass-spec PR a bit later.

@anlutro
Copy link
Contributor Author

anlutro commented Mar 13, 2015

Oddly enough, the above now works when I remove the \ - maybe this is intentional? I actually prefer it without the \, but I still feel that it should be allowed - if it's not too much work to implement.

@mgreter
Copy link
Contributor

mgreter commented Mar 13, 2015

Libsass now much more correctly parses escaped chars. Maybe ruby sass has a special rule for lines with trailing backslashes, which I probably didn't encounter when I did the refactoring. Anyway, seems easy enough to support. But in term of CSS the backslash there does not make much sense IMO!

@anlutro
Copy link
Contributor Author

anlutro commented Mar 13, 2015

sass/sass#1237 may be of interest regarding the backslash. It was my understanding that backslashes to denote multiline strings was valid CSS and should therefore be valid Sass/SCSS.

@anlutro anlutro changed the title Multiline strings are silently truncated Multiline strings with backslashes are silently truncated Mar 13, 2015
@xzyfer
Copy link
Contributor

xzyfer commented Mar 16, 2015

Spec added sass/sass-spec#278

mgreter added a commit to mgreter/libsass that referenced this issue Mar 16, 2015
Handle trailing backslash + newline token
sass#942
@mgreter mgreter self-assigned this Mar 16, 2015
mgreter added a commit to mgreter/libsass that referenced this issue Mar 17, 2015
Handle trailing backslash + newline token
sass#942
mgreter added a commit to mgreter/libsass that referenced this issue Mar 17, 2015
Handle trailing backslash + newline token
sass#942
@am11
Copy link
Contributor

am11 commented Apr 3, 2015

I can reproduce this issue with current master.
In other words, sass/sass-spec#278 is failing.

@xzyfer xzyfer reopened this Apr 4, 2015
@xzyfer xzyfer added this to the 3.2 milestone Apr 4, 2015
@mgreter
Copy link
Contributor

mgreter commented Apr 7, 2015

Unfortunately I, Travis-CI and Appveyor cannot reproduce the issue.
@am11 I'm going to close this since we have this CI tested!
Please open a new issue with your output if problem persists!
P.s. I'm also always testing specs with perl-libsass on windows 😉

@mgreter mgreter closed this as completed Apr 7, 2015
@am11
Copy link
Contributor

am11 commented Apr 7, 2015

@mgreter, this only happens when git is configured to auto-correct endings. Lets say you type/paste the following in notepad:

$media-medium: "screen and (min-width: #{$break-medium-min}) \
and (max-width: #{$break-medium-max})";

(or the spec code)

notepad will surely add \r\n. 😎

@mgreter
Copy link
Contributor

mgreter commented Apr 7, 2015

Can you open a new issue for that with expected and actual output! Thanks!

@am11
Copy link
Contributor

am11 commented Apr 7, 2015

@mgreter done #1067. :)

@xzyfer
Copy link
Contributor

xzyfer commented Apr 8, 2015

There used to be a couple specs dealing with windows line ending but I fear that may have broken since we've auto generated the specs a couple times recently.

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.

4 participants