util: Adds CRLF awareness to quote() #1817
Conversation
|
//cc @xzyfer =) |
| @@ -488,6 +488,12 @@ namespace Sass { | |||
|
|
|||
| int cp = utf8::next(it, end); | |||
|
|
|||
| // in case of \r, check if the next in sequence | |||
| // is \n and then advance the iterator. | |||
| if (cp == '\r' && it < end && utf8::peek_next(it, end) == '\n') { | |||
There was a problem hiding this comment.
Looking through the rest of this file it looks like we generally treat \r as a noop. I think you can drop the peek here.
There was a problem hiding this comment.
Great! Fixed by fb1fcd7.
There was a problem hiding this comment.
@xzyfer, seems like in some cases, we do need to push_back() the carriage return to quoted string: https://travis-ci.org/sass/libsass/jobs/99452071#L1108.
There was a problem hiding this comment.
On second though, \r is a legal character in Unix and Windows. Now I wonder if Ruby Sass is doing the right thing in first place. 💡
There was a problem hiding this comment.
If CSS3 import only accepts network paths, then \\r is legal string of length 2. :)
I have reverted it back to this original code. Now it aligns with Ruby both on Windows and OSX. CI seems to be happy too. 🌴
|
Nice find @am11 |
|
I wonder how we can test this since the spec for #1096 includes this test case and passes on Windows. |
|
Good point. Since we don't have |
|
Sounds unreliable. Especially give that we periodically regenerate all the specs (usually on osx or linux). |
This is to test sass/libsass#1817.
|
@xzyfer, if you commit from Linux and OSX, it will not affect CRLF endings. I have just committed sass/sass-spec#664 from OSX. |
Input: ```scss @import url("a b"); ``` Output on Windows: ``` "@import url(\"a\r\a b\");" ``` Output on Unix (alinged with Ruby Sass, hence the expected output): ``` "@import url(\"a\a b\");" ``` See: sass#1096 (comment) for details.
This is to test sass/libsass#1817.
Input:
Output on Windows:
Output on Unix (alinged with Ruby Sass, hence the expected output):
See: #1096 (comment)
for details.
Sepc: sass/sass-spec#664.