-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Comments
I'm not sure I see the issue? |
|
No, you're just staring to invent artificial definitions. All of this are just
Why exactly |
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
Conversely, |
We've been there already. Strictly speaking it's not even Less who wants to not follow |
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.
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. |
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:
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. |
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. :)
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. |
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. |
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 My mistake I guess was just too minimalistic initial post (assuming a reader has prev. discussions at the mentioned issues in mind). |
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.
or
The first is more traditional but totally breaking (you'll have to rewrite all of your |
@seven-phases-max It depends on what "whatever" is. If whatever is, literally, whatever, then yes, parentheses are required. But not for conditionals
|
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:
or
Where The second version (i.e. "outer parens required" - i.e. like C/JS-like
|
"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:
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. |
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 What if we: a) Deprecate the comma immediately in when guards (and update docs). @rjgotten Thoughts also on this? |
Yes, it's not required. But it is what a consistency, a clean implementation and most important clean documentation are about.
It's actually totally the opposite.
And this is the only argument against this feature request. |
@seven-phases-max It looks like you commented right after my comment. What do you think about the above proposal? |
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. |
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. |
Not closing as other features (e.g. |
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 Infact, even the CSS working group has ditched the Looking at it like that, and especially in light of custom functions and functions like And if you're going to move away from the quaint It's not like For all intents and purposes having outer parens around logical expressions such as used with |
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, What's also great about the MQ4 draft is that it makes the rules around how, why, when
From what I can see from the proposal, single boolean "tests" with MQ4 can be outside of parens, such as .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:
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 Regardless, the simpler solution is just to make expressions non-ambiguous, and the rules are not complex or cumbersome. If you have |
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. |
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
Their proposal is overbloated with things like: "... it must instead be written as
Yes, and that's the problem - most of this code is about handling different parens, operator precedence is the least part of it :) |
And simultaneously I agree that going overboard with required parens when operator precedence ambiguity is not at play is ... well; kind of stupid. ^_^ |
I probably wasn't clear that I wasn't suggesting we model that spec exactly. I implicitly said we didn't have to with 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 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 |
When parser faces either of |
Yes, agreed. Once the inner-paren statement is evaluated it can be evaluated as part of the containing expression. |
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. |
This is just a follow up of discussions at #2776 (comment) and #2798 (comment).
A.k.a.:
In summary, currently (at
2.6.0
) we have these two directly opposite requirements:and in the same time
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.
The text was updated successfully, but these errors were encountered: