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

Master: logical operator 'and' should have precedence over logical operator 'or' #2776

Closed
SomMeri opened this issue Jan 17, 2016 · 14 comments · Fixed by #2779
Closed

Master: logical operator 'and' should have precedence over logical operator 'or' #2776

SomMeri opened this issue Jan 17, 2016 · 14 comments · Fixed by #2779
Assignees
Labels

Comments

@SomMeri
Copy link
Member

SomMeri commented Jan 17, 2016

This is related to #2763 pull request.

The guards of this mixin:

.orderOfEvaluation(@a1, @a2, @a3) when ((@a1) and (@a2) or (@a3)) {
  no-parenthesis: evaluated true 1;
}

should be evaluated to true if called like this .orderOfEvaluation(false, false, true) e.g. the same way as logic in javascript works.

@SomMeri SomMeri added the bug label Jan 17, 2016
@SomMeri SomMeri self-assigned this Jan 17, 2016
@SomMeri
Copy link
Member Author

SomMeri commented Jan 17, 2016

This seem to be more about and having higher predence then or in common programming languages.

@matthew-dean
Copy link
Member

Makes sense.

@matthew-dean
Copy link
Member

Although it's not about left-to-right or right-to-left but order of operations.

@SomMeri SomMeri changed the title Master: logical expressions in guards are evaluated from left to right instead of from right to legt Master: logical operator 'and' should have precedence over logical operator 'or' Jan 17, 2016
@seven-phases-max
Copy link
Member

Yes, it's not about operator precedence (or and and should have the same) but just the order of operands/operations (evaluated left to right):

false and false or true -> (false and false) or true -> true
true or false and false -> (true or false) and false-> false

@matthew-dean
Copy link
Member

@seven-phases-max Neither order nor precedence should change the outcome. Both should evaluate to true. A logical or is a separator between conditions, while and joins conditions. Thus when you have a sequence of or and and, then it helps to visualize all conditions joined by and as being within the same parentheses group.

@seven-phases-max I couldn't tell if you were posting what the problem is or what the outcome should be, but if someone comes across this thread, the correct evaluation would be:

false and false or true -> (false and false) or true -> true
true or false and false -> true or (false and false) -> true

It's not really that "and" is evaluated first so much as "and" continues to append conditions that must also evaluate to true, so all statements joined by "and" must be evaluated as a whole.

Example:
screen shot 2016-01-17 at 4 11 57 pm
Our guiding light is how "and" and "or" are treated in CSS. However, that logic is not entirely consistent within CSS. In media queries, this behavior is like the above. A comma (logical "or" in a media query) completely separates groups of conditions, AFAIK, same as JavaScript or other programming languages typically do.

BUT, in the @supports spec, the W3C says this:

to avoid confusion caused by precedence rules, the syntax does not allow ‘and’, ‘or’, and ‘not’ operators to be mixed without a layer of parentheses.

For example, the following rule is not valid:

@supports (transition-property: color) or
          (animation-name: foo) and
          (transform: rotate(10deg)) {
  // ...
}

Instead, authors must write one of the following:

@supports ((transition-property: color) or
           (animation-name: foo)) and
          (transform: rotate(10deg)) {
  // ...
}
@supports (transition-property: color) or
          ((animation-name: foo) and
           (transform: rotate(10deg))) {
  // ...
}

That said, the original guards feature was designed before @supports was on the scene, and the syntax was intended to mimic media queries. If we wanted, though, we could force parentheses the way that @supports does for those who might be confused about precedence rules.

I also notice that the required syntax for not differs a bit between @supports and @media as well, requiring that statements in the former for not be joined in parentheses, again for clarity. As in:

@supports not ([expr1]) and ([expr2])  { }   // not valid
@supports not ( ([expr1]) and ([expr2]) )  { }   // valid
@supports not ( not ( ([expr1]) and ([expr2]) ) )  { }   // also valid

I wonder if it might be in Less's best interest to adopt this specific rule: "to avoid confusion caused by precedence rules, the syntax does not allow ‘and’, ‘or’, and ‘not’ operators to be mixed without a layer of parentheses."

@seven-phases-max
Copy link
Member

false and false or true -> (false and false) or true -> true
true or false and false -> true or (false and false) -> true

Yes, this is correct (that was my mistake, sorry, looks like I've made some mistake when checking it in a JS console).

Our guiding light is how "and" and "or" are treated in CSS.

We should be very careful with this (down to not even using any CSS stuff where possible), since, as you already referenced above, in CSS they are too far from "normal" C-like behavior. Counting that Less also already adopted many C-like stuff there (width > 100px instead of CSS's min-width: 100px and so on), it indeed could become a real nightmare to try to fit both models into the same box.
On one hand Less has to support @support (and similar stuff) in the way CSS does, but in the same time things like:

the syntax does not allow ‘and’, ‘or’, and ‘not’ operators to be mixed without a layer of parentheses

may simply spoil the very idea of Less being a "more usable" version of CSS. (Well, in this particular case, counting that Less guards can be assumed to be much more often used stuff than CSS conditionals).

Hmm, now after I wrote this - I realize I have no straight-forward idea how to handle this at all.
For instance even this

That said, the original guards feature was designed before @supports was on the scene, and the syntax was intended to mimic media queries.

is not quite true, since Less also never evaluated even not statements (in guards) the way CSS @media did.

@matthew-dean
Copy link
Member

is not quite true, since Less also never evaluated even not statements (in guards) the way CSS @media did.

Remember on a separate thread when we were talking about what the "docs say", and how that's not exactly gospel, especially for some features that were not completely implemented? (Can't remember what feature we were discussing it on.)

This is the same case, except in this case the existing code is not gospel. The fact that Less never evaluated not statements that way does not mean that behavior was ever intentional. ;-) I can tell you that the guard syntax was definitely based on media query syntax, and that the difference in not behavior was likely an oversight.

@seven-phases-max
Copy link
Member

Remember on a separate thread when we were talking about what the "docs say", and how that's not exactly gospel, especially for some features that were not completely implemented?

Yes, but counting the age of the feature itself (in that case it was "young" DR).
But in this case, while it's probably OK in general to break all already written when not (false), (true) code, the actual choice sits in a slightly different room:
The root trick is that Less does not really evaluate any @media or @support (so all kudos for their weirdness go to CSS WG anyway), but do evaluate its guards. So the real choice is between:

  1. Less strictly follows (not even widely adopted yet and in general designed to be used as rare as possible) CSS conditionals for its guards and then is continuously spammed with "why it's so dumb and unexpected?" feature-requests/issues...
  2. Less adopts well-known C-like behaviour (maybe with certain restrictions to not spoil CSS too much) and then it's just a matter of mentioning the guards "do not follow @supports because it's ..." in the docs (yet again counting that Less does not follow other CSS boolean "ops" like > anyway)

:)
(Note I'm not in favour of either choice but just pointing to their consequences).

@matthew-dean
Copy link
Member

I was thinking about this more through dinner. I think we should actually deprecate "not". There's no reason for it. We can evaluate conditions with "=" and "!=" now.

So I wasn't necessarily advocating for a behavior either, other than making things consistent and clear.

In truth, CSS's "min-width" for media statements was always clunky, especially since it refers to viewport width and not the min-width property. And it didn't handle "<" vs "<=" very well.

So yeah, you do make a good point that Less is mimicking CSS but also aiming to improve it or simplify it. In this case, we can probably improve the syntax a bit. It was good to replace the comma with "or" and we can simplify negation as well.

Since "not" is so tricky in CSS, we're probably better off without it and using things like @var != true. That's clearer.

@SomMeri
Copy link
Member Author

SomMeri commented Jan 18, 2016

I have looked at operator precedence in four languages (javascript, java, python, C++) and all four evaluate not before and and and before or . It seems to be common way of solving logical expressions. With apologies to @supports " does not allow operators to be mixed without a layer of parentheses" is odd and lame :). I do not remember this causing confusion when programming. Also, the people who learned all those random css rules and properties and hacks are not demographic that is going to be confused by precedence or logical operators.

I do not think we should remove not for similar reasons. Someone might want to do not(isColor(@var)). But mostly, it is unnecessary limiting. In normal languages I like to use not for clarity e.g. writing !(a && b) because that says exactly what I wanted to say (not true at the same time) where !a || !b is logically the same but does not read into the same sentence. Similarly, if you have two mixins and want to make them mutually exclusive, putting not in front of guard but otherwise keeping exactly the same makes intention clearer. The precedence of not is also better known, no one is confused by !condition && something in javascript.

Right now, things not refers to have to be closed in parenthesis through, that is mostly because that is how it always was.

In any case, these things are easy to change in code and were not released yet.

@seven-phases-max
Copy link
Member

Btw., I've just re-read the latest spec. and found quite curious things worth to note:

  • whitespace is required after a ‘not’ and on both sides of an ‘and’ or ‘or’... (<- changes since prev. spec. version)
  • The @supports rule is at risk; if interoperable implementations are not found, it may be removed to advance the other features in this specification to Proposed Recommendation.

The first one is probably something we can safely ignore since the only reason for it seems to be unambiguous parsing (discussion), i.e. the issue we solve in Less by other means anyway.

The second one is interesting because currently only @supports is declared to have all those or and parens, while @media remains with the old syntax and the only thing the spec. extends @media with is the nesting. Thus if something goes wrong we are doomed at some ghost syntax/semantics (most likely then they will try to use the same new syntax for @media or so, but again the spec. is postponed for another better times).


Speaking of the WG requirement to use pares everywhere itself, it seems to be an invention made to improve browser ability to recover from and/or support conditional chains involving unknown/future properties, and not something done with usability or something like that in mind at all (I can't find the main discussion about that yet, but I guess this thread illustrates what it was about in general).

@seven-phases-max
Copy link
Member

Right now, things not refers to have to be closed in parenthesis through, that is mostly because that is how it always was.

Ah, yes, in a long run, the current Less requirement for parens to be around "other" expressions/values and in the same time to be optional around not or , expressions is a bit weird
(e.g. when not(a) and (b) vs. when (iscolor(a)) - for sure it also comes from @media-like syntax) But indeed it's probably not something important yet (especially considering not so obvious syntax for the other way around e.g. if (a) vs. if (!a) vs. if !(a) vs. if !a?)

@matthew-dean
Copy link
Member

But mostly, it is unnecessary limiting

@SomMeri All good points for not dropping not.

Usually my concerns are around language consistency and simplicity. It's conceivable that using not (@a) is simpler for some Less authors than (@a != true) (I wouldn't propose !@a as valid.) For me, the second can more easily be read aloud as "@A is not true", whereas the first simply reads "when not @A".

As far as usage with the is functions, usage of not is somewhat necessary, since they're positive assertions. This is far beyond the scope of this issue, but this is actually probably the most readable.

.mixin(@a) when(@a is Color) {}
.mixin(@a) when(@a is not Color) {}

Any CSS beginner would be able to understand those statements.

Okay, so we have "and" and "or" figured out, and looks like "not" is staying. Any other language change proposals related to this should probably go into less-meta 3.0 discussion, agree?

@SomMeri
Copy link
Member Author

SomMeri commented Jan 19, 2016

@matthew-dean 👍

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

Successfully merging a pull request may close this issue.

4 participants
@matthew-dean @SomMeri @seven-phases-max and others