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

Migrated -Z trace-layout to serde_json #13740

Merged
merged 1 commit into from Nov 7, 2016
Merged

Conversation

@shinglyu
Copy link
Member

shinglyu commented Oct 13, 2016

Migrated the trace-layout code from old rustc-serialize to serde_json. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #12936 (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because it's a relatively low risk debug tool

This change is Reviewable

@highfive
Copy link

highfive commented Oct 13, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/logical_geometry.rs
@highfive
Copy link

highfive commented Oct 13, 2016

warning Warning warning

  • These commits modify style and layout code, but no tests are modified. Please consider adding a test!
script_layout_interface = {path = "../script_layout_interface"}
script_traits = {path = "../script_traits"}
selectors = "0.13"
serde = "0.8"
serde_json = "0.8"

This comment has been minimized.

Copy link
@nox

nox Oct 14, 2016

Member

serde_derive should come before serde_json.

@bors-servo
Copy link
Contributor

bors-servo commented Oct 15, 2016

The latest upstream changes (presumably #13756) made this pull request unmergeable. Please resolve the merge conflicts.

@shinglyu shinglyu force-pushed the shinglyu:layout_serde branch from c72fb44 to a488312 Oct 17, 2016
@shinglyu shinglyu mentioned this pull request Oct 19, 2016
3 of 5 tasks complete
@shinglyu shinglyu force-pushed the shinglyu:layout_serde branch from a488312 to b29cb8a Oct 20, 2016
@shinglyu
Copy link
Member Author

shinglyu commented Oct 20, 2016

Just rebased to the latest master, @nox r?

@bors-servo
Copy link
Contributor

bors-servo commented Oct 27, 2016

The latest upstream changes (presumably #13870) made this pull request unmergeable. Please resolve the merge conflicts.

@shinglyu shinglyu force-pushed the shinglyu:layout_serde branch from b29cb8a to 756f87c Oct 28, 2016
@shinglyu
Copy link
Member Author

shinglyu commented Oct 28, 2016

FlowClass::TableRowGroup => to_value(f.as_table_rowgroup()),
FlowClass::TableRow => to_value(f.as_table_row()),
FlowClass::TableCell => to_value(f.as_table_cell()),
_ => { Value::Null } // TODO: Support captions

This comment has been minimized.

Copy link
@jdm

jdm Oct 28, 2016

Member

We should probably use explicit variants here, to avoid incorrect defaults.

})
impl Serialize for Fragment {
fn serialize<S: Serializer>(&self, serializer: &mut S) -> Result<(), S::Error>{
let mut state = try!(serializer.serialize_struct("fragment", 3)); //change the length!

This comment has been minimized.

Copy link
@jdm

jdm Oct 28, 2016

Member

I'm not sure what these comments mean.

@@ -10,6 +10,9 @@
#![feature(plugin)]
#![feature(raw)]
#![feature(step_by)]
#![feature(proc_macro)]

This comment has been minimized.

Copy link
@jdm

jdm Oct 28, 2016

Member

Move these in alphabetical order.

fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
self.block_flow.encode(e)
impl Serialize for TableRowGroupFlow {
fn serialize<S: Serializer>(&self, serializer: &mut S) -> Result<(), S::Error>{

This comment has been minimized.

Copy link
@jdm

jdm Oct 28, 2016

Member

nit: space before {

fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
self.bits().encode(e)
impl Serialize for BlockFlowFlags {
fn serialize<S: Serializer>(&self, serializer: &mut S) -> Result<(), S::Error>{

This comment has been minimized.

Copy link
@jdm

jdm Oct 28, 2016

Member

nit: space before {

@jdm jdm assigned jdm and unassigned asajeffrey Oct 28, 2016
@shinglyu shinglyu force-pushed the shinglyu:layout_serde branch from 756f87c to d318a2b Oct 28, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Oct 28, 2016

The latest upstream changes (presumably #13960) made this pull request unmergeable. Please resolve the merge conflicts.

#![feature(raw)]
#![feature(step_by)]
#![feature(structural_match)]
#![feature(rustc_attrs)]

This comment has been minimized.

Copy link
@jdm

jdm Oct 31, 2016

Member

What's this feature for?

This comment has been minimized.

Copy link
@shinglyu

shinglyu Nov 1, 2016

Author Member

These are some features required by serde_json, but looks like they are no longer needed in rustc-1.14.0, I'll try removing them and see if it works.

This comment has been minimized.

Copy link
@shinglyu

This comment has been minimized.

Copy link
@shinglyu

shinglyu Nov 1, 2016

Author Member

Oh we don't need that anymore, I can compile with rustc-1.14.0 with them removed.

fn encode<S: Encoder>(&self, e: &mut S) -> Result<(), S::Error> {
self.block_flow.encode(e)
impl Serialize for TableRowFlow {
fn serialize<S: Serializer>(&self, serializer: &mut S) -> Result<(), S::Error>{

This comment has been minimized.

Copy link
@jdm

jdm Oct 31, 2016

Member

nit: space before { (there are a bunch of other instances in this PR; search for >{)

@jdm
Copy link
Member

jdm commented Nov 4, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 4, 2016

Previous build results for arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-css, mac-rel-wpt1, windows-dev are reusable. Rebuilding only linux-rel-wpt, mac-rel-wpt2...

@bors-servo
Copy link
Contributor

bors-servo commented Nov 4, 2016

💔 Test failed - mac-rel-wpt2

@highfive
Copy link

highfive commented Nov 4, 2016

  ▶ TIMEOUT [expected CRASH] /_mozilla/mozilla/sigsegv.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  └ 3.3 (Core Profile) Mesa 12.0.1
@bors-servo
Copy link
Contributor

bors-servo commented Nov 4, 2016

The latest upstream changes (presumably #14053) made this pull request unmergeable. Please resolve the merge conflicts.

@shinglyu shinglyu force-pushed the shinglyu:layout_serde branch from 10754ed to 8bea421 Nov 7, 2016
@shinglyu
Copy link
Member Author

shinglyu commented Nov 7, 2016

@bors-servo: retry, r=jdm

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2016

📌 Commit 8bea421 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2016

Testing commit 8bea421 with merge 1c26f44...

bors-servo added a commit that referenced this pull request Nov 7, 2016
Migrated -Z trace-layout to serde_json

<!-- Please describe your changes on the following line: -->

Migrated the trace-layout code from old `rustc-serialize` to `serde_json`. This will help us iterate faster on the layout viewer (#13432), #13436, #12675 and fix #12936.
---

<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #12936 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because it's a relatively low risk debug tool

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/13740)

<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 7, 2016

@bors-servo bors-servo merged commit 8bea421 into servo:master Nov 7, 2016
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Nov 7, 2016
0 of 5 tasks complete
bors-servo added a commit that referenced this pull request Dec 28, 2016
Flexbox trace

<!-- Please describe your changes on the following line: -->
This is a follow up for #13740, so r? @jdm

The first patch enables JSON serialization for flexbox flows, the second one fixed format incompatibilities for the layout viewer.

The 3rd and 4th patches are just layout viewer UI enhancements, we could split that to a spearate PR if you prefer.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #13846  (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because it's a trivial debug tool

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/14124)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.