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

[css-values] Serialize all functions in summation #3785

Merged
merged 2 commits into from Apr 4, 2019

Conversation

ewilligers
Copy link
Contributor

@ewilligers ewilligers commented Apr 1, 2019

We previously only allowed for min and max in summation.
https://drafts.csswg.org/css-values-4/#math-function-serialize-a-summation

We can now also have clamp, sin, ..., hypot.

We previously only allowed for min and max in summation.
https://drafts.csswg.org/css-values-4/#math-function-serialize-a-summation

We can now also have clamp, sin, ..., hypot.
@ewilligers ewilligers added the css-values-4 Current Work label Apr 1, 2019
@ewilligers
Copy link
Contributor Author

ewilligers commented Apr 1, 2019

A summation may also contain products or quotients of functions. I suspect they should also have their order unchanged. calc(1 + calc(2px / 2em) + 3 + sqrt(16) + calc(5px / 5em)) should become calc(4 + calc(2px / 2em) + sqrt(16) + calc(5px / 5em))

@ewilligers ewilligers requested a review from frivoal April 1, 2019 01:41
@AmeliaBR
Copy link
Contributor

AmeliaBR commented Apr 1, 2019

calc(1 + calc(2px / 2em) + 3 + sqrt(16) + calc(5px / 5em)) should become
calc(4 + calc(2px / 2em) + sqrt(16) + calc(5px / 5em))

The nested calc() should be dropped in favour of simple parentheses per the rules on simplifying a calculation. But you're correct that this complicates the sort order considerably, since terms can have all sorts of combinations of units.

I believe the whole purpose of sorting terms at all is so that two algebraically equal expressions have the same simplification. To be consistent with that goal, we should probably define full sorting rules for complex dimensions (e.g., by number of dimensions and then alphabetically by numerator units and then by denominator units and then by numerical magnitude...) and for functions (e.g., alphabetically by function name). So, your equation would be more like:

calc(4 + (2px / 2em) + (5px / 5em) + sqrt(16))

However, since serialization currently supports algebraic simplification, we could expect it to combine terms with equivalent dimensions, and maybe compute out trig functions of simple numbers:

calc(8 + (2px / 1em) )

Either way, we need a spec! But I see no problem with merging your simple PR and then opening a separate issue for the full discussion.

@tabatkins tabatkins merged commit 3c0c987 into w3c:master Apr 4, 2019
@tabatkins
Copy link
Member

Yeah, this is acceptable for now. I don't think we want to do additional sorting into the complex terms; the value seems very low for the effort required for all parties.

@ewilligers ewilligers deleted the function-summation branch April 4, 2019 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-values-4 Current Work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants