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

Use the current parser location for CSS error #18585

Closed
wants to merge 9 commits into from
Closed

Use the current parser location for CSS error #18585

wants to merge 9 commits into from
+1,149 −929

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Sep 20, 2017 •
edited

… rather than the start location of the current construct. This likely places the error just after of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861

This change is Reviewable
@SimonSapin
Copy link
Member Author

SimonSapin commented Sep 20, 2017

Do not merge yet, waiting on Gecko try results to have a patch ready to update tests.

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Sep 20, 2017

⌛ Trying commit 123cfde with merge c8be42c...
bors-servo added a commit that referenced this pull request Sep 20, 2017
@bors-servo
Auto merge of #18585 - servo:error-location, r=<try>
Loading status checks…
c8be42c 
Use the current parser location for CSS error

… rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861
@bors-servo
Copy link
Contributor

bors-servo commented Sep 21, 2017

💔 Test failed - windows-msvc-dev
@SimonSapin SimonSapin force-pushed the error-location branch from 123cfde to 4c7144e Sep 21, 2017
@SimonSapin
Copy link
Member Author

SimonSapin commented Sep 21, 2017

@bors-servo try
1 similar comment
@SimonSapin
Copy link
Member Author

SimonSapin commented Sep 22, 2017

@bors-servo try
@SimonSapin SimonSapin force-pushed the error-location branch from 4c7144e to 09a77b5 Sep 22, 2017
bors-servo added a commit that referenced this pull request Sep 22, 2017
@bors-servo
Auto merge of #18585 - servo:error-location, r=
Loading status checks…
88cc4b8 
Use the current parser location for CSS error

… rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861

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

bors-servo commented Sep 22, 2017

⌛ Testing commit 09a77b5 with merge 88cc4b8...
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2017

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=False
@SimonSapin SimonSapin force-pushed the error-location branch from 09a77b5 to f7ff9e9 Oct 5, 2017
@SimonSapin SimonSapin force-pushed the error-location branch from f7ff9e9 to f0a4441 Oct 5, 2017
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 5, 2017

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Oct 5, 2017

⌛ Trying commit f0a4441 with merge eb3d40c...
bors-servo added a commit that referenced this pull request Oct 5, 2017
@bors-servo
Auto merge of #18585 - servo:error-location, r=<try>
Loading status checks…
eb3d40c 
Use the current parser location for CSS error

… rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861

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

bors-servo commented Oct 5, 2017

💔 Test failed - linux-rel-css
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 5, 2017

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Oct 5, 2017

⌛ Trying commit 3292c95 with merge 5ad02ca...
bors-servo added a commit that referenced this pull request Oct 5, 2017
@bors-servo
Auto merge of #18585 - servo:error-location, r=<try>
Loading status checks…
5ad02ca 
Use the current parser location for CSS error

… rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861

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

bors-servo commented Oct 5, 2017

💔 Test failed - linux-dev
@jdm
Copy link
Member

jdm commented Oct 10, 2017

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

bors-servo commented Oct 10, 2017

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

bors-servo commented Oct 10, 2017

⚡ Previous build results for android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt4, windows-msvc-dev are reusable. Rebuilding only mac-rel-wpt3...
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: r-
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: r=emilio clean
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: r-
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: ping
@bors-servo
Copy link
Contributor

bors-servo commented Oct 10, 2017

😪 I'm awake I'm awake
@SimonSapin
CSS parsing error types: flatten nested enums more still
Loading status checks…
c36ac69
@SimonSapin SimonSapin force-pushed the error-location branch from fd82855 to c36ac69 Oct 10, 2017
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 10, 2017

Pushed another dummy amend to hopefully get homu unstuck.

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

bors-servo commented Oct 10, 2017

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

bors-servo commented Oct 10, 2017

⌛ Testing commit c36ac69 with merge ecdafef...
bors-servo added a commit that referenced this pull request Oct 10, 2017
@bors-servo
Auto merge of #18585 - servo:error-location, r=emilio
Loading status checks…
ecdafef 
Use the current parser location for CSS error

… rather than the start location of the current construct. This likely places the error just *after* of the unexpected token whereas before would be best, but that’s likely a much bigger change.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1378861

<!-- 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/18585)
<!-- Reviewable:end -->
@SimonSapin SimonSapin closed this Oct 10, 2017
@SimonSapin SimonSapin reopened this Oct 10, 2017
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: r-
@jdm
Copy link
Member

jdm commented Oct 10, 2017

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

bors-servo commented Oct 10, 2017

📌 Commit c36ac69 has been approved by emilio
@SimonSapin SimonSapin mentioned this pull request Oct 10, 2017
Merged
@jdm
Copy link
Member

jdm commented Oct 10, 2017

@bors-servo: r-
@SimonSapin
Copy link
Member Author

SimonSapin commented Oct 10, 2017

Moved to #18808.
@SimonSapin SimonSapin closed this Oct 10, 2017
@SimonSapin SimonSapin deleted the error-location branch Oct 10, 2017
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
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@SimonSapin @bors-servo @jdm @emilio @highfive
You can’t perform that action at this time.