Skip to content

Take the reorder_line fast path only in LTR paragraphs - #156

Open
youdie006 wants to merge 1 commit into
servo:mainfrom
youdie006:reorder-line-rtl-paragraph
Open

youdie006 wants to merge 1 commit into
servo:mainfrom
youdie006:reorder-line-rtl-paragraph

Conversation

@youdie006

Copy link
Copy Markdown
Contributor

reorder_line returns the line unchanged when !level::has_rtl(&self.levels[line]) (src/lib.rs:595, :828; src/utf16.rs:272, :505). Those are the levels before rule L1. In an RTL paragraph, L1 resets segment separators and trailing whitespace to the paragraph level (1), so a line of LTR text can still need reordering:

text, base RTL line master expected
"a\tb" 0..3 "a\tb" "b\ta"
"a b" 0..2 "a " " a"

BidiTest.txt has the same cases (L S L; 4 → @Reorder: 2 1 0, L WS; 4 → @Reorder: 1 0); the conformance tests check reordered_levels/reorder_visual, not reorder_line, so they don't hit this path.

This keeps the fast path, but only for LTR paragraphs, where L1 resets to level 0 and cannot create an odd level. The fast path came in with 181cd1d; the shortcut before it looked at the post-L1 levels.

The new rows at the end of test_reorder_line check all four reorder_line implementations and fail on main. cargo test (also --features serde, --no-default-features, --no-default-features --features=hardcoded-data), cargo fmt --check, and cargo test on 1.47.0 pass.

Written with AI assistance (Claude); I have reviewed the change.

The fast path checks the levels before rule L1. In an RTL paragraph, L1
resets segment separators and trailing whitespace to level 1, so a line
with no odd level can still need reordering, and it came back in
logical order. In an LTR paragraph L1 only resets to level 0, so the
fast path stays correct there.
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.

1 participant