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

array operators don't match spec #632

Closed
ansis opened this issue Aug 1, 2014 · 7 comments
Closed

array operators don't match spec #632

ansis opened this issue Aug 1, 2014 · 7 comments
Labels
Milestone

Comments

@ansis
Copy link
Contributor

ansis commented Aug 1, 2014

The implementation uses ||, &&, !, $or, $and, $nor while the spec uses |, &, ^, !

var arrayOperators = {
'||': or, '$or': or,
'&&': and, '$and': and,
'!': nor, '$nor': nor
};

https://github.com/mapbox/mapbox-gl-style-spec/blob/d1fc14b44e68dc30fc9990bc1329107265cdd8f8/reference/v4.json#L426-L438

Which do we want?

@mourner @nickidlugash @edenh

@ansis ansis added this to the v0.1.0 milestone Aug 1, 2014
@ansis ansis added the bug label Aug 1, 2014
@kkaefer
Copy link
Contributor

kkaefer commented Aug 4, 2014

@mourner
Copy link
Member

mourner commented Aug 4, 2014

Yeah, but we need to choose one. I also remember that we settled on ||/&& notation in one of the older filter discussion...

@jfirebaugh
Copy link
Contributor

+1 for || & &&

@mourner
Copy link
Member

mourner commented Aug 4, 2014

Should we also keep the aliases ($and etc.)? They can make the style more readable.

@mourner
Copy link
Member

mourner commented Aug 6, 2014

It also looks like there's a difference to how operators apply to object vs array. In native, you can use both with any operator, the only difference being that object is AND by default and array is OR, which gets overriden by parent. In JS, you can only use and, or and nor with an array, and only use not with an object.

Seems like the native way is more convenient, so we can adopt it in JS, while renaming operator names (and adding aliases) in the spec.

@mourner
Copy link
Member

mourner commented Aug 6, 2014

On the other hand, there's no easy way to do NOT in native: yo have to use NOR in combination with AND which is pretty bulky.

@mourner mourner modified the milestones: v0.2.0, post-v0.2.0 Aug 6, 2014
@mourner mourner added the style label Aug 6, 2014
@mourner mourner modified the milestones: v0.2.*, v0.3 Aug 11, 2014
@jfirebaugh
Copy link
Contributor

Fixed with v6 filters.

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

No branches or pull requests

4 participants