-
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
How to handle Maths #1880
Comments
Yes, I suggested to start this only because if we want some "default" strict math in 3.0 we'll have to invent something less heavy than the current one (and a possibility of fixing all the problems without introducing any new |
I hadn't realised that its growing https://developer.mozilla.org/en-US/docs/Web/CSS/border-radius :( I think at the moment I am in favour of initially expanding strictMaths to be
and then for 2.0.0 setting the default to be Division So.. Media Queries - if off, switch to division for sub nodes |
Yep, I think they are towards to allowing "shorthand values" (i.e. |
related: #1480 |
Another option.. process calc calls but only where the units make it possible e.g |
This will fix |
If strict maths is being revisited, I'd like to suggest that Less functions like |
@calvinjuarez V2 provides a plugin subsystem where a plugin can add an arbitrary number of functions to the environment and in this sense the core won't be able to decide if such built-in function expects a single value or an expression, i.e. it is allowed to accept In that context |
Another thing where less currently breaks valid css is background shorthand:
|
Sorry I didn't respond to this before 2.0 was released. I think that's a good instinct. Naked division operators simply conflict with too much of CSS, and will conflict more and more as people use more CSS3 features. And I understand people's pushback to not require parens for all math, since it's not needed in many cases.
Strictly speaking, yes that's absolutely correct. This should not be evaluated BEFORE it reaches the function, especially for a custom one. However, I think it would be smart to allow functions to opt to process arguments like this, if it makes sense. So, percentage would receive 16/17 as a text argument, and then the percentage function could make a call to some helper function to see if the text is math. In the case of percentage, the argument is not ambiguous. A CSS3 declaration is not valid here. So, other user-defined functions could operate the same way: opt to evaluate arguments for valid math equations. Therefore, it's not necessarily true that strict math "forces" a double parentheses in this case. I think to suggest that causes some pushback on the idea of strict math, that it must, as a necessity, be verbose in all cases. |
Our --math options could be more like:
So, we could deprecate --strict-math=true and make it an alias for --math=strict |
They are allowed already (just test how
simply because this way each function would have to have those ~20 extra lines of that extra-helpers-conversion-arg-checking-smart-arg-handling-stuff while the own function code is just one(!) line in most cases. |
Each function has to have 20 extra lines? How do you figure? If percentage already works with single parentheses strict math on, then I don't understand @calvinjuarez's issue. You seemed to imply in your response that single parentheses was not achievable. |
That's just a typical exaggeration of: maybe 5, maybe 10, maybe 20... Who would care of exact numbers if real-code/aux-code ratio -> My initial reply was implying he assumes this Speaking of options. In a perfect world I would dream there're be no options for this at all (i.e. it should be the one and the only and the rest to be marked as deprecated and eventually removed)... All these extra options make the codebase non-maintainable.. even for a tiny one-liner fix/change the number of edge-cases you have to predict and tests you need to perform grows exponentially... (almost for no reason). |
I was just thinking it would be something like:
I agree. But we'll need the options in the short term. Especially if we're deviating from strict math and legacy behavior. They can both be marked as deprecated if we perfect a "smarter math" option. |
But that was not my point really, what I meant is probably something like: while this always is/was possible, nobody bothers to write such code... (with a few limited exceptions like)... |
Yeah, I get you. |
would be an improvement, certainly. (I can't think of any time that As far as which functions would try to be smart about their arguments, there's no reason to try to anticipate everything. A helper function as @matthew-dean suggests could be implemented fairly simply as feature requests are made and discussed for specific functions to be smarter.
Edit: Actually, just whenever a math function is passed only one argument. |
What's the status on this? We're getting complaints that LESS tries to parse invalid CSS properties as math. e.g. It also seems http://www.w3.org/TR/css-grid-1/#grid-template-rowcol won't work with LESS. Can someone patch this so if someone explicitly wants to use LESS to do division on a property that they need to wrap it in parens or something? |
@corysimmons Didn't you just ask this on #2769 and get an answer? o_O |
One thing that we didn't discuss when this went around the first time. It seems that the only real math conflict is division. And division is essentially an issue because we essentially are "repurposing" a CSS "separation" character. Rather than try to "wrap" division in any number of ways, or wrap all math (as our first solution to this problem) I wonder why we never talked about the obvious: not repurposing an existing operator in the first place, especially when it so clearly a) makes Less code ambiguous, b) causes conflict. After we've had time to digest this, wrapping math in parentheses, even if it's just division, still causes ambiguity for the cases where the math is already in parentheses. (Which could have also meant that parentheses was a wrong choice of "math wrapper".) So why not deprecate To be fair, the other math symbols are all repurposed in other parts of CSS, it's just that their usage in math doesn't cause any ambiguity. I was going to propose some ideas, but, lo and behold, there are already standard alternative characters to division. Other than
In other words: lost-column: 1/3; // value
lost-column: 1:3; // division I know it would take a bit of parser tweaking to use a colon in math, but it is mathematically sound, apparently. I can't think of other symbols that would make better substitutes. Maybe Thoughts? |
Alternatively, we could still support the division symbol within parentheses and/or brackets, as in:
|
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. |
@matthew-dean foo: unit(1/2, px); with
^- wrong.
must throw a error (and this is what gets broken in the commit). |
What are you talking about?
Check out the branch. Try it out. |
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. |
My apologizes. I guess I missed the indirect control of a this part.
I can't, sorry. |
No worries. 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. |
Not at all actually. I suggested the 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 |
Fair enough re: 12px/10px // Is this division, or not?
10px/1.5 // is this division, or not? If you want to definitely leave 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 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 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. |
Last call for a decisionSo in an effort to work the impossible, I'd like to propose we do this (referring back to comments from 3 years ago.) Give
To settle the debate, allow the
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 Next stepsI recommend this goes into a PR as an option, and then, as discussed, gets switched as the default for a future major version |
I.e. the period acts sort of like the inverse of an escape sequence? And all things considered, probably one of the cleanest possible solutions. |
@rjgotten Got a start here: https://github.com/matthew-dean/less.js/tree/strict-math-division I'm still getting a weird error one of the tests, where it it seems to turn one of the dimension nodes into an operation node (and then throws an error because it can't perform an operation on an operation node. It doesn't seem to be a parsing problem because another test not running under |
What I'd like to do is close this issue and create new issues that handle the remaining math questions. Specifically:
|
Fixes #1880 - Adds two new math modes and deprecates strictMath
* WIP - Added strictMath: 'division' option * Removes `less-rhino` (broken for a long time) - Fixes #3241 * Expressions require a delimiter of some kind in mixin args * Removes "double paren" issue for boolean / if function * WIP each() re-implementation * Allows plain conditions without parentheses * Added each() and tests * Added tests calling mixins from each() * Fixes #1880 - Adds two new math modes and deprecates strictMath * Allows named args for detached ruleset (anonymous mixin) * Makes sure this doesn't regress needing parens around mixin guard conditions * Remove javascriptEnabled from browser options check, add to defaults * Workaround weird Jasmine conversion of < to HTML entity * Removes remaining Rhino cruft * Remove rhino files from bower * Bump Jasmine version * Added .() example to each * v3.6.0 * Update CHANGELOG.md * add explicit nested anonymous mixin test * add explicit nested anonymous mixin test result * derp: align test with expected output * v3.7.0 (#3279) * Update CHANGELOG.md * Update CHANGELOG.md * Update CHANGELOG.md
* Rename relativeUrls option to rewriteUrls * Refactor contexts.js - Rename isPathRelative to pathRequiresRewrite - Add special handling for rewriteUrls=relative - Add rewritePath for path rewriting - Ensure that explicit relative paths stay explicit relative - Remove code style inconsistencies * Add missing tests for url rewriting * Rename rewrite-urls option value to explicit-relative * Add missing tests for url rewriting * Refactor rewrite urls options - Rename explicit-relative to local - Add rewrite-urls argument validation - Replace unknown CSS property background-url with background-image Based on discussion #3041 * Re-add tests for deprecated relative-urls option * Improve tests - Remove redundant tests - Add rootpath-rewrite-urls-all test - Add rootpath-rewrite-urls-local test * Fix typo in unknown argument warning * Revert old tests to deprecated relativeUrls option again * Added more CSS Grid tests * All tests passing * Merge branch 'master' into feature/rewrite-urls (#3282) * WIP - Added strictMath: 'division' option * Removes `less-rhino` (broken for a long time) - Fixes #3241 * Expressions require a delimiter of some kind in mixin args * Removes "double paren" issue for boolean / if function * WIP each() re-implementation * Allows plain conditions without parentheses * Added each() and tests * Added tests calling mixins from each() * Fixes #1880 - Adds two new math modes and deprecates strictMath * Allows named args for detached ruleset (anonymous mixin) * Makes sure this doesn't regress needing parens around mixin guard conditions * Remove javascriptEnabled from browser options check, add to defaults * Workaround weird Jasmine conversion of < to HTML entity * Removes remaining Rhino cruft * Remove rhino files from bower * Bump Jasmine version * Added .() example to each * v3.6.0 * Update CHANGELOG.md * add explicit nested anonymous mixin test * add explicit nested anonymous mixin test result * derp: align test with expected output * v3.7.0 (#3279) * Update CHANGELOG.md * Rewrite the rewriteUrls feature to use same pattern as strictMath-to-math * Update console warning for --relative-urls
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 atpadding
/margin
and ending withbackground
/border-radius
and eventually there can be more) so we just can't hardcode them all like it's done forfont
(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).The text was updated successfully, but these errors were encountered: