Repository navigation
Add Arabic (العربية) interface translation - #1532
hasan-sdaif wants to merge 7 commits into
Conversation
|
Thanks @hasan-sdaif, this is a big piece of work and the catalog is in good shape. I can't merge it yet, though: I built it and ran it, and the interface doesn't read correctly in Arabic. This is the gap #1202 describes, and the PR doesn't close it. It doesn't compile as submitted
What it looks like with that call removed (rebased on
What it needs
Smaller things
I'd suggest splitting this: first a small PR with the bidi pass and its test (the owner still has to agree to that approach in #1202), then this catalog on top, which can go in quickly once labels read correctly. A catalog that is registered but unreadable would be worse for Arabic users than none. If you'd like to talk it through, the team is on Discord: https://discord.gg/artcraft |
6ed5a7b to
ce52cdd
Compare
|
Thank you for building it — this is exactly the review I hoped for, and you are right on every point. I owe you an apology for the compile error especially: I wrote that call against the egui docs I had in my head instead of the version in the lockfile, and I can't build Rust on my machine, which is a poor excuse for shipping it. I've pushed
On the split you propose: agreed, and I'd go further — the catalog PR should not go in until labels actually read right-to-left, for the reason you give (a registered but unreadable Arabic is worse than none). If the owner signs off on the #1202 approach, I'm happy to take a first shot at the bidi pass with For the font question in #1202: from translating these catalogs, Noto Sans Arabic covers everything the strings use, but I have no experience with the presentation-forms route, so I can't speak to that trade-off. شكراً لك على وقتك في تجربتها فعلياً. |
ce52cdd to
dcd3b55
Compare
|
Thank you for this — the build report and the egui reading were exactly what I needed. The branch now carries the bidi pass you described, on top of a rebase onto current main:
One honest limitation: a label long enough to wrap still wraps from its visual end (egui wraps runs as it meets them), so the first line of a wrapped multi-line label holds its end. Menu and button labels don't wrap in practice. On splitting: the pass is one commit on its own, but its tests and the |
Messhias
left a comment
There was a problem hiding this comment.
This PR body does not show a completed cargo xtask ci run. Run it end to end (fmt, clippy, tests, assets, brands, layers, wasm), tick the checklist when green, and reply here with the outcome. That is required before any approve from here.
|
@hasan-sdaif — thanks for the bidi.rs pass and the CLDR plural rule; I've carried both into a branch that has the catalog from my side (hicbest/vectorcraft@arabic-fresh, ~4,300 rows with the status and error messages, complete_menus: true). If you'd like, I can fold it into this PR, or open it as a second PR that builds on yours. Whatever's easier for @bflatastic and the owner. On Discord too. |
|
@hicbest — thank you for carrying it forward, and honestly this is the best possible outcome for the PR: your catalog is the deeper one (~4,300 rows with the status and error messages, A few notes so the fold is mechanical, whichever shape the maintainers pick:
I'll leave the call between folding into this PR vs. a second PR on top of it to you, @bflatastic and the owner — I have no preference as long as Arabic users end up with one complete, correctly-ordered catalog. And I'll catch you on Discord. |
cf05da3 to
724cece
Compare
Fonts belong in storytold/craft-fonts (AGENTS.md), never in this repo. - Delete assets/fonts/NotoSansArabic-Regular.ttf - Delete assets/fonts/OFL-NotoSansArabic.txt - Drop their rows from ASSETS.md - theme.rs adds the Arabic UI faces through CRAFT_FONTS_DIR / craft-fonts only, mirroring the Japanese faces in storytold#1532 - Delete the two tests that assumed a bundled Noto Sans Arabic (arabic_glyphs_are_available_without_system_fonts, bundled_fonts_cover_the_catalogs)
|
Running it now, exactly as asked: the full family gate — |
|
The gate ran end to end. The outcome, in full:
That test and everything it sweeps live in The first run (this PR tree plus the workflow file only): https://github.com/hasan-sdaif/vectorcraft/actions/runs/38672678228 I will tick the checklist in the description once the main run reports, with the evidence links. And if the maintainers would like the upstream |
Messhias
left a comment
There was a problem hiding this comment.
Gate looks almost there (fmt/clippy/assets/brands/layers green; one engine expandAppearance ids test left). Confirm that failure on plain main (or fix it in a separate PR), tick cargo xtask ci when the catalog branch is fully green, and reply with the evidence links.
A complete ar catalog (3,783 rows) for the whole interface: every menu label, the canvas context menu, the macOS menu bar labels, every tl! literal and the status and error messages, with CLDR six-form plurals (zero/one/two/few/many/other) and the craft-fonts Arabic faces installed when present. Registers ar in LANGUAGES (every ar-* locale resolves to it), resolves like the other menu-complete catalogs, and keeps the product, workspace and perspective preset names in English (MENU_KEEP_AS_IS). The interface text itself still lays out left-to-right: egui 0.36 has neither bidi reordering nor a global direction switch, so mirroring the shell is left for the pass discussed in storytold#1202.
egui 0.36 lays a label's runs left to right and does not run the Unicode bidi algorithm (its text code still carries TODO(emilk): heed bidi characters), so an Arabic label read with its words in reverse order (storytold#1202): each word was shaped correctly, but it was drawn at the wrong end of the line. Languages flagged rtl now run each translation through a UAX storytold#9 pass when the catalog is built: the runs come out in visual order (unicode-bidi, already a dependency of crates/text), the letters inside each run stay in logical order for the shaper, and brackets are mirrored, since egui draws the glyph of the character it is handed. What a user types keeps its logical order — only catalog lookups go through the pass — and a headless layout test pins the order the labels come out in, so it cannot regress.
Check Script and its report (File › Scripts), Image Trace's Auto
Grouping, the Offset Path message, {path}: not a file, and Load EPS
into Clipboard, which had been missing all along. The Arabic catalog
note now describes the visual-order storage instead of a mirroring
that was never there.
UAX storytold#9 L2 reverses a right-to-left level's runs, and a `{placeholder}` sitting between Arabic words came out reversed (`}n{`) — the reordering was treating the label's own syntax as text, so the fill could no longer find it. Each placeholder now rides through the pass as one private-use character (restored afterwards, per placeholder, so a reordered sentence still finds its own), and curly brackets are left out of the mirror table for the same reason: BidiMirroring.txt pairs them, but in an interface label they are almost always a placeholder's own syntax. Found by replaying the whole catalog through a Python UAX storytold#9 replica of the pass.
The parse ran the bidi visual pass over a whole @plural row before splitting it on the pipes — but a pipe is a neutral character, so over the joined row the pass reordered the six forms themselves, and a count of one could read the many form (عنصراً instead of عنصر). The pass now runs per form, never across the row; the plural suite pins the order. Also the missing Symbol Options… row the menu-completeness test found (the dialog command label, with the trailing ellipsis the convention keeps).
724cece to
e93ce8a
Compare
The rebase onto current main brought the shortcut-set import dialog, the precise cursors preference, the 3D lighting rows and two save and export errors — 45 rows, Modern Standard Arabic like the rest of the catalog, placeholders and the «» quote convention kept, product names in English as always.
|
@Messhias — the evidence, and two corrections I owe you before it. Correction 1: my "1972 of 1973" report was wrong, and the run it came from hid why. That verification run stopped at the engine failure — Correction 2: the engine failure is not "pre-existing on main" — it's fixed on main. The plain-main verification run came back green (run 38676566364), and the reason is main's own 664e2ce — "keep command comparison fixtures independent of the clock" — which pins the very helper ( What the hidden failures were, and the fixes (both in the branch now):
The branch is rebased onto current main (b12a52f) and the full family gate is green on it: fmt, clippy, tests, assets, brands, layers, wasm — 6,599 tests, 0 failures — on the branch's tree plus only the workflow file that runs the gate: run 38739562408. The PR description is rewritten to match the branch as it now stands (the bidi pass it grew, the verification record, the checklist with The fold question with @hicbest's #1822 stays open for you and the owner — whichever shape wins, the bidi pass and the plurals here know how to carry that catalog. |
|
Thanks @hasan-sdaif — the notes make it easy to fold either way. I'll If we fold into #1532: I'll open a small PR against your branch with If we land #1822 first: you rebase #1532 onto it, and we keep the bidi Either way, one complete Arabic catalog with correct word order is the |
FredApps
left a comment
There was a problem hiding this comment.
ReviewerA — source-read COMMENT (not an approval)
I read the head of the diff: an Arabic interface translation. The catalogue header says the translations are original ("Clean-room: written from the plain meaning of the English text using ordinary Arabic vector-drawing vocabulary… no proprietary localisation resource was consulted"), a statement that satisfies the clean-room rule if true; the contributor should confirm it. The lock gains unicode-bidi 0.3.18 for the visual reordering, the same approach as vectorcraft#1963 and deckcraft#256, and vectorcraft#1822 adds another Arabic catalogue, so three Arabic PRs compete for this product and the maintainers must choose one and settle the shaping and bidi stack. A native check is needed. I read the catalogue header. I have not run it, and no exact-head CI result exists for this head. Nothing was run.
I ran nothing and have no exact-head CI result for this PR, so this is not an approval. Where a diff was long I read its code hunks rather than every generated or documentation line.
Status: waiting_author.
Arabic (العربية) for VectorCraft — the complete interface catalog, plus the right-to-left reading path that makes it actually readable.
The catalog
3,879 rows, Modern Standard Arabic throughout:
complete_menus: true, enforced by the completeness test againstmenu_strings());tl!literal in the crate — enforced by theevery_tl_literal_is_translatedtest;@msgrows and the{name}-style templates, values translated in turn;fewagain, 111 ismanyagain), each row tested over its counts;MENU_KEEP_AS_IS),«»quotes,{placeholders}whole, the trailing…of dialog commands kept.Every
ar-*locale (ar-SA,ar-EG,ar_AE.UTF-8…) resolves to it; the craft-fonts Arabic faces (Noto Sans Arabic) join the UI families when present, mirroring the CJK path.The reading path (bidi.rs)
egui 0.36 lays text runs left-to-right without a global direction switch, so a registered-but-unreadable Arabic would be worse than none. This PR carries the pass #1202 describes: Arabic translations are stored in the visual order egui draws, converted at catalog-parse time, with
{placeholders}kept whole and left-to-right islands (aPDFinside a sentence) keeping their own order. The layout test pins the combination on a real headless egui layout.The parse runs the pass per plural form, never across a
@pluralrow — a pipe is a neutral character, and over the joined row the pass would reorder the six forms themselves (the plural suite caught exactly that: a count of one reading the many form).Verification
All on GitHub Actions, on this fork, per the family's loop:
cargo xtask ci(fmt, clippy, tests, assets, brands, layers, wasm) — on the branch's tree plus only the workflow file that runs it: the green run (the checklist below ticks with it);expandAppearancefailure seen in an earlier verification run was that clock-straddle flake, fixed on main, absent since the rebase;Checklist
cargo xtask cigreen on the branch tree (fork-side run linked above)maincomplete_languages_translate_every_menu_stringgreenevery_tl_literal_is_translatedgreenCoordination
@hicbest's #1822 offers the deeper catalog (~4,300 rows with the status and error messages) on top of this branch's code — the fold notes are on my comment there; whichever shape the maintainers prefer, nothing is done twice.
شكراً لمراجعتكم — أتمنى أن تكون هذه الإضافة مفيدة لمستخدمي العربية.