Skip to content

applib: mirror paired brackets when reversing RTL text#1631

Open
kaluaim wants to merge 1 commit into
coredevices:mainfrom
kaluaim:arabic-bracket-mirror
Open

applib: mirror paired brackets when reversing RTL text#1631
kaluaim wants to merge 1 commit into
coredevices:mainfrom
kaluaim:arabic-bracket-mirror

Conversation

@kaluaim

@kaluaim kaluaim commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

What

Characters with the Unicode Bidi_Mirrored property — (), [], {}, <>, «», ‹› — 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.

cmp_bracket

Test

brackets_mirror: a balanced (نص) stays wrapped (( ص ن )), and a lone ( before a letter comes out mirrored as ). ./pbl test passes.

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>
@kaluaim
kaluaim requested review from gmarull and jplexer as code owners June 30, 2026 09:18

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gmarull Codex is right — this only fixes pure-RTL lines; mixed Arabic+Latin needs real bidi reordering, not just mirroring. I wrote a small UAX#9 bidi engine that fixes all cases — diff. Want it as a PR?

cmp_bidi

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll defer to your judgement here, also pls @jplexer check

@kaluaim
kaluaim marked this pull request as draft June 30, 2026 10:00
@kaluaim
kaluaim marked this pull request as ready for review June 30, 2026 14:41
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.

2 participants