Implement L1: Adjust levels of whitespace characters #2
Comments
This was referenced May 5, 2017
bors-servo
pushed a commit
that referenced
this issue
May 15, 2017
To be able to implement L1, we need access to more information from `BidiInfo`, namely `original_classes` of the `text`, in `visual_runs()`, which would mean it should pass through `reorder_line()`. The fact that information from `BidiInfo` is needed for both steps of the public API (generating `BidiInfo` and consuming it per-paragraph/per-level) made me change the API design and move these methods into `impl BidiInfo`. Then, since we needed access to `text` for every `BidiInfo` consumption, I added a reference to `text` to `BidiInfo`, which also enables more compile-time checks for `BidiInfo` isntance not outliving the text in the user code. NOTE: We are already breaking API in version 0.3.0 and paving for full spec support is a good reason to do so, IMHO. The L1 rule works by one pass on the text of the line. Conformance Test: this implementation reduces the number of failures from 60494 to 23770 (out of total 256747 cases). Fix #2
bors-servo
pushed a commit
that referenced
this issue
May 15, 2017
Add Conformance Test and implement L1 rule Initial results: `60494 test cases failed! (196253 passed)` Because of the current limitations of rust's test framework, and the huge number of failures, the base (not including matching pairs) conformance test is executed in one run, and a summary is panic'ed (if there's any failure) and for the integration test to pass, it's marked as `should_panic`, with summary of the test run as `expected` string. Fix #1 To be able to implement L1, we need access to more information from `BidiInfo`, namely `original_classes` of the `text`, in `visual_runs()`, which would mean it should pass through `reorder_line()`. The fact that information from `BidiInfo` is needed for both steps of the public API (generating `BidiInfo` and consuming it per-paragraph/per-level) made me change the API design and move these methods into `impl BidiInfo`. Then, since we needed access to `text` for every `BidiInfo` consumption, I added a reference to `text` to `BidiInfo`, which also enables more compile-time checks for `BidiInfo` isntance not outliving the text in the user code. NOTE: We are already breaking API in version 0.3.0 and paving for full spec support is a good reason to do so, IMHO. The L1 rule works by one pass on the text of the line. Conformance Test: this implementation reduces the number of failures from 60494 to 23770 (out of total 256747 cases). Fix #2 <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/unicode-bidi/30) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The
visual_runsfunction does not yet implement step L1, which defines steps for adjusting the levels of certain whitespace and formatting characters.In Gecko this is performed in
nsBidi::AdjustWSLevelsandnsBidi::SetTrailingWSStart.This may also be a good place to adjust levels for
removed_by_x9characters, as innsBidi::AdjustWSLevels.The text was updated successfully, but these errors were encountered: