bug(developer): generation of KVK from LDML keyboard seems to be creating an invalid .kvk file #12395
Comments
|
@srl295 @LornaSIL @DavidLRowe FYI. The visual keyboard compiler in kmc-ldml has multiple gaps, so will work on fixing those. |
mcdurdin
added a commit
that referenced
this issue
Sep 11, 2024
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.
TODO-LDML: string variables appear to have a secondary bug -- they seem
to be returning the string 'undefined'. I have disabled the related
tests and will examine this separately, and enable those tests once
fixed.
TODO-LDML: we should probably add a compiler warning + unit test for
`<layers formId="us"><layer id="base">`, because this pattern does not
make sense: when using non-touch forms, the `<layer>` element should use
`modifiers` attribute, and correspondingly, `modifiers` attribute should
_not_ be used when `formId` is `touch`.
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 are replaced by Core implementations; see
#12291.
Fixes: #12395
mcdurdin
added a commit
that referenced
this issue
Sep 12, 2024
Validation of vars was not properly checking for forward references to string variables. This, coupled with a null vs undefined bug in subsequent use, meant that forward reference variables were ending up with a literal string value of 'undefined'. This also fixes the test for visual-keyboard-compiler, where the fixture was actually buggy and was the trigger for investigating this problem. Fixes: #12403 Relates-to: #12395
mcdurdin
added a commit
that referenced
this issue
Sep 12, 2024
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
mcdurdin
added a commit
that referenced
this issue
Sep 12, 2024
Validation of vars was not properly checking for forward references to string variables. This, coupled with a null vs undefined bug in subsequent use, meant that forward reference variables were ending up with a literal string value of 'undefined'. This also fixes the test for visual-keyboard-compiler, where the fixture was actually buggy and was the trigger for investigating this problem. Fixes: #12403 Relates-to: #12395
mcdurdin
added a commit
that referenced
this issue
Sep 16, 2024
…iling visual keyboard Fixes: #12395
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From: keymanapp/keyboards#2993
Continuing: #12278, #12284 (fixing #12056)
The imperial_aramaic.kvk file generated during the build has multiple issues:
visualkeyboard.header.kbdnamefield is blank, but needs to beimperial_aramaic; seekeyman/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts
Line 34 in 6353f92
layerelements grouped byshiftattribute); seekeyman/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts
Line 100 in 6353f92
\u{....}instead of being raw Unicode text, e.g.\u{0020}, anddisplaysare missing; seekeyman/developer/src/kmc-ldml/src/compiler/visual-keyboard-compiler.ts
Line 88 in 6353f92
The text was updated successfully, but these errors were encountered: