Add list is_bracketed support to the C API

[xzyfer]

Lists gained an is_bracketed attribute in #2284. The primary
semantic difference is that theis_bracketed changes list equality
(#2281) which can matter in custom functions.

This is a breaking change so I'd like to get it into the first 3.5
beta.

/cc
@nex3 to sanity check this actually matters for custom functions
@mgreter because you most about the C API
@chriseppstein because of the implication for eyeglass

[mgreter]

Technically this looks ok, but I have two points:

1. I'm not too excited about the term "delimiter" here. Maybe that is just me, but I only use "delimiter" for stuff the goes between things, not around? I know that a lot of list/array implementation call their separator delimiter. So having both delimiter and separator for lists seems confusing too me. I would rather have called it Sass_List_Type ...

2. There would be a way to make this a "non" breaking change. I guess bracketed lists will be a seldomly used feature? So is it really needed in the constructor? This way we would not need to alter the constructor for lists. Implementors could then use "sass_list_set_type/delimiter" to change it when needed. IMO there are a few other flags that are not exposed via the constructor directly. Alternatively there could also be an explicit constructor for bracketed lists. I.e. sass_make_array; calling it array vs list would sound ok for me; but would need some more thought in regard of the "sass_is_list" tests etc.

Edit: I see that "delimiter" can be used in both ways, but I still think it is confusing when we already have a separator and naming it type seems at least a bit more obvious to me.

[xzyfer]

I've thought about bracketed lists a bit more. I'm going to drop to delimiter all together and just tagged the list as bracketed or not. In which case maybe having a different make function for bracketed lists make sense. It'll still return a Sass_List just with the bracketed bool set. I have Wip passing tests for dropping the delimiter I'll push this week.

…

On 3 Jan. 2017 2:21 am, "Marcel Greter" ***@***.***> wrote: ***@***.**** commented on this pull request. Technically this looks ok, but I have two points: 1. I'm not too excited about the term "delimiter" here. Maybe that is just me, but I only only use "delimiter" for stuff the goes between things, not around? I know that a lot of lists call their separator delimiter. So having both delimiter and separator for lists seems confusing too me. I would rather have called it Sass_List_Type ... 2. There would be a way to make this a "non" breaking change. I guess bracketed lists will be a seldomly used feature? So is it really needed in the constructor? This way we would not need to alter the constructor for lists. Implementors could then use "sass_list_set_type/delimiter" to change it when needed. IMO there are a few other flags that are not exposed via the constructor directly. Alternatively there could also be an explicit constructor for bracketed lists. I.e. sass_make_array; calling it array vs list would sound ok for me; but would need some more thought in regard of the "sass_is_list" tests etc. — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#2282 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWEtHvqcobZKGbf5yfhjV99r3i7pfks5rORYVgaJpZM4LYzuH> .

[xzyfer]

I'm not too excited about the term "delimiter" here

This has been addressed in #2284. The delimiter attribute was dropped in favour of the more semantic is_bracketed property which is what the delimiter attribute was being used to infer.

There would be a way to make this a "non" breaking change.

I'm fine with making this a breaking a change as long it's document the change accordingly, which they will be. API breakages were expected in 3.5 and 4.0.

I guess bracketed lists will be a seldomly used feature?

I disagree. Given the sheer amount of grid "frameworks" that exist, and the number of new ones that came out for flexbox, I expect this feature to get a decent workout. We shouldn't get in the way of experimentation.

IMO there are a few other flags that are not exposed via the constructor directly.

Separator, and bracket are core semantics of a list. The same isn't true for most other list attributes which are largely implementation details of LibSass.

Alternatively there could also be an explicit constructor for bracketed lists. I.e. sass_make_array;

I don't think arrays are right here. These are lists. They share all the same semantics as lists.

[mgreter]

I'm fine with making this a breaking a change as long it's document the change accordingly, which they will be. API breakages were expected in 3.5 and 4.0.

Then you should also increase ABI version number as AFAIK this is the first time we break ABI.

I guess you'll add the documentation also to this PR?

[xzyfer]

Update the documentation and bumped the ABI version to 1:0:0

[mgreter]

You forgot to increment for windows:
https://github.com/sass/libsass/blob/master/res/resource.rc

[xzyfer]

Thanks I'll update it. I'm not able to find any documentation on the semantics of bumping those version numbers. Sounds like everyone just does their own thing. Do you know of any good recommendations? /cc @am11 Regards, Michael

…

On Fri, Jan 6, 2017 at 12:31 AM, Marcel Greter ***@***.***> wrote: You forgot to increment for windows: https://github.com/sass/libsass/blob/master/res/resource.rc — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#2282 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjZWAMXBI5q-_mBgvv21lz3M-0znK-jks5rPPCygaJpZM4LYzuH> .

[am11]

@xzyfer responded with a PR #2288 😎