-
-
Notifications
You must be signed in to change notification settings - Fork 112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(developer): rewrite ldml visual keyboard compiler 🍒 🏠 #12406
fix(developer): rewrite ldml visual keyboard compiler 🍒 🏠 #12406
Conversation
The visual keyboard compiler was never finished in 17.0. This rewrites it to: 1. Use the kmxplus data rather than reading from xml directly 2. Fill in `visualkeyboard.header.kbdname` 3. Support modifiers 4. Handle encoded characters like `\u{1234}` 5. Handle string variables like `${one}`* Additional unit tests have been added to verify the behavior of the visual keyboard compiler in more detail. * String variable tests will be enabled in next commit (which is a cherry-pick of #12404). Other fixes: 1. The LDML XML reader was relying on its input being a Node.js `Buffer` even though it was declared `Uint8Array`, as it implicitly used `Buffer.toString()` to do text conversion. (`Buffer` subclasses from `Uint8Array`). This breaks when using `Uint8Array` directly and means we had an implicit dependency on Node.js. See also #12331. 2. XML errors were not captured in the LDML XML reader. See also #12331. 3. The unused and unfinished touch-layout-compiler.ts and keymanweb-compiler.ts have been removed along with corresponding unit tests and fixtures. These will be replaced by Core implementations; see #12291. Fixes: #12395 Cherry-pick-of: #12402
User Test ResultsTest specification and instructions
Test Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RSLGTM
@dinakaranr thank you for that test. I have addressed an issue in the compiler, so can you please retest on the Mac? Make sure you rebuild the keyboard before copying to the Mac for testing. Thanks! @keymanapp-test-bot retest TEST_LDML_VISUAL_KEYBOARD_MAC |
Changes in this pull request will be available for download in Keyman version 17.0.330 |
The visual keyboard compiler was never finished in 17.0. This rewrites it to:
visualkeyboard.header.kbdname
\u{1234}
${one}
*Additional unit tests have been added to verify the behavior of the visual keyboard compiler in more detail.
Other fixes:
Buffer
even though it was declaredUint8Array
, as it implicitly usedBuffer.toString()
to do text conversion. (Buffer
subclasses fromUint8Array
). This breaks when usingUint8Array
directly and means we had an implicit dependency on Node.js. See also chore(common): xml2js -> fast-xml-parser #12331.Fixes: #12395
Cherry-pick-of: #12402
User Testing
Preparation:
TEST_LDML_VISUAL_KEYBOARD_WINDOWS: Install the imperial_aramaic.kmp file into Keyman for Windows 17.0.329. Verify that the On Screen Keyboard displays Aramaic characters.
TEST_LDML_VISUAL_KEYBOARD_MAC: Install the imperial_aramaic.kmp file into Keyman for macOS 17.0.329. Verify that the On Screen Keyboard displays Aramaic characters.
TEST_LDML_VISUAL_KEYBOARD_LINUX: Install the imperial_aramaic.kmp file into Keyman for Linux 17.0.329. Verify that the On Screen Keyboard displays Aramaic characters.