Skip to content

Create quoted strings coming via API #1289

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

Merged
merged 1 commit into from
Jun 30, 2015
Merged

Conversation

saper
Copy link
Member

@saper saper commented Jun 21, 2015

Make sure quoted strings coming via API
are created as a String_Quoted instance.
Adjust string "quoted" property sent
via API on output.

Make sure quoted strings coming via API
are created as a String_Quoted instance.
Adjust string "quoted" property sent
via API on output.
@chriseppstein
Copy link
Contributor

👍

@@ -322,7 +322,7 @@ extern "C" {
return sass_make_color(val->color.r, val->color.g, val->color.b, val->color.a);
} break;
case SASS_STRING: {
return sass_make_string(val->string.value);
return sass_string_is_quoted(val) ? sass_make_qstring(val->string.value) : sass_make_string(val->string.value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if/else would be more consitent IMHO

@mgreter
Copy link
Contributor

mgreter commented Jun 23, 2015

LGTM for a first fix! I am aware that we need a better and more complete API for Number and String handling. My goal is to also have operations available to the C-Bindings, so they don't need to implement addition, multiplication, concat etc. themselve. But not sure when/if I or someone else will come around to actually do it ...

@saper
Copy link
Member Author

saper commented Jun 23, 2015

Oh noes... I actually want to use a pure C connector to libsass... please no C++ madness!

@mgreter
Copy link
Contributor

mgreter commented Jun 23, 2015

@saper not sure what you mean? I'm talking about C functions like sass_number_subtract, sass_string_concat, sass_value_compare, sass_value_lt ... So don't see where the problem is?

@saper
Copy link
Member Author

saper commented Jun 23, 2015

I understood wrongly - that you intend to introduce C++ operators. Personally I think that API is already way too large and too bloated. I'd rather have small JavaScript classes handling complex numbers/units or color class than rely on the API. The API should get the data in and out. Of course it does not help that the structures are opaque - I'd love to be able to see them. That would save us a LOT of API calls.

@xzyfer
Copy link
Contributor

xzyfer commented Jun 29, 2015

@saper aside from style cleanups this is ready for 3.3. We'll be starting betas for 3.3. shortly as it's a massive update.

mgreter added a commit that referenced this pull request Jun 30, 2015
Create quoted strings coming via API
@mgreter mgreter merged commit 321b874 into sass:master Jun 30, 2015
@drewwells
Copy link
Contributor

@mgreter 💯 to exposing operators, we could very much use that

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

Successfully merging this pull request may close these issues.

5 participants