Skip to content

/ rust-cssparser

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

Implement color value serialization. #60

Merged
merged 4 commits into from Oct 1, 2014
Merged

Implement color value serialization. #60

merged 4 commits into from Oct 1, 2014

Conversation

@jdm
Copy link
Member

jdm commented Sep 19, 2014

No description provided.

@jdm jdm force-pushed the jdm:serialization branch from a4ddae1 to 5d410a8 Oct 1, 2014
jdm added 3 commits Sep 19, 2014
@jdm
Implement basic, incorrect color value serialization.
ca04a3b
@jdm
Improve color serialization to reflect rgb/rgba rules.
5b8e47c
@jdm
Fix color serialization.
Loading status checks…
3716158
@jdm jdm force-pushed the jdm:serialization branch from 5d410a8 to 3716158 Oct 1, 2014
@jdm jdm changed the title Implement basic, incorrect color value serialization. Implement color value serialization. Oct 1, 2014
@jdm
Copy link
Member Author

jdm commented Oct 1, 2014

r? @SimonSapin
@SimonSapin
Copy link
Member

SimonSapin commented Oct 1, 2014

Please add a couple of unit tests. Right now format!("{}", 4f32) does not include a dot (which would make it non-integer and fail parsing as a CSS color) but I’m not confident that it never will. (The Python equivalent returns 4.0.)

Also, rounding loses precision. Maybe that’s OK?
@jdm
Copy link
Member Author

jdm commented Oct 1, 2014

The precision loss matches Firefox; I think Blink as well?
@SimonSapin
Copy link
Member

SimonSapin commented Oct 1, 2014

Ok, then. r+ with unit tests.
@jdm
Add color serialization unit tests.
Loading status checks…
03231db
@jdm
Copy link
Member Author

jdm commented Oct 1, 2014

Added!
SimonSapin added a commit that referenced this pull request Oct 1, 2014
@SimonSapin
Merge pull request #60 from jdm/serialization
Loading status checks…
7b0549a 
Implement color value serialization.
@SimonSapin SimonSapin merged commit 7b0549a into servo:master Oct 1, 2014
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
@jdm @SimonSapin
You can’t perform that action at this time.