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

Consistent guard expressions. #2807

Closed
seven-phases-max opened this issue Feb 7, 2016 · 29 comments
Closed

Consistent guard expressions. #2807

seven-phases-max opened this issue Feb 7, 2016 · 29 comments

Comments

@seven-phases-max
Copy link
Member

This is just a follow up of discussions at #2776 (comment) and #2798 (comment).
A.k.a.:

"Less needs to finally stop pretending that guards are similar to CSS @media, they are not".

In summary, currently (at 2.6.0) we have these two directly opposite requirements:

when (true) and (true) // valid
when (true) = (true) // not valid

and in the same time

when (true = true) // valid
when (true and true) // not valid

It's not that this is impossible to code, but it does not look practical, taking into account that all this expressions may nest each other and must follow certain operator precedence.

@matthew-dean
Copy link
Member

I'm not sure I see the issue?

@matthew-dean
Copy link
Member

when (true) and (true)    // valid - expressions are "true" and "true" -  "and" is a joining condition
when (true) = (true)      // not valid - you can't test the equality of expressions
when (true = true)     // valid  - "true = true" is an expression
when (true and true)   // not valid - you can't join two expressions without putting the expressions in parentheses

@seven-phases-max
Copy link
Member Author

"and" is a joining condition

No, you're just staring to invent artificial definitions. All of this are just left op right binary expressions:
a + b
a * b
a = b
a > b
a or b
and so on.
Some of them are logical (thus returning true/false) others are more generic (returning number/color or whatever) but still these are just binary expressions of the same category. Thus having different parenthesis requirements for some of them is the language defect.


when (true) = (true) // not valid - you can't test the equality of expressions

Why exactly ((@a > @b) = (@x and @y)) is valid in all conventional languages (assuming = is ==) but in Less it's not? ;)

@rjgotten
Copy link
Contributor

rjgotten commented Feb 8, 2016

@seven-phases-max

Your problem is that you're seeing this as a simple nesting of binary expressions. It's not.

Mixin guard expressions follow the guard syntax of CSS media directives first and foremost. These specifically require parentheses around conditions and join conditions together using an and operator. The contents of the conditions are, for mixins atleast, specified as binary expressions that evaluate to a type-weak boolean, i.e. , a truthy/falsy value.

when (@a > @b) = (@x and @y) is not a valid mixin guard expression because the = binary comparison operator is being used at the level of the CSS media guard syntax and not of the normal boolean expression syntax.

Conversely, when ((@a > @b) = (@x and @y)) is valid because it transfers from a CSS media guard to a normal boolean expression when the outer set of parentheses is opened.

@seven-phases-max
Copy link
Member Author

@rjgotten

Mixin guard expressions follow the guard syntax of CSS media directives first and foremost.

We've been there already.
And that's why this feature request is here in first place, if we want a consistency in guards we must forget about any @media stuff - because it's:

odd and lame

Strictly speaking it's not even Less who wants to not follow @media-like conditions, but also CSS itself:
(orientation: portret) -> orientation == portret
(min-width: 100px) -> width > 100px
and so on, but
[attr="foo"] // uh?
Those @media-conditions turn out to be nothing but a set of syntactic and semantic kludges sitting on top of each other invented only to keep a browser CSS parser as simple as possible. It does make perfect sense given the very limited applications of the @media and @support, but can't make any sense for guards (unless the goal is to produce the most confusing expression apparatus ever ;), especially yet again counting that Less guards already have very few to do with @media (starting right at the basic not (false), (true) expression).

@matthew-dean
Copy link
Member

No, you're just staring to invent artificial definitions

That's what I'm best at! ;-)

Let's skip to the important part at the end. TL;DR You're essentially right, that parentheses should just provide "grouping" and evaluation order, and shouldn't change the types of operations allowed.

Detailed explanation:


I wasn't being arbitrary about the expressions. There are three types of operators being talked about here: arithmetic; equality and relational; and conditional. That's what I mean by "joining condition", that in your examples you had different types of operators that have different rules, so you can't directly compare them.

Do many programming languages allow you to use all operators in similar ways? Yes. Do all? No. One of the tricky things for Less, of course, is that arithmetic operators are an invention of Less on top of CSS, whereas conditional operators are not.

That doesn't mean we shouldn't treat them all that way. I was responding to the comment (I guess implied comment) that current Less operator rules are self-contradictory, which they aren't. So my only goal was explanation, not an endorsement of the behavior.

So, to be clear, I'm not against this: Well.... I kinda am. This gets too complicated on the parsing side. Arithmetic and comparison needs to stay in parentheses.

when (less) = (less)  // same as:
when (less = less)

Less should essentially collapse all arithmetic operations, then equality/relational, then conditional, then the "not" keyword (to negate the entire evaluation), and that order could happen several times depending on the nesting of parentheses.

@matthew-dean
Copy link
Member

For some reason I didn't see the previous 2 comments when I posted mine.

The thing is, @rjgotten and @seven-phases-max, you're both right, and I'm not just saying that to be diplomatic. That is:

  1. Guards follow(ed) the syntax of media queries. And as such, and and or (alternatively comma) have a historically different syntactical role than arithmetic / equality.
  2. Media query syntax is actually pretty shitty. (min-width to test the width property? Really?)

From the hand-off of Less to a larger team mid 1.x and throughout Less 2.0, I think we were extremely cautious about syntax / changes. And that was actually very smart to maintain consistency. Moving forward into 3.x+, I've been suggesting we kill our darlings; that is, don't treat historical behavior / syntax of either CSS or Less as the best way to do it.

We can still maintain the goals of simplicity, intuitiveness, and a concise feature set of the language without preserving all the things that we may have a different perspective on now.

So, at this point, I would agree with @seven-phases-max just because it's reasonable that a developer / designer new to Less would be confused by current behavior. Being more "faithful" to CSS over intuitiveness is not the best trade-off.

@rjgotten
Copy link
Contributor

rjgotten commented Feb 8, 2016

@seven-phases-max

Those @media-conditions turn out to be nothing but a set of syntactic and semantic kludges sitting on top of each other invented only to keep a browser CSS parser as simple as possible. It does make perfect sense given the very limited applications of the @media and @support, but can't make any sense for guards (unless the goal is to produce the most confusing expression apparatus ever ;), especially yet again counting that Less guards already have very few to do with @media (starting right at the basic not (false), (true) expression).

@matthew-dean

Media query syntax is actually pretty shitty.

Don't get me wrong. I whole-heartedly agree with those statements. I was simply not aware you were ok with breaking back-compat in such a way, so I was trying to explain why current versions of Less have guards the way they do. :)

So, at this point, I would agree with @seven-phases-max just because it's reasonable that a developer / designer new to Less would be confused by current behavior. Being more "faithful" to CSS over intuitiveness is not the best trade-off.

I would as well. The mix-and-match between CSS-ish media guards and more classic boolean binary expressions is a bad thing that negatively impacts readability.

@matthew-dean
Copy link
Member

I was simply not aware you were ok with breaking back-compat in such a way, so I was trying to explain why current versions of Less have guards the way they do

Right, looks like I was doing the same thing. As far as breaking changes, we've already proposed some breaking changes for 3.0 in other threads. So, while I'm not suggesting we break everything, regardless, I would like 3.0 to be an evolutionary step forward for Less.

@seven-phases-max
Copy link
Member Author

To be honest I did not even consider this as important (why bother if it works as-is?), until I actually dug into #2798 and found that most likely the only way to fix it is to throw yet another hundred of ifs in :)
And all that code just for relatively minor improvement (and the overall thing still remains quite limited and becomes even more little-bit awkward) w/o even being sure that it won't fail in some not-predictable edge-case, because most of those ifs are about detecting different parens for outer/inner and media-like/non-media-like expressions w/o any straight-forward grammar rules.
Hence the ticket (i.e. current behaviour starts to harm).

My mistake I guess was just too minimalistic initial post (assuming a reader has prev. discussions at the mentioned issues in mind).

@seven-phases-max
Copy link
Member Author

Technically the only thing to decide (beside just "yes/no" for the ticket as such) is if we want to require outer parens or not.
I.e. should it be:

when (whatever)

or

when whatever

The first is more traditional but totally breaking (you'll have to rewrite all of your when (@a), (@b) and (@c) stuff). The second is partially backward-compatible (not counting any operator precedence), but someone might not really like when not @a-ilke things.
(Additionally the first assumes the comma logical operator is simply removed, while the second needs a decision on keeping it or not - counting this side-effect)

@matthew-dean
Copy link
Member

@seven-phases-max It depends on what "whatever" is. If whatever is, literally, whatever, then yes, parentheses are required. But not for conditionals

when (whatever) // pass
when whatever // fail
when not whatever // fail
when not (whatever) // pass
when not (whatever) and (whatever) // pass - evaluated as when not ((whatever) and (whatever))
when (whatever), (whatever) // pass
when (whatever), whatever // fail
when (whatever), (whatever) // pass
when (whatever, whatever)  // technically, this would fail as comma is outside parens... but maybe is better that it doesn't
when (whatever or whatever)  // pass, and probably the reason the above should pass
when not (not whatever and (whatever))   // pass

@seven-phases-max
Copy link
Member Author

There's no such thing as "conditionals" (like you mean it above). They are all expressions. There should be no exceptions of any kind. Period. So it is either:

when whatever // all parens always optional

or

when (whatever) // parens after when are always required

Where whatever is an expression.

The second version (i.e. "outer parens required" - i.e. like C/JS-like if for example) implies:

when (whatever) // pass
when whatever // fail
when not whatever // fail
when not (whatever) // fail
when not (whatever) and (whatever) // fail
when (whatever), (whatever) // fail
when (whatever), whatever // pass, but the second whatever is a selector
when (whatever, whatever)  // fail (goodbye logical comma, we don't like you)
when (whatever or whatever)  // pass
when not (not whatever and (whatever)) // fail

@matthew-dean
Copy link
Member

"Conditional" as in "an expression between two entities joined by a conditional or logical operator". https://docs.oracle.com/javase/tutorial/java/nutsandbolts/opsummary.html

I recognize they are all expressions and that you want all expressions to behave exactly the same. I don't agree that there can be "no exception of any kind". That's really a preference. Seeing all expressions as syntactically identical is a programming convention more than necessity. It's not actually required that Less behaves like C++ or JS or Java.

Re-thinking it, the reason I would advocate to continue allowing conditional expressions without parentheses is:

  1. Historical precedence and CSS language similarity.
  2. The behavior is unambiguous to the parser.
  3. It's not a breaking change.

So I would go with my original statement at the top of the thread. What matters is consistency, and treating expressions with conditional operators slightly different is not inconsistent, because they are the only exception and those exceptions are very specific and documented. Depending on your programming background, it may seem more natural to see "an expression as an expression, period". But as I've said, not all programming languages treat conditional operators equally to other operators. (I'm trying to find a good example.)

As I said in the other thread, the comma with the "selector" should not be valid and should fail.

@matthew-dean
Copy link
Member

That said, if we want to deprecate current behavior and require parentheses in the future for all when expressions, and do stuff like lose the comma, I would be on-board with that, because potentially that would let us do things like actually allow guards on multiple selectors down the road.

So, @seven-phases-max I don't disagree with you that putting parentheses around the entire "root" expression after when would further simplify things. I think some of our argument here is around semantics and differentiating expressions. We're arguing theory more than design. So let's take a breath and regroup.

What if we:

a) Deprecate the comma immediately in when guards (and update docs).
b) Remove comma in 3.0.
c) As of 3.0, deprecate conditional expressions without parentheses (and update docs)
d) Remove deprecated when expressions in 4.x. (All expressions require parentheses)

@rjgotten Thoughts also on this?

@seven-phases-max
Copy link
Member Author

It's not actually required that Less behaves like C++ or JS or Java.

Yes, it's not required. But it is what a consistency, a clean implementation and most important clean documentation are about.
Keep to maintain exceptional behaviour ("here we behave like @media, here we behave like Fortran, here we behave opposite to @media, here we don't behave at all... etc.") and you're continuously trapped with things like #2798 every time you're trying to make even a minor improvement/addition.
That's what this feature request is about.

1.Historical precedence and CSS language similarity.

We've been there already.

The behavior is unambiguous to the parser.

It's actually totally the opposite.

It's not a breaking change.

And this is the only argument against this feature request.

@matthew-dean
Copy link
Member

@seven-phases-max It looks like you commented right after my comment. What do you think about the above proposal?

@matthew-dean
Copy link
Member

The behavior is unambiguous to the parser.

It's actually totally the opposite.

What I meant was: "It needn't be ambiguous to the parser." That is, current selector parsing would likely have to change. But if we deprecate anyway, then we don't need to add more support to something being deprecated.

@stale
Copy link

stale bot commented Nov 14, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@seven-phases-max
Copy link
Member Author

Not closing as other features (e.g. if function) now inherit this inconsistency too.

@rjgotten
Copy link
Contributor

rjgotten commented Nov 16, 2017

It's been a few more years working with Less and bumping up against the inane handling of complex mixin guards now. And a few more years of the oddity being thrown in my face of @media-like syntax being able to do logical AND in one clause with and keywords, but needing disjoint comma-separated clauses for logical OR.

Infact, even the CSS working group has ditched the @media conventions with @supports, which strictly uses an or keyword and doesn't even support comma-separation, afaik. It's like the language design in CSS has gone completely schizophrenic there.

Looking at it like that, and especially in light of custom functions and functions like if now also supporting logical expressions, Less as a language really; really REALLY needs uniform, unambiguous first-class support for logical expressions, separate from CSS constructs such as @media and @supports, which are just that: CSS constructs. Not binary logical expressions.

And if you're going to move away from the quaint @media conventions, you might as well move away from the requirement to wrap such logical expressions in parenthesis.

It's not like @mediaouter parens are used for logical grouping or operator precedence anyway.
If anything they should have been braces, since @media guard conditions are shaped as CSS rulesets that must pass a match test. But ofcourse, that would introduce an ambiguity with the rule's own body and probably would cause problems with the way CSS parsers must try to gracefully recover from mismatched closing braces and other such syntax errors, and as such: parens instead.

For all intents and purposes having outer parens around logical expressions such as used with when is just 'noise'. They make sense as a logical grouping construct to enforce operator precedence and should ideally be used only for that purpose. (The sole exception is probably if you'd need to disambiguate whether something is actually an expression that should be evaluated or not.)

@matthew-dean
Copy link
Member

matthew-dean commented Nov 17, 2017

Infact, even the CSS working group has ditched the @media conventions with @supports, which strictly uses an or keyword and doesn't even support comma-separation, afaik. It's like the language design in CSS has gone completely schizophrenic there.

Media Queries Level 4 is going this route too, so it's not schizophrenic so much as the direction moving forward. I think the working group realizes that in the effort to create backwards-compatibility with CSS parsers, they done fucked up making media queries make any sense. The current proposal has this:

@media (width <= 320px) { /* styles for viewports <= 320px */ }
@media (width > 320px) { /* styles for viewports > 320px */ }

Yeah yeah, I know this is a draft, but as you say, @supports is doing this, and I think there's broad support to making more sensible media queries. It never made sense to have min-width refer to the total width, instead of having anything to do with the min-width property.

What's also great about the MQ4 draft is that it makes the rules around how, why, when not() works very clear, much more so than the original proposal of media queries, and makes it easy to conform Less rules to match.

For all intents and purposes having outer parens around logical expressions such as used with when is just 'noise'. They make sense as a logical grouping construct to enforce operator precedence and should ideally be used only for that purpose. (The sole exception is probably if you'd need to disambiguate whether something is actually an expression that should be evaluated or not.)

From what I can see from the proposal, single boolean "tests" with MQ4 can be outside of parens, such as @media screen {}, but logical expressions are within @media (width > 320px) {}. I think it's a clear principle, and the same can apply to Less e.g.

.mixin() when @var1 or @var2 {}  //valid
.mixin() when @var2 or (@var3 > 320px) {} //valid
.mixin() when @var2 or @var3 > 320px {} // not valid

THAT SAID, we can easily apply "implicit parens" around the root expression. Circling back to the original issue, you shouldn't have to do this:

.mixin() when ((true) = (true)) {}

These should all be just fine:

.mixin() when ((true) = (true)) {}
.mixin() when (true) = (true) {}
.mixin() when true = true {}

However, this would be NOT fine:

.mixin() when true = true or false = true and false {}

That is, ambiguous expressions need parens. MQ4, for example, says this:

For example, (color) and (pointer) or (hover) is illegal, as it’s unclear what was meant.

If I'm not mistaken, Less had a PR a while back that introduced a large amount of parsing code that, in part I think, established order of operations to try to resolve ambiguity in when statements. Unless I was misreading what that code did when I went through it.

Regardless, the simpler solution is just to make expressions non-ambiguous, and the rules are not complex or cumbersome. If you have and and or in your expression, they can't be at the same "level". You have to specify precedence order of operations by parens grouping. That should be the rule, IMO.

@rjgotten
Copy link
Contributor

rjgotten commented Nov 17, 2017

If you have and and or in your expression, they can't be at the same "level". You have to specify precedence order of operations by parens grouping. That should be the rule, IMO.

Wholeheartedly agreed. Even in languages which do not require it, the implicit order introduced by left or right associativity leads to more bugs than one dares care for. People will always get that wrong at some point.

@seven-phases-max
Copy link
Member Author

seven-phases-max commented Nov 17, 2017

single boolean "tests" vs. logical expressions

Hmm, no, this still looks like they are doing their best to stay backward compatible with current media code (more specifically to stick to their initially defective not). "Boolean test" is an artificial invention - these all are "logical expressions" (simply because they all produce the same boolean result)
(To be strict they call theme "logical expressions" (not/and/or) vs. "media conditions" (anything else)).

These should all be just fine: ... true examples

Their proposal is overbloated with things like: "... it must instead be written as not ((color) or (hover))"
So no, if we go their way, even trivial not (true or true) remains impossible - must be ridiculous not ((true) or (true)) instead. (Doh! there's even no operator precedence ambiguity in this example... :)

PR a while back that introduced a large amount of parsing code that, in part I think, established order of operations to try to resolve ambiguity in when statements.

Yes, and that's the problem - most of this code is about handling different parens, operator precedence is the least part of it :)

@rjgotten
Copy link
Contributor

rjgotten commented Nov 17, 2017

(Doh! there's even no operator precedence ambiguity in this example... :)

And simultaneously I agree that going overboard with required parens when operator precedence ambiguity is not at play is ... well; kind of stupid. ^_^

@matthew-dean
Copy link
Member

So no, if we go their way, even trivial not (true or true) remains impossible - must be ridiculous not ((true) or (true)) instead. (Doh! there's even no operator precedence ambiguity in this example... :)

I probably wasn't clear that I wasn't suggesting we model that spec exactly. I implicitly said we didn't have to with when true = true being valid, per the suggestion. I probably didn't make that clear enough. I was more making the points that a) the MQ4 spec is better than the previous spec in terms of syntax and clarify, b) many of those same ideas and solutions can apply to when syntax (but don't have to).

But no, the spec isn't perfect and I think you're right that they're trying to not make the changes too drastic (and preserve backwards compatibility). Things like not (color or hover) are, on its face, not ambiguous at all, so no, there's no need to require parens for color and hover for stuff like that. I wouldn't suggest we have to do that.

I would say that for the most part, I think we're now all in relative agreement, or close enough to fix the most glaring errors. So, in my mind, this is ready to implement. (Side note: I renamed the ReadyForImplementation label to up-for-grabs because Github surfaces that and other specific labels to new users. I also changed labels to lowercase to match the other Less repos and to keep with Github convention.)

@seven-phases-max
Copy link
Member Author

seven-phases-max commented Nov 17, 2017

not (color or hover) are, on its face, not ambiguous at all, so no, there's no need to require parens for color and hover for stuff like that

When parser faces either of not/or/and it should either expect or not expect parens around their operands. That is, if a or b is valid it is valid anywhere, if it's not - it's not. Thus:
(true) or (true) automatically implies not((true) or (true)), (true) and (not((true) or (true))) and so on and vice-versa :(
(Unless we're going to double the amount of code of interest once more and handle one more "where am I exactly" flag in each and every function there...)

@matthew-dean
Copy link
Member

When parser faces either of not/or/and it should either expect or not expect parens around their operands. That is, if a or b is valid it is valid anywhere, if it's not - it's not. Thus:
(true) or (true) automatically implies not((true) or (true)), (true) and (not((true) or (true))) and so on and vice-versa :(
(Unless we're going to double the amount of code of interest once more and handle one more "where am I exactly" flag in each and every function there...)

Yes, agreed. Once the inner-paren statement is evaluated it can be evaluated as part of the containing expression.

@stale
Copy link

stale bot commented Mar 17, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 17, 2018
@stale stale bot closed this as completed Mar 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants