Skip to content

color: Add a way to hook into the parsing of the components of the color functions. #207

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

Merged
merged 1 commit into from
Dec 2, 2017

Conversation

emilio
Copy link
Member

@emilio emilio commented Dec 1, 2017

This way we'll be able to implement calc-in-color without duplicating a ton of
code for calc() handling in Servo.

Minor version bump because the API is not broken, just expanded to support
providing an optional ColorComponentParser.


This change is Reviewable

@emilio
Copy link
Member Author

emilio commented Dec 1, 2017

r? @SimonSapin

@SimonSapin
Copy link
Member

Do you have a Servo patch yet that uses this API? (To make sure it is sufficient, e.g. in BasicParseError v.s. ParseError.)

r+ with changes if so


Reviewed 2 of 2 files at r1.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


src/color.rs, line 151 at r1 (raw file):

pub enum NumberOrPercentage {
    Number(f32),
    Percentage(f32),

Please use Percentage { unit_value: f32 } to remind that "100%" is represented as 1.0_f32, not 100.0_f32, and add a doc-comment explaining that. (And maybe Number { value : f32 } for consistency.)


src/color.rs, line 155 at r1 (raw file):

impl NumberOrPercentage {
    fn get(&self) -> f32 {

Please rename this to unit_value(), with a doc comment repeating the 100% => 1.0 thing.


Comments from Reviewable

…lor functions.

This way we'll be able to implement calc-in-color without duplicating a ton of
code for calc() handling in Servo.

Minor version bump because the API is not broken, just expanded to support
providing an optional `ColorComponentParser`.
@emilio emilio force-pushed the towards-calc-color branch from bf479d5 to 97890a3 Compare December 1, 2017 22:54
@emilio
Copy link
Member Author

emilio commented Dec 1, 2017

Can you take another look? Had to add a couple things for servo.

@SimonSapin
Copy link
Member

@bors-servo r+


Reviewed 2 of 2 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@bors-servo
Copy link
Contributor

📌 Commit 97890a3 has been approved by SimonSapin

@bors-servo
Copy link
Contributor

⌛ Testing commit 97890a3 with merge 6e8a5a4...

bors-servo pushed a commit that referenced this pull request Dec 1, 2017
color: Add a way to hook into the parsing of the components of the color functions.

This way we'll be able to implement calc-in-color without duplicating a ton of
code for calc() handling in Servo.

Minor version bump because the API is not broken, just expanded to support
providing an optional `ColorComponentParser`.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-cssparser/207)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: SimonSapin
Pushing 6e8a5a4 to master...

@bors-servo bors-servo merged commit 97890a3 into master Dec 2, 2017
bors-servo pushed a commit to servo/servo that referenced this pull request Dec 2, 2017
style: support calc() in color functions.

Depends on #19456 and servo/rust-cssparser#207.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19457)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Dec 2, 2017
style: support calc() in color functions.

Depends on #19456 and servo/rust-cssparser#207.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19457)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Dec 5, 2017
style: support calc() in color functions.

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19457)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Dec 5, 2017
style: support calc() in color functions.

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19457)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Dec 5, 2017
…emilio:color-calc); r=SimonSapin

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

Source-Repo: https://github.com/servo/servo
Source-Revision: 3cef09ae217ece7fa276d1be653c7c36dee7febc

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 5e87eadeda400ec41303d8d2aa506bf65fd86ea4
xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Dec 6, 2017
…emilio:color-calc); r=SimonSapin

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

Source-Repo: https://github.com/servo/servo
Source-Revision: 3cef09ae217ece7fa276d1be653c7c36dee7febc
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…emilio:color-calc); r=SimonSapin

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

Source-Repo: https://github.com/servo/servo
Source-Revision: 3cef09ae217ece7fa276d1be653c7c36dee7febc

UltraBlame original commit: 43e678a70a2a7175b59ebd5e84ab6e071ec9e009
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…emilio:color-calc); r=SimonSapin

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

Source-Repo: https://github.com/servo/servo
Source-Revision: 3cef09ae217ece7fa276d1be653c7c36dee7febc

UltraBlame original commit: 43e678a70a2a7175b59ebd5e84ab6e071ec9e009
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…emilio:color-calc); r=SimonSapin

Depends on #19456 and servo/rust-cssparser#207.

Fixes https://bugzilla.mozilla.org/show_bug.cgi?id=984021

Source-Repo: https://github.com/servo/servo
Source-Revision: 3cef09ae217ece7fa276d1be653c7c36dee7febc

UltraBlame original commit: 43e678a70a2a7175b59ebd5e84ab6e071ec9e009
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants