applib: mirror paired brackets when reversing RTL text#1631
Conversation
Characters with the Bidi_Mirrored property -- parentheses, square and curly brackets, angle brackets and guillemets -- must swap to their mirror when an RTL run is reversed, so a bracket keeps opening toward the text it encloses. Without this, a parenthesised Arabic phrase rendered with the brackets pointing the wrong way. Map each codepoint through a small mirror table where utf8_reverse_for_rtl emits it. The weak-LTR digit run is left untouched; digits and numeric separators are not mirrored. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Khalid Nuaim <i@kalua.im>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7403874f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| size_t bytes_written = utf8_encode_codepoint(cp, dest + dest_offset); | ||
| size_t bytes_written = utf8_encode_codepoint(prv_mirror_codepoint(cp), dest + dest_offset); |
There was a problem hiding this comment.
Avoid mirroring delimiters split into LTR segments
For mixed LTR text such as abc (שלום), the segmenter leaves the opening ( in the preceding LTR segment and starts the RTL segment at the Hebrew text, because it breaks when the strong direction changes while punctuation remains with the current segment (src/fw/applib/graphics/text_layout.c:750-755). This line then unconditionally mirrors the trailing ) from the RTL segment to ( after reversal, so the rendered sequence becomes two opening parentheses and no closing one (e.g. abc ((םולש). Please only mirror paired punctuation when both sides are handled in the same RTL run, or adjust neutral delimiter segmentation so the pair stays together.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'll defer to your judgement here, also pls @jplexer check

What
Characters with the Unicode
Bidi_Mirroredproperty —(),[],{},<>,«»,‹›— must swap to their mirror when an RTL run is reversed, so a bracket keeps opening toward the text it encloses.utf8_reverse_for_rtl()reversed the codepoint order of an RTL run but emitted each bracket unchanged, so a parenthesized Arabic phrase like(مرحبا عربي)came out as)مرحبا عربي(— the parentheses pointing the wrong way.Map each codepoint through a small mirror table where the reversal emits it. The weak-LTR digit run is left untouched (digits and numeric separators are not mirrored).
Before / after
Notification body
(مرحبا عربي). Before: the parentheses point outward; after: they wrap the phrase.Test
brackets_mirror: a balanced(نص)stays wrapped ((ص ن)), and a lone(before a letter comes out mirrored as)../pbl testpasses.