less / less.js

Less. The dynamic stylesheet language.
http://lesscss.org
Apache License 2.0
17.03k stars 3.4k forks source link

How to handle Maths #1880

Closed lukeapage closed 6 years ago

lukeapage commented 10 years ago
  1. We decided on strict maths going forward but there is general unease about forcing () around every calculation
  2. I don't think we want to change things massively or go back to the drawing board

See #1872

Possibility to add another case for calc which like font, with strict mode off, essentially turns strict mode on for a rule ? Too many exceptions in the future?

@seven-phases-max : Well, there're a lot of other possibilities, e.g. ./ or require parens for division (e.g. 1/2->1/2 but (1/2)->0.5) etc... Also, the "special cases" (e.g. properties where x/y can appear as shorthand) are not so rare (starting at padding/margin and ending with background/border-radius and eventually there can be more) so we just can't hardcode them all like it's done for font (and because of that I think that the current font "workaround" is just a temporary and quite dirty kludge that ideally should be removed too).

matthew-dean commented 6 years ago

I'd also add that I'm strongly against any "compatible units" guessworks (like let a/b remain to be a division and b/c to be not) in this particular context.

I think I was on board with that more as an olive branch to those that want naked divisors. But I think that your comment [here] is the most workable proposal. It was the original intention of "strict math", to eliminate ambiguity. I think the concern became operations within mixin and function calls leading to all sorts of parens within parens. But, in general, I'm with you that the efforts to make math easy in Less have also, ironically, made it very difficult, because of the increased ambiguity.

Also, I later realized that font: 10px/3 is valid shorthand. So there really is no algorithmic solution that can help there.

To get back to your question, @rjgotten...

Why not flip your reasoning around? If something looks like a math expression and it has compatible units then it will be treated as a math expression by default. And if you do not want that to happen ... well; then use the ~"..." escape hatch. It's exactly what it's there for, after all...

The idea/relationship of Less to CSS is similar to TypeScript to JavaScript. That is, rename your valid .css to .less and you can start adding Less features. Similar to how you can rename .js to .ts and start adding TypeScript features. If you don't add anything, you should get output the same valid Less / JavaScript, because the languages are supersets of the base language.

However, in the case of these ratios or other / divisors in CSS, Less already fails right out of the gate, by default. Your regular CSS arbitrarily becomes a Less math expression with a different result, even though you've changed nothing. That violates the contract of the language. Being able to change your .less into an entirely different Less expression in order to get back the original CSS you started with misses the point. That work should never be required. Less should not require you to change your valid CSS into compatible Less expressions in order to get back the valid CSS you had in the first place. That's just a broken model.

The same reasoning applies to calc(). Yes, you can string-escape your expressions, but you shouldn't have to. .css renamed to .less should produce the same effective CSS. This should be the base goal of the project, to not interfere / overwrite / over-interpret the original stylesheet. Anything else is trying to push the problem onto the developer for no other sin than using the parser/language in the first place.

thany commented 6 years ago

Yes, you can string-escape your expressions, but you shouldn't have to. .css renamed to .less should produce the same effective CSS. This should be the base goal of the project, to not interfere / overwrite / over-interpret the original stylesheet. Anything else is trying to push the problem onto the developer for no other sin than using the parser/language in the first place.

That.

LESS knows where the / is allowed and where it isn't in css. Where it isn't but appears anyway, it should assume maths to be done. Also where maths is surrounded by rounds brackets where rounds brackets aren't allowed in css should be interpreted by LESS.

In other words, LESS should try to interpret as little maths as possible to get to valid css. As soon as the output it valid, stop executing any more maths.

seven-phases-max commented 6 years ago

@thany

Too many assumptions of what the compiler knows (and some of them are simply wrong). Either way "the round brackets" mode is nearly what --sm=on does, so use that and forget about this thread (most likely you just not using any math in your projects beside calc (introduced a few years after Less is designed) so you can't see how the extra parens are annoying. But others do.)


For the rest see https://github.com/less/less.js/issues/1880#issuecomment-345194431.

thany commented 6 years ago

@seven-phases-max Don't forget that the / also has meaning in background and border-radius shorthands, and possibly others I'm not thinking of right now. LESS will happily treat those as a division. With or without strict math mode, LESS should "know when to stop" doing its own little maths.

rjgotten commented 6 years ago

@thany LESS should "know when to stop" doing its own little maths.

No, it shouldn't. There is no way to know where future CSS shorthands with the division slash are going to appear. Having the Less compiler 'know' about that is a fundamentally flawed approach and the very thing this discussion is attempting to find a solution to.

Sofar there are two, sane and predictable choices for / as a division operator:

(Also; please note that the Less name is no longer spelled in all caps.)

seven-phases-max commented 6 years ago

Don't forget that the / also has meaning in background and border-radius shorthands,

This is basically what this thread starts with. And see the summary of the proposed changes at https://github.com/less/less.js/issues/1880#issuecomment-345194431 (w/o any assumptions of what compiler should or should not know).

matthew-dean commented 6 years ago

No, it shouldn't. There is no way to know where future CSS shorthands with the division slash are going to appear. Having the Less compiler 'know' about that is a fundamentally flawed approach and the very thing this discussion is attempting to find a solution to.

I agree with this completely. @thany While you agreed with me in what you quoted of what I wrote, you're arriving at a different conclusion than I would make. Less shouldn't and can't know when to "stop doing maths". What I would say is that the breaking change should be that Less is more conservative about starting math (to use the American/Canadianism) in the first place.

@rjgotten

Never treat it as a division operator, unless it is within a known math context. Where known math context could/would be decided as in the current --strict-math=on behavior.

Just to clarify, this part (which I agree with):

Never treat it as a division operator, unless it is within a known math context.

Actually has 4 possible solutions, which I know you know, but just re-summarizing for the thread:

  1. Do all math only in parentheses. This has apparently been rejected by the commons, which is fine, although it's still available as an optional toggle (strictMath).
  2. Do all math everywhere, but division only in parentheses.
  3. Do all math everywhere, but division only in parentheses. Unless the division operator is printed as ./
  4. Do all math everywhere, but fix math so that 12px/4px does not result in division. (Multiplication and division only with unit-less values.) In other words, division everywhere, but with much more conservative rules. In the cases where it doesn't solve things, resort back to escaping. So, not a complete solution, but still an (arguable) improvement over the current situation.

From a usability perspective, I like making math "smarter" as in #4. From an engineering and maintenance standpoint, and to protect against future CSS changes, I like #3 as the most robust solution.

However, I think in reality, we would need to do both #3 AND #4 in order to fix calc(). Right now, Less ignoring all units when doing math is a real mess. 100vh - 12px should never be touched by Less (parentheses or not). But IMO neither should 12px/4px (parentheses or not), but I might be in the minority on that one.

So, I don't see this so much as a "math conflicting with CSS syntax" problem so much as Less being way overly aggressive with prematurely solving math equations to begin with.

seven-phases-max commented 6 years ago

Multiplication and division only with unit-less values.

it's not going to do the trick since there're things like:

font: small-caps bold 24px/3 ...;
lost-column: 1/3;
// etc.

and they are not divs.

rjgotten commented 6 years ago

However, I think in reality, we would need to do both #3 AND #4 in order to fix calc()

calc() is a pain. Ironically it is probably best solved by implementing it as an actual Less function, that would ideally take its parsed expression tree and attempt to simplify the expression. That is: it should pre-compute and combine compatible components such as 4px + 12px (or 4px + @a when @a is known to have a pixel value) but leave incompatible components, e.g. those with incompatible units, alone.

E.g.

@a : 4px;
@b : 2;
width : calc(100%/@b - 10px + @a);

should eventually render

width : calc(50% - 6px);
seven-phases-max commented 6 years ago

(repeating myself from https://github.com/less/less.js/issues/1880#issuecomment-345345735) And I do not see any benefit of vastly overenginneering the complier in order to optimize the expressions inside calc. If you're writing calc then you're fine with the browser to do the job whatever long expression you have there. So as already mentioned above I'm for "don't touch anything inside calc" (= don't try to be smarter than it's really necessary) and leave it for browser (or for a css-minifier since, if I recall correctly, some of them already do pretty good job on optimizing calc subexpressions).


The "smart unit behaviour mamabo-jambo" reminds me of the min/max monsters (bloating stack of non-maintainable and never ever used code) - how much I regret I did not scream against the "units mambo" back then, oh (So I'll keep whining here until the very idea of "different operator semantics depending on the operand units" is totally annihilated :P).


P.S. As soon as calc becomes a function receiving a non-arithm-evaluated expression (it will have to anyway) one will be able to write a plugin and override it with whatever optimization desired.

thany commented 6 years ago

@rjgotten Having the Less compiler 'know' about that is a fundamentally flawed approach

I think it's a fundamentally correct approach. LESS is a compiler to CSS, so it makes all the sense in the world that LESS knows about what it is compiling to.

@matthew-dean Less shouldn't and can't know when to "stop doing maths". What I would say is that the breaking change should be that Less is more conservative about starting math (to use the American/Canadianism) in the first place.

Interesting you think of it from the other side, so to speak. Here's what I'm proposing:

background: url(...) no-repeat 50% 50% / 40px + 10px 40px;

What LESS should do here, is obvious to me:

background: url(...) no-repeat 50% 50% / (40px + 10px) 40px;

Resulting in:

background: url(...) no-repeat 50% 50% / 50px 40px;

It shouldn't calculate the 50% / 50px part in this, because (1) it shouldn't be able to because of incompatible units and (2) because this is already far enough for a background value. So this is where it'll "stop doing maths".

That's what I meant by LESS "knowing when to stop".

Were it a different property like this:

padding-left: 50% / 10px + 5px;

It should break with an error (incompatible units). One possible output would be 50% / 15px which is invalid for this property. Another outcome could be 5% which it'll currently do, which is just wrong in every direction. And:

padding-left: 50px / 10px + 5px;

Should result in:

padding-left: 10px;

As expected. So in this case, the / is invalid for padding-left and is is taken into LESS and do its maths thingy.

thany commented 6 years ago

@matthew-dean Actually has 4 possible solutions, which I know you know, but just re-summarizing for the thread:

One more: 5) Use the \ operator for divisions in LESS, and deprecate using /. MATLAB has something like this, and certain flavours of BASIC used it to force integer division. So using the backslash is not completely unheard of.

/edit We can also lobby for including a ÷ key on keybaords and use that as the division operator. That's the one I learned in elementary school :)

seven-phases-max commented 6 years ago

What you suggest was already discussed many times before (do not hesitate to look at the threads referenced here). So here're just tiny-lazy comments:

Use the \ operator for divisions in LESS, and deprecate using /.

https://github.com/less/less.js/issues/1872#issuecomment-35245890

enough for a background... is invalid for padding-left

Meet the "Guys, my browser/polyfill/whatever just has added/updated/expanded support for a fnord property, will you please release a new Less version for me?" issue. Meet the font issue. etc. etc. Well, @rjgotten already commented above on why that kind of "knowledge" is the path to nowhere.

thany commented 6 years ago

@seven-phases-max Backslash was only a suggestion. You might as well use the ? for division. It doesn't really matter. What I'm suggesting, I guess, is to not use one character (/) that has very different and ambiguous meanings.

Meet the "Guys, my browser/polyfill/whatever just has added/updated/expanded support for a fnord property, will you please release a new Less version for me?" issue.

CSS is a well-defined standard. You shouldn't support anything outside the standard. That's your problem, I feel. You mustn't support the fnord property, because it isnt part of any standard. When this new unspecced property happens, LESS may fall back to its default behaviour, which could be the current, or requiring parentheses, or whatever else as long as it isn't ambiguous.

Meet the font issue.

The font issue proves that the / problem has existed since the absolute beginnings of LESS. Not just when background got the capability to include a value for background-size or when border-radius came about. Tbh, by stating the font issue, you've just made the best argument against yourself :)

seven-phases-max commented 6 years ago

Tbh, by stating the font issue, you've just made the best argument against yourself :)

I think you misunderstand something. It was me who initially proposed to totally remove the / as a div operator. So for the rest of stuff I suppose it's the same issue of you not paying attention to what you're replied with.

rjgotten commented 6 years ago

CSS is a well-defined standard.

If you're going to stick to the specification parts that have reached fully maturity at the TR level, maybe. Otherwise? No it's not. There are satellite specifications of new CSS 'modules' and revisions of existing modules with new additions in them popping up monthly, if not weekly.

matthew-dean commented 6 years ago

@seven-phases-max

it's not going to do the trick since there're things like: font: small-caps bold 24px/3 ...

No, I get that. My point was exactly that: unit-less only div/multiplication lessens the problem but doesn't solve the problem.

And I think in general your suggestion of ./ for all division still seems quite logical.

@thany Rather than responding to all the specific examples, I'll say that in general, the efforts to make Less math smarter will just kick the ball to a different yard line. It's still the same problems, just in a different spot. As @seven-phases-max has said, nothing you've suggested hasn't been discussed.

And I do not see any benefit of vastly overenginneering the complier in order to optimize the expressions inside calc. If you're writing calc then you're fine with the browser to do the job whatever long expression you have there. So as already mentioned above I'm for "don't touch anything inside calc" (= don't try to be smarter than it's really necessary) and leave it for browser (or for a css-minifier since, if I recall correctly, some of them already do pretty good job on optimizing calc subexpressions).

I agree with this entirely. We would kill 99% of new math-related issues if we just whitelisted calc(). While I've suggested some ways that Less could more smartly do math to ALSO avoid calc() issues (which maybe it should), I apologize if that was interpreted as an argument against this idea. I support this too.

The only caveat (as discussed here/elsewhere) is that a developer will inevitably want to use variables in calc(), so we need to be careful to make sure we continue to swap out vars, but just don't do math. I'm not sure how challenging that is. If we managed to do that, I would support a change in calc() handling today.

@thany

The font issue proves that the / problem has existed since the absolute beginnings of LESS.

This may be true and is a fair point, but there were many suitable workarounds and it wasn't necessarily standard practice at the time to use the font property shorthand. It's really since multiple backgrounds and calc() arrived after Less started that made this more of an issue, and now the CSS Grid syntax means there is now a ton of conflict where initially none existed in everyday practical terms.

matthew-dean commented 6 years ago

Incidentally, this is how Sass resolves the problem, illustrated by this example:

p {
  font: 10px/8px;             // Plain CSS, no division
  $width: 1000px;
  width: $width/2;            // Uses a variable, does division
  width: round(1.5)/2;        // Uses a function, does division
  height: (500px/2);          // Uses parentheses, does division
  margin-left: 5px + 8px/2px; // Uses +, does division
  font: (italic bold 10px/8px); // In a list, parentheses don't count
}

It's not a one-to-one, since you can do math (currently) within arguments to Less functions, AND Less functions can return pixel values (so Less could do font: print10px()/8px, theoretically? no?), so I'm just pasting it illustratively, not as any sort of suggestion. It's just interesting how they approached the problem.

Personally, I think making developers try to remember in which cases math will occur based on the existence of magic expressions is kind of crazy-making (like being pre-pended by a +?), but to each their own.

seven-phases-max commented 6 years ago

Personally, I think making developers try to remember in which cases math will occur based on the existence of magic expressions is kind of crazy-making

+1


Not even counting that the "solution" solves none of the problems discussed here beyond what lessc --sm does already. Why would I'll write 0 + 1/2 instead of (1/2) when what I want is just a division? And different results for 1/2 and $var/2 is just a phenomenally brilliant ... ~stupidity~ "Happy debugging, losers!" message.

thany commented 6 years ago

@matthew-dean Incidentally, this is how Sass resolves the problem, illustrated by this example:

That's a brilliant solution without resorting to a different operator. If the expression is possibly valid CSS, leave it.

Personally, I think making developers try to remember in which cases math will occur based on the existence of magic expressions is kind of crazy-making

Disagree. It's a simple set of rules, no different from any other sets of (crazy or not) rules you need to remember.

There are a few edge-cases where maths does occur without meaning to, but then you'll just apply parentheses and you're done.

matthew-dean commented 6 years ago

Not even counting that the "solution" solves none of the problems discussed here beyond what lessc --sm does already. Why would I'll write 0 + 1/2 instead of (1/2) when what I want is just a division? And different results for 1/2 and $var/2 is just a phenomenally brilliant ... stupidity "Happy debugging, losers!" message.

Ha, yes, this.

@thany

That's a brilliant solution without resorting to a different operator. If the expression is possibly valid CSS, leave it.

Not to get into the weeds, but it's not going to work for Less, for syntactical reasons and also language consistency. It may be brilliant for Sass, which I would say is arguable, but I'm not personally involved with that project from a design perspective, so who's to say. All I know is that the options on the table are the best place to start, and I'm confident if you spent as much time going through some of the message threads related to this issue and spent time with the language, you'd come to the same conclusion.

thany commented 6 years ago

One observation we cannot get away from: if you import valid vanilla CSS, its output should be functionally identical. So the only conclusion I can draw is that LESS should not touch expressions that are already possibly valid CSS. How this is done, is not up to me. But the fact remains that importing vanilla CSS is unreliable, mostly as a result of LESS executing divisions too eagerly.

Importing vanilla CSS in Sass works virtually flawlessly, because as said, Sass leaves the / operator alone if it doesn't indicate to the compiler to go and do stuff, according to a few simple syntax rules. These syntax rules describe ways to force a division that cannot occur in valid vanilla CSS, and that's why it's brilliant.

seven-phases-max commented 6 years ago

You still argue with your own imaginations and not with what they tell you. Answer the simple question: "How 0 + 1/2 can be possibly better than (1/2)?". If 100%-CSS compatibility is the only thing you care set the lovely -sm and forget about this thread.

matthew-dean commented 6 years ago

One observation we cannot get away from: if you import valid vanilla CSS, its output should be functionally identical. So the only conclusion I can draw is that LESS should not touch expressions that are already possibly valid CSS. How this is done, is not up to me. But the fact remains that importing vanilla CSS is unreliable, mostly as a result of LESS executing divisions too eagerly.

This part I essentially agree with, and I think you'd find a lot of agreement in this thread on that point. Most of the disagreements have been around solutions, with each solution having various side-effects.

I'd be on-board with making these breaking changes as a priority:

  1. math outside parentheses requiring ./ instead of a naked /. (And, obviously, function arguments requiring an extra set of parentheses e.g. my-function(12px/10px, arg2, arg3) does not divide 12/10 but my-function((12px/10px), arg2, arg3) does and my-function(12px./10px, arg2, arg3) does) Unless we want to do some other special indicator of / such as \/ or just \ e.g. 12px\10px. 12px./10px still looks a little weird.

@seven-phases-max - I want to revisit the backslash. Related to this, I know you mentioned https://github.com/less/less.js/issues/1872#issuecomment-35245890, but I don't see any outright conflict, since those identifiers are not and I think cannot be part of math expressions. Or am I wrong? I suppose it's possible to come up with a theoretical case like a function name with an escaped character as part of the name, but that seems more like an intellectual exercise than a real-world case. Escaped selectors, yes, those are commonly used for various reasons, but in property values, it just seems unlikely, or, in an edge case, okay to break / workaround with the historical (just escape this text) workaround.

Can we explore a bit more that using a single backslash for division would cause real-world conflicts? My instincts are that \ is less problematic than the current situation around /, and it's more dev-friendly than ./. If we did the research and found it was feasible, then we could make the breaking change to use it everywhere, instead of context-switching allowing / in parentheses. And then we could support / with a legacy switch.

  1. (second priority) calc() being special-cased so that it can do variable replacement but will not evaluate any math expressions
seven-phases-max commented 6 years ago

Can we explore a bit more that using a single backslash for division would cause real-world conflicts?

Well, the problem that \anycharacter is valid CSS and has its own meaning. Sure, you barely find such code in real projects (except maybe the ie \9-like hacks), but... do we want to feed that lion? Fixing one "uh-oh, my 100%-valid CSS does not compile" by introducing another "uh-oh" of the same sounds a bit weird :)

As for calc I guess we had a consensus from the very beginning - so it's basically only waits for a volunteer to implement it (I estimate the quick fix to be done in just 5-6 lines of new code - so it's really only about a brave man to do this - minor implementation details may vary but I think they are fine to be decided in process).

matthew-dean commented 6 years ago

Well, the problem that \anycharacter is valid CSS and has its own meaning. Sure, you barely find such code in real projects (except maybe the ie \9-like hacks), but... do we want to feed that lion? Fixing one "uh-oh, my 100%-valid CSS does not compile" by introducing another "uh-oh" of the same sounds a bit weird :)

I hear you, it's a possible trade-off. And yeah I know that \anycharacter is valid CSS. I guess I'm wondering if it's a better set of trade-offs. It's just that when I wrote out 12px./10px it just felt weird, syntactically. I feel like Less syntactically has tried to re-purpose CSS as much as possible without creating conflicts.

Like, ./ provides syntax clarity and completely avoids conflict, but so did enforcing parentheses everywhere for math, which is why I supported it, but there was backlash, and I'm worried about that here. So is there any legitimate case where we would mistake 10px\10 as the developer's intent to escape \10? I know that's a hard theoretical, and your point would probably be that we don't really know.....It's a complicated question, and adding new syntax is always fraught, especially with Less since we don't know all CSS in the wild nor future CSS.

matthew-dean commented 6 years ago

As for calc I guess we had a consensus from the very beginning - so it's basically only waits for a volunteer to implement it (I estimate the quick fix to be done in just 5-6 lines of new code - so it's really only about a brave man to do this - minor implementation details may vary but I think they are fine to be decided in process).

Good! Should we track it separately to bump visibility?

seven-phases-max commented 6 years ago

@seven-phases-max:

Sure, you barely find such code in real projects (except maybe the ie \9-like hacks)

Right. Oh that bumptious "westerners"... :)

thany commented 6 years ago

@seven-phases-max Answer the simple question: "How 0 + 1/2 can be possibly better than (1/2)?"

I don't see where you're going with that. (1/2) should be executed by LESS because it can't possibly be valid CSS, so it must be meant as LESS. 0 + 1/2 should become 1/2 where LESS executes the 0 + 1 part because that's the part that cannot be valid CSS. The 1/2 part could be valid, so better not touch it.

seven-phases-max commented 6 years ago

@thany OK, now realize (1/2) works as you expect in both Less and Sass.. And 0 + 1/2 (as well as 1 * 1/2 etc.) is the part of the solution you call brilliant above and "the briliant soution" resut is 0.5. Still no clue what's happening here? Reread everything (starting from you first post) above once more and try to answer to yourself "What exactly I'm complaining about?".

thany commented 6 years ago

@seven-phases-max And 0 + 1/2 (as well as 1 * 1/2 etc.) is the part of the solution you call brilliant above and "the briliant soution" resut is 0.5.

No, the solution would be 1/2 not 0.5, since 1/2 might be valid CSS. You don't seem to want LESS to know when a slash is valid, so assume it always might be. Therefor 1/2 is the only logical outcome. In the same way, 2 * 1/2 would result in 2/2, because the * would otherwise make it invalid CSS.

Still no clue what's happening here?

I'm perfectly clear what's happening here. LESS is executing math divisions way eagerly and blindly ignores units.

Reread everything (starting from you first post) above once more and try to answer to yourself "What exactly I'm complaining about?".

There's no need for personal attacks.

seven-phases-max commented 6 years ago

LESS is executing math divisions way eagerly and blindly ignores units.

So this is what you're complaining about, right?

There's no need for personal attacks.

So what should I do instead? Repeating the original two posts again and again (and again)? Until it comes clear to you: @community:

use -sm option

@thany:

then it should be on by default.

@seven-phases-max:

It can't be because this would immediately break zillion projects out there. So if you treat the default behaviour as an issue just set the documented option and be done.


So you're repeating "it should", "it should", "it should" expecting what? Guess for us it's easier to answer just "No, it should not" instead of wasting our time trying to explain in details on why what you suggest is not going to work or makes no sense in general (the explanations you either do not want to understand or simply can't).


So what this thread is about? It's about "realizing that while an eventual breaking change of making an "-sm-like behaviour default is unavoidable, we have to provide facilities for more comfortable arithmetic other than the dreadful: margin: (1/2) (3/4) (5/6) (7/8); for those who use the arithm a lot". You posts here either suggest something that is no way better than (or doing exactly the same as) already existing -sm behaviour or argue with something that is out of question from the very beginning (like there). In other words, just a random noise.

matthew-dean commented 6 years ago

@seven-phases-max @thany Let's take the temperature down a notch. There are strong opinions all around.

I think most people are on the same page around both of these ideally not being interpreted by Less as a math expression:

font: 12px/10px;
width: calc(100% - 20px);

So, there's mostly agreement on those points, and most of the arguments are around possible solutions. The calc() problem looks like it has enough consensus to move forward. Basically: don't touch calc, and treat vars in calc() like string interpolation. That's pretty much how Sass does it, although it requires the interpolation syntax in calc(), which I don't think we need.

The solutions to the font part gets a lot hairier and the leading solutions have been:

  1. Require parens for everything by default (first attempt - almost implemented, but community rejected)
  2. Flip the strictMath switch (which was added instead of making it the default, and @seven-phases-max's suggestion) and then explicitly do all your math. It is, technically, a possible solution for most people at present, but....the question comes up so frequently and we know, psychologically, that someone new to any system likely keeps any defaults, so it's problematic. And not every developer can change their build settings, especially in teams.
  3. Essentially change the division operator in Less. Leading contender in the thread has been ./, which works but is a little strange to look at. \ is an unknown at this point without research. Could work, could cause unknown conflicts with escaping. It's cleaner syntax, at (potentially, but unknown) higher risk. If someone would take the time to demo that this will NOT cause conflict i.e. providing examples of escaping and math inter-mingled in a way the parser can clearly distinguish the two, then it's still a possibility. So it just takes the work. @thany, if you or someone else is a fan of this, then this is the work required. The last thing we want is just to move the breaks somewhere else.

I think, to simplify this thread, we should, from here, focus JUST on #3. I posted the Sass example mostly as a curiosity, but I do not believe those solutions are good or completely solve the problem, and they are conceptually incompatible with Less. Arguing about the validity of requiring 0 + 1/2 is not going to get us anywhere.

So, with a good solution on the table for calc(), which gets us 50% of the way there for most posted issues, I would recommend only focusing on this question in the short term:

If the Less division operator should be changed, what should it be changed to?

Honestly, if we change calc() and change /, I think we would quiet 99% of the noise around math in Less.

matthew-dean commented 6 years ago

Hey all, I fixed calc() - https://github.com/less/less.js/pull/3162

I was kinda astonished when I came up with a solution that worked without much add-on code. Basically, we already had code in for the strictMath switch, and checks in expressions to not evaluate math if it was outside parens, so I just added an additional switch on the call to turn math off while evaluating the calc() args, and then back on. We already had all the pieces to leave math alone, but still substitute vars. Huh.

Check out https://github.com/less/less.js/pull/3162/files#diff-a94aaffd78b1d3c5eda7a42d5be1ca0d and https://github.com/less/less.js/pull/3162/files#diff-4e696271823c96903a91fff84983bab6

Are the tests sufficient?

seven-phases-max commented 6 years ago

@matthew-dean :coffee:

Are the tests sufficient?

per comments in the PR add please a test like:

foo: 1 + 2 calc(3 + 4) 5 + 6; // expected result: 3 calc(3 + 4) 11;

And a minor remark: this fix obviously introduces the same issue as in #1627, i.e.

@a: floor(1.1);
@b: floor(1 + .1);

div {
    foo: calc(@a + 20%); // ok
    bar: calc(@b + 20%); // error: invalid floor arguments
    baz: @b;             // ok
}

But for calc it is fine I guess (after all there's simply no other simple ways to fix it w/o major redoing of the lazy-eval concept). So I think it's just a matter of putting a notice into the docs to keep lazy-eval in mind when using vars within calc.

matthew-dean commented 6 years ago

And a minor remark: this fix obviously introduces the same issue as in #1627

Good catch!

Switching the context mathOn property per call solves this, similar to your comment in the PR. I've added a test for float(1 + .1) which passes!

seven-phases-max commented 6 years ago

Switching the context mathOn property per call solves this, similar to your comment in the PR. I've >added a test for float(1 + .1) which passes!

:) No, the "floor" error must be there when -sm: off. See my newer comment in the PR. The restoring only takes care of 1 + 2 calc(3 + 4) 5 + 6-like things.

matthew-dean commented 6 years ago

:) No, the "floor" error must be there when -sm: off.

Why? Any functions inside a calc() should be Less functions. Even if they're not, there are no CSS-native functions inside calc() that I'm aware of that can take raw math expressions. The expected result would be to evaluate vars and Less functions, but leave raw functions inside calc() alone.

matthew-dean commented 6 years ago

Side note: I did a branch experiment of switching / to \ as a division operator in Less. There are already lots of tests for escaping (and I added more, to address: #3160), and they all still work fine, and math works fine with the operators switched. I didn't see any inherent conflict. Branch is here on my fork: https://github.com/matthew-dean/less.js/commit/509d34fff7e234846afa150b099cd259755a39d0

matthew-dean commented 6 years ago

Re: nested functions

For this:

div {
    bar: calc(floor(1 + .1) + 20%);
}

As a developer, the expected result should be:

div {
    bar: calc(1 + 20%);
}

That's exactly what happens now (with these changes). It should not throw an error.

seven-phases-max commented 6 years ago

@matthew-dean No, the way you wrote it, this:

foo: unit(1/2, px);

with -sm:on will compile to:

foo: .5px;

^- wrong. Same for nested functions. Moreover, the fact that most of functions usually can't take arithmetic expressions as argument is not a reason to violate -sm: on; Thus under -sm: on both lines:

foo: floor(1 + .1);
bar: calc(floor(1 + .1) + 20%);`

must throw a error (and this is what gets broken in the commit).

matthew-dean commented 6 years ago

What are you talking about?

unit(1/2, px) with -sm:on:

ERROR: error evaluating function `unit`: the first argument to unit must be a number. Have you forgotten parenthesis?

calc(floor(1 + .1) + 20%) with -sm:on

ERROR: error evaluating function `floor`: argument must be a number

Check out the branch. Try it out.

matthew-dean commented 6 years ago

One thing that might be more helpful with reviewing code is that if you aren't sure if something will produce incorrect output, please verify it. Or ask me to try a specific input to verify it works as expected. Or ask questions if you're not sure how / why it works. Calling it wrong without knowing if it is is not helpful.

seven-phases-max commented 6 years ago

My apologizes. I guess I missed the indirect control of a this part.

Check out the branch. Try it out

I can't, sorry.

matthew-dean commented 6 years ago

My apologizes. I guess I missed the indirect control of a this part.

No worries. With that addressed, does it look like it's working as you would expect?

seven-phases-max commented 6 years ago

With that addressed, does it look like it's working as you would expect?

Yes, so far I can't imagine any other suspicious things there.

thany commented 6 years ago

@matthew-dean Essentially change the division operator in Less. Leading contender in the thread has been ./, which works but is a little strange to look at. \ is an unknown at this point without research [...] @thany, if you or someone else is a fan of this, then this is the work required. The last thing we want is just to move the breaks somewhere else.

Not at all actually. I suggested the \ operator as a last resort after not getting through, trying to get LESS to do only its own maths. If a new division operator is what it takes... Well, calc() can also do addition, subtraction, and multiplication. New operators for those? I think it's impractical. A new division operator might be a solution for things like font, background, and border-radius, but it's no solution for CSS maths.

I still think the real solution is for LESS to know about context. It should (yes, here I go again with my "should", what other word must I use?...) know that calc() is a CSS function and it should only evaluate maths that CSS can't do. But floor() doesn't exist in CSS, so that one it would have to evaluate (entirely) as to not spew out invalid CSS. I think this is mentioned before though, in different wording.

matthew-dean commented 6 years ago

I still think the real solution is for LESS to know about context. It should (yes, here I go again with my "should", what other word must I use?...) know that calc() is a CSS function and it should only evaluate maths that CSS can't do. But floor() doesn't exist in CSS, so that one it would have to evaluate (entirely) as to not spew out invalid CSS. I think this is mentioned before though, in different wording.

Fair enough re: \, but as far as context, based on your other posts, I can guarantee it's a far more complex problem than you're thinking that it is. You really have to boil it down to:

12px/10px  // Is this division, or not?
10px/1.5 // is this division, or not? 

If you want to definitely leave / as a division operator, to mimic calc(), and not have it be ambiguous, then you absolutely need to make sure that the / is in parentheses (other than the calc() exception). That would provide predictable context.

That suggestion at the beginning of this thread is also still a possibility, and had some strong support. Essentially, that the default setting be that all math is supported outside parentheses EXCEPT division (and again, except calc(), is is now the case from 3.0+). Maybe that is the way to go. That would allow:

font: 10px/1.5;   // not division
font: (10px/10);  // division, result is 1px
font: 10px+15px/1.5;   // addition but not division, result is 25px/1.5
font: (10px+15px/1.5);  // result is 20px

Thing is, the result for 10px+15px/1.5 might still be hard for devs to grok, with an expression that seems to get "half-evaluated" unless it's in parentheses. If we had gone ahead and turned strict math on by default, it probably would have been fine. It's even less ambiguous. [shrug] Either way, wrapping math in some kind of context IS the way to eliminate ambiguity, and the only way for division other than changing the division operator.

The community has to essentially decide the direction. There are viable options in this thread. Each has downsides. Each has pain points. None will have full consensus. But IMO any of them are better than the current scenario. Just gotta swallow that pill.

matthew-dean commented 6 years ago

Last call for a decision

So in an effort to work the impossible, I'd like to propose we do this (referring back to comments from 3 years ago.)

Give --strict-math 3 settings

  1. off
  2. division
  3. strict (alias on for back-compat)

To settle the debate, allow the --strict-math=division switch to do the following:

  1. Do not perform division with just / character outside of parentheses. E.g. border-radius: 55px / 25px; --> no-math (yes, that's valid CSS)
  2. Allow division with two different methods: a. . prefix - border-radius: 55px ./ 25px; b. parentheses - border-radius: (55px / 25px);
  3. Both forms are valid in parentheses - e.g. border-radius: (55px ./ 25px); would be valid

So, as a developer, if you feel one version is not your preference, you can use the other. Some may prefer not to use parentheses; some may prefer not to use a modified division operator. Something for everyone. And no more unpleasant surprises for those new to Less using what is now a widespread syntax in CSS, with the / being used in font, background, border-radius, @media queries, CSS Grid properties, and probably more in the future.

Next steps

I recommend this goes into a PR as an option, and then, as discussed, gets switched as the default for a future major version

rjgotten commented 6 years ago

@matthew-dean

I.e. the period acts sort of like the inverse of an escape sequence? Not a bad idea, really...

And all things considered, probably one of the cleanest possible solutions.