Support libsass string quotes in the c/js api bindings.

[chriseppstein]

This code compiles but the tests fail and I'm not sure why -- I think it might be a libsass bug. I could use some help debugging it.

[chriseppstein]

@matryo @mgreter @xzyfer @saper any ideas about how to make this work? It's blocking us here at work I can't wait for the big refactor.

[saper]

One reason why it fails is because quoted flag gets discarded from time to time...

[xzyfer]

I'm jumping on a plane to SF in a couple hours, I'll take a look at a sensible hour today.

Regards, Michael

On Fri, Jun 19, 2015 at 9:56 PM, Marcin Cieślak [email protected] wrote:

One reason why it fails because quoted flag gets discarded from time to time...

— Reply to this email directly or view it on GitHub https://github.com/sass/node-sass/pull/1006#issuecomment-113695419.

[saper]

Well, basically quoted string support in libsass for custom functions is not implemented...

[saper]

C SASS API kind of expects that a quoted string will be, ... well, quoted.

So it's not just a flag to be set, one actually needs to convert the mystring to "mystring" or 'mystring'.

[saper]

I have prepared an early set of changes to libsass to make quoted strings work:

1. https://github.com/chriseppstein/node-sass/pull/1 modifies the test to provide an actual quoted string from the client (based on how currently API is constructed),

2. https://github.com/saper/libsass/tree/clonefix fixes passing of quoted strings via the C API in libsass.

[saper]

@chriseppstein Can you try applying https://github.com/sass/libsass/pull/1289 to libsass? May not fix all quoted-vs-unquoted string issues, but certainly lets us pass them over to node and back.

[mgreter]

@chriseppstein @saper IMO the merged PR on libsass side done by @saper does the correct thing. I can also give you a little background info why we have two string types (ATM). Strings that appear in sass are not always unquoted. There are "instances" that will always remain static (String_Constant) up to the output (preserving white-space and "comments" too). That's probably why you always want to deal with String_Quoted on the C-API side (looks like it simply never got implemented).

String_Quoted should work pretty straight forward from there on. You pass the "parsed" string to the constructor and it will be unquoted internally (storing the unquoted value and the quotemark flag). As a side note, I'm getting more confident that we could use a bool flag for the quotemark. We previousely had wrongly parsed some String_Constants as String_Quotes, which made it necessary to preserve the actual quotemark to pass the specs. Now we parse more of them correctly as String_Constant and should be able to get rid of explicitly storing the quotemark char, and instead always rely on autoquote rules. To get autoquote behavior (meaning we detect the best quotechar), you can set quotemark char to *.

[saper]

Ah now I start to understand. So maybe "String_Quoted" should not be derived class of String_Constant? Are they in some circumstances interchangeable? Because in that case we should use common base class (I can see there is "String")... need to read more...