feat(developer): ldml scan codes support 🙀 #9615

srl295 · 2023-09-22T22:58:14Z

pivot: this now dynamically generates the vkey map based on CLDR scan codes
add infrastructure to detect imported items
warn (hint) if user adds a custom scancode
also includes an LDML resource update - including removal of the vkeys, see feat(core): ldml vkey support 🙀 #7135
also minor, use source maps when launching kmc from tests.

For: #9403 - unicode-org/cldr:f16d66601f309f29a64a897282fdcab440d42661 - No local modifications neded1

For: #9403 - read implied imports for scancodes

For: #9403 - use Symbols to track import status

For: #9403 - improve how forms are read in the XML structure - warnings when custom (non-default-import) scancodes are loaded - error, as usual, when an unknown form is present

For: #9403 - unit test the set and row count for codes - fix errors!

keymanapp-test-bot · 2023-09-22T22:58:18Z

User Test Results

Test specification and instructions

User tests are not required

Test Artifacts

Android
Developer
iOS
- Keyman for iOS (simulator image)
- FirstVoices Keyboards for iOS (simulator image)
- TestFlight internal PR build version - 17.0.184 (0.9615.9309)
Keyboards
- Test Keyboards
Linux
- Keyman for Linux
macOS
- Keyman for macOS
Web
- KeymanWeb Test Home
Windows

mcdurdin

Not quite ready to approve this, sorry!

common/web/types/src/consts/virtual-key-constants.ts

mcdurdin · 2023-09-25T02:37:31Z

common/web/types/src/ldml-keyboard/ldml-keyboard-xml.ts

+  static impliedImport = Symbol.for('@keymanapp:implied_import');
+  /** item came in via import */
+  static import = Symbol.for('@keymanapp:import');


This introduces a new namespace that we need to be careful with -- the @keymanapp Symbol namespace. Conceptually I think it is fine but I am just a little wary about future trouble with it. Might be good to use a longer prefix, for disambiguation e.g.:

Suggested change

static impliedImport = Symbol.for('@keymanapp:implied_import');

/** item came in via import */

static import = Symbol.for('@keymanapp:import');

static impliedImport = Symbol.for('@keymanapp:common/ldml/implied_import');

/** item came in via import */

static import = Symbol.for('@keymanapp:common/ldml/import');

Also, just verifying is this just a way of tagging arbitrary structures with an "imported" flag? And you wanted to use this to avoid potential stumbling blocks in the way we use those structures, because it won't be an interable key on an object?

On reflection, I think this doesn't need a named symbol at all. this symbol should be used only within one run of kmc. it doesn't need to be independently detected. I will drop the namespace.

yes, it's just a way to tag the structure as imported - it's not iterable on the object, it doesn't affect the API at all at any level. It's metadata answering the question "how did this data object come in?"

developer/src/kmc-ldml/src/compiler/messages.ts

developer/src/kmc-ldml/test/fixtures/sections/layr/hint-custom-form.xml

For: #9403 - Just use a local Symbol() instead of a named one, there's no reason for a namespace here.

For: #9403 - fix a bad comment - Warn_UnsupportedCustomForm (formerly hint)

resources/standards-data/ldml-keyboards/techpreview/import/scanCodes-implied.xml

For: #9403 - per review comments - track CLDR PR 3282

srl295 · 2023-09-25T22:08:50Z

@mcdurdin ptal

- use vkey k.K_oE2 for 0x7D in KS and JIS keyboards For: #9403

srl295 · 2023-09-28T18:42:31Z

i'm temporarily skipping the scancode test— to show that the LDML updates are otherwise passing.

- at least make sure we're using a consistent (if wrong) vkey for each scancode? for: feat(developer): ldml scancodes 🙀 #9403

- emit scancodes.json for: feat(developer): ldml scancodes 🙀 #9403

srl295 · 2023-09-29T00:03:30Z

temporary file output => https://gist.github.com/srl295/48685c96bb23ee19ce1f14428ed5d91c

 {
  "scan": "0A",
  "vname": "K_9",
  "vcode": 57
 },
 {
  "scan": "0B",
  "vname": "K_0",
  "vcode": 48
 },
 {
  "scan": "0C",
  "vname": "K_HYPHEN",
  "vcode": 189
 },

mcdurdin · 2023-09-29T02:32:12Z

That works, a bit verbose, but certainly easy to work with. It will be a separate generated file?

Evil alternative:

const scancodes = [
  ["0A","K_9",57],
  ["0B","K_0",48],
  ["0C","K_HYPHEN",189]
].map(x => ({scan:x[0], vname:x[1], vcode:x[2]}));

- interim step - also re-add statically For: #9403

… the same 🙀 For: #9403

- also fix visual keyboard compiler to use same code For: #9403

srl295 · 2023-10-01T00:04:55Z

@mcdurdin PTAL at progress but TODO:

unit test: verify that scan code map is 1:1
kmc: error if CLDR scancode results in no vkey

Is there any other checking needed?

srl295 · 2023-10-01T00:22:43Z

  0x56: k.K_oE2, // << Dup
  0x73: k.k_oC1,
  0x7D: k.K_oE2, // << Dup

except we already have dups.

- spec: 'hardware' is no longer an enum but a str. Ripple effects here. - custom scancodes and layouts are, interestingly, supported. But it's a warning. - tests for bad scancodes. Fixes: #9403

srl295 · 2023-10-01T03:08:38Z

Okay, PTAL.

see if the scancode map needs updating. But it's in one place! It was already not 1:1

mcdurdin · 2023-10-02T01:30:25Z

  0x56: k.K_oE2, // << Dup
  0x73: k.k_oC1,
  0x7D: k.K_oE2, // << Dup

except we already have dups.

This would only be a problem if both 0x56 and 0x7D scan codes appear on the same hardware keyboard, which they do not: 0x56 is on ISO and ABNT2, and 0x7D is on JIS.

mcdurdin

LGTM

common/web/types/src/consts/virtual-key-constants.ts

developer/src/kmc-ldml/test/fixtures/basic.txt

Co-authored-by: Marc Durdin <[email protected]>

srl295 · 2023-10-02T17:22:58Z

  0x56: k.K_oE2, // << Dup
  0x73: k.k_oC1,
  0x7D: k.K_oE2, // << Dup
except we already have dups.
This would only be a problem if both 0x56 and 0x7D scan codes appear on the same hardware keyboard, which they do not: 0x56 is on ISO and ABNT2, and 0x7D is on JIS.

Maybe I should exclude this specific dup.

- fix casing of k_oC1 - allow vkey 226 to be doubly defined - drop LdmlVkeyNames table and test, not needed anymore (was for vkey #7135) Fixes: #9403

mcdurdin · 2023-10-02T22:36:41Z

Maybe I should exclude this specific dup.

Sounds sensible -- then we can track if future dups appear.

common/web/types/test/ldml-keyboard/test-ldml-keyboard-xml-reader.ts

- fixes to the special case test Fixes: #9403

mcdurdin

LGTM.

keyman-server · 2023-10-03T18:02:26Z

Changes in this pull request will be available for download in Keyman version 17.0.185-alpha

srl295 added 5 commits September 22, 2023 15:07

chore(resources): ldml update keyboard -> keyboard3 🙀

5ddc8c8

For: #9403 - unicode-org/cldr:f16d66601f309f29a64a897282fdcab440d42661 - No local modifications neded1

feat(developer): ldml scancodes 🙀

6a54ebd

For: #9403 - read implied imports for scancodes

feat(common): ldml scancodes 🙀

3a677da

For: #9403 - use Symbols to track import status

feat(common): ldml scancodes 🙀

44f061f

For: #9403 - improve how forms are read in the XML structure - warnings when custom (non-default-import) scancodes are loaded - error, as usual, when an unknown form is present

feat(common): ldml scancodes 🙀

f56e4ff

For: #9403 - unit test the set and row count for codes - fix errors!

srl295 requested a review from mcdurdin as a code owner September 22, 2023 22:58

srl295 self-assigned this Sep 22, 2023

srl295 requested review from jahorton, darcywong00 and rc-swag as code owners September 22, 2023 22:58

keymanapp-test-bot bot added this to the A17S22 milestone Sep 22, 2023

github-actions bot added developer/ common/ developer/compilers/ common/resources/ Build infrastructure core/ Keyman Core common/web/ epic-ldml feat labels Sep 22, 2023

mcdurdin requested changes Sep 25, 2023

View reviewed changes

srl295 added 2 commits September 25, 2023 15:58

feat(common): ldml import infrastructure 🙀

2726604

For: #9403 - Just use a local Symbol() instead of a named one, there's no reason for a namespace here.

feat(developer): scancode hint to warning 🙀

87c18fd

For: #9403 - fix a bad comment - Warn_UnsupportedCustomForm (formerly hint)

srl295 mentioned this pull request Sep 25, 2023

CLDR-16604 kbd: jis scancodes incorrect unicode-org/cldr#3282

Merged

1 task

srl295 commented Sep 25, 2023

View reviewed changes

resources/standards-data/ldml-keyboards/techpreview/import/scanCodes-implied.xml Outdated Show resolved Hide resolved

feat(common): scancode fixes for ks and iso 🙀

71aee7a

For: #9403 - per review comments - track CLDR PR 3282

srl295 requested a review from mcdurdin September 25, 2023 22:08

srl295 added 2 commits September 25, 2023 17:08

Merge branch 'master' into feat/developer/9403-scancodes-epic-ldml

8774651

feat(common): scancode fixes for ks and jis 🙀

2c588ed

- use vkey k.K_oE2 for 0x7D in KS and JIS keyboards For: #9403

srl295 added 2 commits September 28, 2023 17:16

feat(developer): ldml dynamic scancodes 🙀

6e2d858

- at least make sure we're using a consistent (if wrong) vkey for each scancode? for: feat(developer): ldml scancodes 🙀 #9403

feat(developer): ldml dynamic scancodes 🙀

968da50

- emit scancodes.json for: feat(developer): ldml scancodes 🙀 #9403

mcdurdin modified the milestones: A17S22, A17S23 Sep 30, 2023

srl295 added 4 commits September 30, 2023 17:09

feat(common): scancode - generate scancode.ts 🙀

b9f9d9c

- interim step - also re-add statically For: #9403

feat(common): scancode - confirming that the calculated scancodes are…

4b9d7cb

… the same 🙀 For: #9403

chore(core): use sourcemaps when calling kmc for tests

64eaf39

feat(common): scancode - move vkey map generation into KeysCompiler 🙀

ffe00c0

- also fix visual keyboard compiler to use same code For: #9403

srl295 requested a review from mcdurdin October 1, 2023 00:02

Merge branch 'master' into feat/developer/9403-scancodes-epic-ldml

f5ceac8

feat(developer): scancode and forms 🙀

17eba2d

- spec: 'hardware' is no longer an enum but a str. Ripple effects here. - custom scancodes and layouts are, interestingly, supported. But it's a warning. - tests for bad scancodes. Fixes: #9403

mcdurdin approved these changes Oct 2, 2023

View reviewed changes

common/web/types/src/consts/virtual-key-constants.ts Outdated Show resolved Hide resolved

developer/src/kmc-ldml/test/fixtures/basic.txt Outdated Show resolved Hide resolved

srl295 and others added 2 commits October 2, 2023 09:23

Actualizar virtual-key-constants.ts

1b85166

Co-authored-by: Marc Durdin <[email protected]>

Merge branch 'master' into feat/developer/9403-scancodes-epic-ldml

0cae964

feat(developer): scancode and forms 🙀

8200f84

- fix casing of k_oC1 - allow vkey 226 to be doubly defined - drop LdmlVkeyNames table and test, not needed anymore (was for vkey #7135) Fixes: #9403

mcdurdin reviewed Oct 2, 2023

View reviewed changes

common/web/types/test/ldml-keyboard/test-ldml-keyboard-xml-reader.ts Outdated Show resolved Hide resolved

feat(developer): scancode and forms 🙀

2b2661b

- fixes to the special case test Fixes: #9403

mcdurdin approved these changes Oct 3, 2023

View reviewed changes

srl295 merged commit 01c171c into master Oct 3, 2023
2 checks passed

srl295 deleted the feat/developer/9403-scancodes-epic-ldml branch October 3, 2023 13:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(developer): ldml scan codes support 🙀 #9615

feat(developer): ldml scan codes support 🙀 #9615

srl295 commented Sep 22, 2023 •

edited

Loading

keymanapp-test-bot bot commented Sep 22, 2023 •

edited

Loading

mcdurdin left a comment

mcdurdin Sep 25, 2023

srl295 Sep 25, 2023

srl295 Oct 1, 2023

srl295 commented Sep 25, 2023

srl295 commented Sep 28, 2023

srl295 commented Sep 29, 2023

mcdurdin commented Sep 29, 2023

srl295 commented Oct 1, 2023 •

edited

Loading

srl295 commented Oct 1, 2023

srl295 commented Oct 1, 2023

mcdurdin commented Oct 2, 2023

mcdurdin left a comment

srl295 commented Oct 2, 2023

mcdurdin commented Oct 2, 2023

mcdurdin left a comment

keyman-server commented Oct 3, 2023

feat(developer): ldml scan codes support 🙀 #9615

feat(developer): ldml scan codes support 🙀 #9615

Conversation

srl295 commented Sep 22, 2023 • edited Loading

keymanapp-test-bot bot commented Sep 22, 2023 • edited Loading

User Test Results

Test Artifacts

mcdurdin left a comment

Choose a reason for hiding this comment

mcdurdin Sep 25, 2023

Choose a reason for hiding this comment

srl295 Sep 25, 2023

Choose a reason for hiding this comment

srl295 Oct 1, 2023

Choose a reason for hiding this comment

srl295 commented Sep 25, 2023

srl295 commented Sep 28, 2023

srl295 commented Sep 29, 2023

mcdurdin commented Sep 29, 2023

srl295 commented Oct 1, 2023 • edited Loading

srl295 commented Oct 1, 2023

srl295 commented Oct 1, 2023

mcdurdin commented Oct 2, 2023

mcdurdin left a comment

Choose a reason for hiding this comment

srl295 commented Oct 2, 2023

mcdurdin commented Oct 2, 2023

mcdurdin left a comment

Choose a reason for hiding this comment

keyman-server commented Oct 3, 2023

srl295 commented Sep 22, 2023 •

edited

Loading

keymanapp-test-bot bot commented Sep 22, 2023 •

edited

Loading

srl295 commented Oct 1, 2023 •

edited

Loading