Skip to content

add support for "!=" and "==" operators on colors#79

Closed
SBD580 wants to merge 3 commits intoleafo:masterfrom
SBD580:master
Closed

add support for "!=" and "==" operators on colors#79
SBD580 wants to merge 3 commits intoleafo:masterfrom
SBD580:master

Conversation

@SBD580
Copy link

@SBD580 SBD580 commented Apr 2, 2013

No description provided.

first check for 'op_${opName}' and then for 'op_${ltype}_${rtype}',
as we might want to "{color} != {color}". 

This is not an optimal solution, as a way to override a general operator
by it's arguments type is something good and not viable this way.
@robocoder
Copy link
Collaborator

Can you elaborate? I don't understand what this fixes. The != operator seems to work for me.

@SBD580
Copy link
Author

SBD580 commented Apr 5, 2013

try to compile something like this:

@if #ffffff != #000000 {
backgroun-color: #fff;
}

I'm getting an Exception:
Uncaught exception 'Exception' with message 'color: unknow op != ...

SBD580 added 2 commits April 5, 2013 14:36
The previous fix break the tests and was not good. This is a new one
which just implement the op_neq_color_color method so the "!=" operator will work on colors as well
Implement the op_eq_color_color in order to support the equality between two colors.
In addition, fix small issue on "!=" operator when dealing with alpha value.
@leafo leafo closed this in be2364e Apr 5, 2013
@leafo
Copy link
Owner

leafo commented Apr 5, 2013

I pushed a simpler fix, does that look okay to you?

@SBD580
Copy link
Author

SBD580 commented Apr 5, 2013

Oh, I actually didn't know it is possible to compare arrays directly with the '==' and '!=' operators - COOL. But what about rgb(0,0,0,1) and rgb(0,0,0) - they will not be equal that way.

In addition, I would implement this in separate methods (op_neq_color_color and op_eq_color_color) as this method usually return Color and not a Boolean and multiple return types are not a good practice I think.
Also, in case you would like to add another equality operator ('<>', '=' etc) you would not need to change the code in several places. Just a thought.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants