Skip to content

/ servo

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

Put stylo's pseudo-class into a list file #15153

Merged
merged 1 commit into from Jan 25, 2017
Merged

Put stylo's pseudo-class into a list file #15153

merged 1 commit into from Jan 25, 2017

Conversation

@upsuper
Copy link
Member

upsuper commented Jan 23, 2017 •
edited by larsbergstrom

r? @emilio

This change is Reviewable
@highfive
Copy link

highfive commented Jan 23, 2017

Heads up! This PR modifies the following files:

  • @bholley: components/style/gecko/selector_parser.rs, components/style/gecko/non_ts_pseudo_class_list.rs
  • @emilio: components/style/gecko/selector_parser.rs, components/style/gecko/non_ts_pseudo_class_list.rs
@highfive
Copy link

highfive commented Jan 23, 2017

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!
@emilio
emilio approved these changes Jan 23, 2017
View changes
Copy link
Member

emilio left a comment

Looks fine, r=me % comments.

Just one question, this is just in order to simplify stuff? or you plan to autogenerate/add more pseudo classes in the near future?
components/style/gecko/non_ts_pseudo_class_list.rs
* This file contains a helper macro includes all supported non-tree-structural
* pseudo-classes.
*
* This file is NOT INTENDED to be compiled as a standalone module.

This comment has been minimized.

Sign in to view
@emilio

emilio Jan 23, 2017

Member

This is not autogenerated right? Please note it in this comment (potentially keeping the FIXME that you removed from the other file).
components/style/gecko/non_ts_pseudo_class_list.rs
("active", Active, active, IN_ACTIVE_STATE, _),
("focus", Focus, focus, IN_FOCUS_STATE, _),
("fullscreen", Fullscreen, fullscreen, IN_FULLSCREEN_STATE, _),
("hover", Hover, hover, IN_HOVER_STATE, _),

This comment has been minimized.

Sign in to view
@emilio

emilio Jan 23, 2017

Member

I don't love the gecko_type name, but I don't have a better suggestion :)
@emilio
Copy link
Member

emilio commented Jan 23, 2017

@bors-servo delegate+
@bors-servo
Copy link
Contributor

bors-servo commented Jan 23, 2017

✌️ @upsuper can now approve this pull request
@upsuper
Copy link
Member Author

upsuper commented Jan 23, 2017

I don't have plan to auto-generate it at this moment, since we have far fewer pseudo-classes than Gecko currently. But I guess this kind of list would be easier to auto-generate once we want to do so. Before that, I think it is still useful to have this list so that it's trivial to add support for many of the pseudo-classes.
@upsuper
Put stylo's pseudo-class into a list file
Loading status checks…
4bb9821
@upsuper upsuper force-pushed the upsuper-forks:pseudo-class-list branch from 00387d5 to 4bb9821 Jan 23, 2017
@upsuper
Copy link
Member Author

upsuper commented Jan 23, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jan 23, 2017

📌 Commit 4bb9821 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jan 24, 2017

⌛ Testing commit 4bb9821 with merge a0b928f...
bors-servo added a commit that referenced this pull request Jan 24, 2017
@bors-servo
Auto merge of #15153 - upsuper:pseudo-class-list, r=emilio
Loading status checks…
a0b928f 
Put stylo's pseudo-class into a list file

r? @emilio

<!-- 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/15153)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 24, 2017

💔 Test failed - windows-gnu-dev
@upsuper
Copy link
Member Author

upsuper commented Jan 24, 2017

LLVM ERROR: IO failure on output stream.

:/
@upsuper
Copy link
Member Author

upsuper commented Jan 24, 2017

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented Jan 24, 2017

⚡ Previous build results for android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev are reusable. Rebuilding only windows-gnu-dev...
@bors-servo
Copy link
Contributor

bors-servo commented Jan 24, 2017

💔 Test failed - windows-gnu-dev
@Ms2ger
Copy link
Contributor

Ms2ger commented Jan 24, 2017

@bors-servo retry servo/saltfs#585
@jdm
Copy link
Member

jdm commented Jan 24, 2017

@bors-servo: r-
@mbrubeck
Copy link
Contributor

mbrubeck commented Jan 24, 2017

@bors-servo r=emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jan 24, 2017

📌 Commit 4bb9821 has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Jan 25, 2017

⌛ Testing commit 4bb9821 with merge dc8fc44...
bors-servo added a commit that referenced this pull request Jan 25, 2017
@bors-servo
Auto merge of #15153 - upsuper:pseudo-class-list, r=emilio
Loading status checks…
dc8fc44 
Put stylo's pseudo-class into a list file

r? @emilio

<!-- 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/15153)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jan 25, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-gnu-dev, windows-msvc-dev
@bors-servo bors-servo merged commit 4bb9821 into servo:master Jan 25, 2017
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio

Assignees

@emilio emilio

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@upsuper @highfive @emilio @bors-servo @Ms2ger @jdm @mbrubeck
You can’t perform that action at this time.