From 41f77a874cc82905f7dd5a8c82551f2ee17135ff Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Thu, 2 Feb 2023 17:33:09 -0600 Subject: [PATCH 1/3] =?UTF-8?q?feat(common):=20ldml=20different=20hardware?= =?UTF-8?q?=20support=20=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - add HardwareToKeymap in common/web/types - add a mt-iso keyboard, very close to stock mt.xml - add machinery for parsing modifier key sequences - more test machinery - add assertions against invalid modifier sequences Fixes: #7965 --- .../types/src/consts/virtual-key-constants.ts | 29 +++- .../unit/ldml/keyboards/k_011_mt_iso.xml | 91 ++++++++++++ core/tests/unit/ldml/keyboards/meson.build | 1 + .../src/kmc-keyboard/src/compiler/keys.ts | 54 ++++--- .../src/kmc-keyboard/src/compiler/messages.ts | 6 +- .../src/compiler/visual-keyboard-compiler.ts | 2 +- developer/src/kmc-keyboard/src/util/util.ts | 16 +++ .../test/fixtures/sections/keys/escaped.xml | 2 +- .../fixtures/sections/keys/hardware_iso.xml | 22 +++ .../fixtures/sections/keys/hardware_us.xml | 22 +++ .../sections/keys/invalid-bad-modifier.xml | 17 +++ developer/src/kmc-keyboard/test/test-keys.ts | 132 +++++++++++++----- developer/src/kmc-keyboard/test/test-utils.ts | 18 ++- 13 files changed, 346 insertions(+), 66 deletions(-) create mode 100644 core/tests/unit/ldml/keyboards/k_011_mt_iso.xml create mode 100644 developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_iso.xml create mode 100644 developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_us.xml create mode 100644 developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml diff --git a/common/web/types/src/consts/virtual-key-constants.ts b/common/web/types/src/consts/virtual-key-constants.ts index b3ebdc15c5..38e1412e5d 100644 --- a/common/web/types/src/consts/virtual-key-constants.ts +++ b/common/web/types/src/consts/virtual-key-constants.ts @@ -1,6 +1,10 @@ // Define standard keycode numbers (exposed for use by other modules) -// TODO: merge with common\web\keyboard-processor\src\text\codes.ts +// TODO-LDML: merge with common\web\keyboard-processor\src\text\codes.ts + +/** + * May include non-US virtual key codes + */ export const USVirtualKeyCodes = { K_BKSP:8, K_TAB:9, @@ -129,7 +133,9 @@ export const USVirtualKeyCodes = { const k = USVirtualKeyCodes; -export const USVirtualKeyMap: number[][] = [ +export type KeyMap = number[][]; + +export const USVirtualKeyMap: KeyMap = [ // ` 1 2 3 4 5 6 7 8 9 0 - = [bksp] [ k.K_BKQUOTE, k.K_1, k.K_2, k.K_3, k.K_4, k.K_5, k.K_6, k.K_7, k.K_8, k.K_9, k.K_0, k.K_HYPHEN, k.K_EQUAL ], // [tab] Q W E R T Y U I O P [ ] \ @@ -142,11 +148,7 @@ export const USVirtualKeyMap: number[][] = [ [ k.K_SPACE ], ]; -/** - * TODO-LDML: WIP ISO layout for #7965 - * May not be correct to use US codes? - */ -export const ISOVirtualKeyMap: number[][] = [ +export const ISOVirtualKeyMap: KeyMap = [ // ` 1 2 3 4 5 6 7 8 9 0 - = [bksp] [ k.K_BKQUOTE, k.K_1, k.K_2, k.K_3, k.K_4, k.K_5, k.K_6, k.K_7, k.K_8, k.K_9, k.K_0, k.K_HYPHEN, k.K_EQUAL ], // [tab] Q W E R T Y U I O P [ ] @@ -159,6 +161,19 @@ export const ISOVirtualKeyMap: number[][] = [ [ k.K_SPACE ], ]; +/** + * Map from a hardware constnat to a keymap + * For the 'key' see constants.layr_list_hardware_map + */ +export const HardwareToKeymap: Map = new Map( + [ + ["us", USVirtualKeyMap], + ["iso", ISOVirtualKeyMap], + //TODO-LDML: jis #8161 + //TODO-LDML: abnt2 #8161 + ] +); + /** * Maps LDML VKey Names from CLDR VKey Enum in TR35 to Keyman virtual key codes */ diff --git a/core/tests/unit/ldml/keyboards/k_011_mt_iso.xml b/core/tests/unit/ldml/keyboards/k_011_mt_iso.xml new file mode 100644 index 0000000000..2b89cef958 --- /dev/null +++ b/core/tests/unit/ldml/keyboards/k_011_mt_iso.xml @@ -0,0 +1,91 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/core/tests/unit/ldml/keyboards/meson.build b/core/tests/unit/ldml/keyboards/meson.build index d061d3e270..b9b8847a66 100644 --- a/core/tests/unit/ldml/keyboards/meson.build +++ b/core/tests/unit/ldml/keyboards/meson.build @@ -12,6 +12,7 @@ tests = [ 'k_003_transform', 'k_004_tinyshift', 'k_010_mt', + 'k_011_mt_iso', 'k_100_keytest', 'k_101_keytest', 'k_102_keytest', diff --git a/developer/src/kmc-keyboard/src/compiler/keys.ts b/developer/src/kmc-keyboard/src/compiler/keys.ts index 677fc478f9..c5734e4a01 100644 --- a/developer/src/kmc-keyboard/src/compiler/keys.ts +++ b/developer/src/kmc-keyboard/src/compiler/keys.ts @@ -5,8 +5,7 @@ import { SectionCompiler } from "./section-compiler.js"; import GlobalSections = KMXPlus.GlobalSections; import Keys = KMXPlus.Keys; -import USVirtualKeyMap = Constants.USVirtualKeyMap; -import { calculateUniqueKeys, translateLayerAttrToModifier } from '../util/util.js'; +import { calculateUniqueKeys, translateLayerAttrToModifier, validModifier } from '../util/util.js'; export class KeysCompiler extends SectionCompiler { @@ -14,19 +13,33 @@ export class KeysCompiler extends SectionCompiler { return constants.section.keys; } - private validateHardwareLayer(layer: LDMLKeyboard.LKLayer) { - const uniqueKeys = calculateUniqueKeys([...this.keyboard.keys?.key]); + private validateHardwareLayer(hardware: string, layer: LDMLKeyboard.LKLayer) { let valid = true; - if(layer.row.length > USVirtualKeyMap.length) { + + const {modifier} = layer; + if (!validModifier(modifier)) { + this.callbacks.reportMessage(CompilerMessages.Error_InvalidModifier({modifier, layer: layer.id})); + valid = false; + } + + const keymap = Constants.HardwareToKeymap.get(hardware); + if (!keymap) { + this.callbacks.reportMessage(CompilerMessages.Error_InvalidHardware({hardware})); + valid = false; + return valid; // can't do anything else here + } + + const uniqueKeys = calculateUniqueKeys([...this.keyboard.keys?.key]); + if(layer.row.length > keymap.length) { this.callbacks.reportMessage(CompilerMessages.Error_HardwareLayerHasTooManyRows()); valid = false; } - for(let y = 0; y < layer.row.length && y < USVirtualKeyMap.length; y++) { + for(let y = 0; y < layer.row.length && y < keymap.length; y++) { const keys = layer.row[y].keys.split(' '); - if(keys.length > USVirtualKeyMap[y].length) { - this.callbacks.reportMessage(CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({row: y+1})); + if(keys.length > keymap[y].length) { + this.callbacks.reportMessage(CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({row: y+1, hardware})); valid = false; } @@ -53,15 +66,17 @@ export class KeysCompiler extends SectionCompiler { public validate() { let valid = true; - if(!this.keyboard.layers?.[0]?.layer?.length) { + + const theLayers = this.keyboard.layers?.[0]; // TODO-LDML: handle >1 layers. #8160 + + if(!theLayers?.layer?.length) { valid = false; this.callbacks.reportMessage(CompilerMessages.Error_MustBeAtLeastOneLayerElement()); } - // TODO-LDML: handle >1 layers! - if(this.keyboard.layers?.[0]?.form == 'hardware') { - for(let layer of this.keyboard.layers[0].layer) { - valid = this.validateHardwareLayer(layer) && valid; // note: always validate even if previously invalid results found + if(theLayers?.form == 'hardware') { + for(let layer of theLayers?.layer) { + valid = this.validateHardwareLayer(theLayers?.hardware, layer) && valid; // note: always validate even if previously invalid results found } } return valid; @@ -69,17 +84,16 @@ export class KeysCompiler extends SectionCompiler { public compile(sections: GlobalSections): Keys { // Use LayerMap + keys to generate compiled keys for hardware + const theLayers = this.keyboard.layers?.[0]; // TODO-LDML: handle >1 layers. #8160 - if(this.keyboard.layers?.[0]?.form == 'hardware') { + if(theLayers?.form == 'hardware') { let sect = new Keys(); - for(let layer of this.keyboard.layers[0].layer) { - this.compileHardwareLayer(sections, layer, sect); + for(let layer of theLayers.layer) { + this.compileHardwareLayer(sections, layer, sect, theLayers.hardware); } return sect; } - // TODO-LDML: generate vkey mapping for touch-only keys - return null; } @@ -87,8 +101,10 @@ export class KeysCompiler extends SectionCompiler { sections: GlobalSections, layer: LDMLKeyboard.LKLayer, sect: Keys, + hardware: string, ): Keys { const mod = translateLayerAttrToModifier(layer); + const keymap = Constants.HardwareToKeymap.get(hardware); let y = -1; for(let row of layer.row) { @@ -102,7 +118,7 @@ export class KeysCompiler extends SectionCompiler { let keydef = this.keyboard.keys?.key?.find(x => x.id == key); sect.keys.push({ - vkey: USVirtualKeyMap[y][x], + vkey: keymap[y][x], mod: mod, to: sections.strs.allocAndUnescapeString(keydef.to), flags: 0 // Note: 'expand' is never set here, only by the .kmx builder diff --git a/developer/src/kmc-keyboard/src/compiler/messages.ts b/developer/src/kmc-keyboard/src/compiler/messages.ts index cd137d125b..bd3b885c0e 100644 --- a/developer/src/kmc-keyboard/src/compiler/messages.ts +++ b/developer/src/kmc-keyboard/src/compiler/messages.ts @@ -16,7 +16,7 @@ export class CompilerMessages { static Error_HardwareLayerHasTooManyRows = () => m(this.ERROR_HardwareLayerHasTooManyRows, `'hardware' layer has too many rows`); static ERROR_HardwareLayerHasTooManyRows = SevError | 0x0003; - static Error_RowOnHardwareLayerHasTooManyKeys = (o:{row: number}) => m(this.ERROR_RowOnHardwareLayerHasTooManyKeys, `Row #${o.row} on 'hardware' layer has too many keys`); + static Error_RowOnHardwareLayerHasTooManyKeys = (o:{row: number, hardware: string}) => m(this.ERROR_RowOnHardwareLayerHasTooManyKeys, `Row #${o.row} on 'hardware' ${o.hardware} layer has too many keys`); static ERROR_RowOnHardwareLayerHasTooManyKeys = SevError | 0x0004; static Error_KeyNotFoundInKeyBag = (o:{keyId: string, col: number, row: number, layer: string, form: string}) => @@ -87,6 +87,10 @@ export class CompilerMessages { `layers has invalid value hardware=${o.hardware}`); static ERROR_InvalidHardware = SevError | 0x0015; + static Error_InvalidModifier = (o:{layer: string, modifier: string}) => m(this.ERROR_InvalidModifier, + `layer has invalid modifier='${o.modifier}' on layer id=${o.layer}`); + static ERROR_InvalidModifier = SevError | 0x0016; + static severityName(code: number): string { let severity = code & CompilerErrorSeverity.Severity_Mask; switch(severity) { diff --git a/developer/src/kmc-keyboard/src/compiler/visual-keyboard-compiler.ts b/developer/src/kmc-keyboard/src/compiler/visual-keyboard-compiler.ts index 12a9c858e8..8eee0d0ad2 100644 --- a/developer/src/kmc-keyboard/src/compiler/visual-keyboard-compiler.ts +++ b/developer/src/kmc-keyboard/src/compiler/visual-keyboard-compiler.ts @@ -48,7 +48,7 @@ export default class VisualKeyboardCompiler { flags: VisualKeyboard.VisualKeyboardKeyFlags.kvkkUnicode, shift: shift, text: keydef.to, // TODO-LDML: displays - vkey: Constants.USVirtualKeyMap[y][x] + vkey: Constants.USVirtualKeyMap[y][x] // TODO-LDML: #7965 US-only }); } } diff --git a/developer/src/kmc-keyboard/src/util/util.ts b/developer/src/kmc-keyboard/src/util/util.ts index 276959f12f..699b89bc0a 100644 --- a/developer/src/kmc-keyboard/src/util/util.ts +++ b/developer/src/kmc-keyboard/src/util/util.ts @@ -76,3 +76,19 @@ export function translateLayerAttrToModifier(layer: LDMLKeyboard.LKLayer) : numb // TODO-LDML: other modifiers, other ids? return constants.keys_mod_none; } + +/** + * @param modifier modifier sequence such as undefined, "none", "shift altR" etc + * @returns true if valid + */ +export function validModifier(modifier?: string) : boolean { + if (!modifier) return true; // valid to have no modifier, == none + for (let str of modifier.split(' ')) { + if (!constants.keys_mod_map.has(str)) { + return false; + } + } + return true; +} + + diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/escaped.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/escaped.xml index 0b448e754b..f1d60148ce 100644 --- a/developer/src/kmc-keyboard/test/fixtures/sections/keys/escaped.xml +++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/escaped.xml @@ -10,7 +10,7 @@ - + diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_iso.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_iso.xml new file mode 100644 index 0000000000..6eeeb0bec3 --- /dev/null +++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_iso.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_us.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_us.xml new file mode 100644 index 0000000000..de7f29acc5 --- /dev/null +++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_us.xml @@ -0,0 +1,22 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml new file mode 100644 index 0000000000..e61c3c18e2 --- /dev/null +++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + diff --git a/developer/src/kmc-keyboard/test/test-keys.ts b/developer/src/kmc-keyboard/test/test-keys.ts index 08612dca0b..2540741ed3 100644 --- a/developer/src/kmc-keyboard/test/test-keys.ts +++ b/developer/src/kmc-keyboard/test/test-keys.ts @@ -1,10 +1,12 @@ import 'mocha'; import { assert } from 'chai'; import { KeysCompiler } from '../src/compiler/keys.js'; -import { compilerTestCallbacks, loadSectionFixture } from './helpers/index.js'; -import { KMXPlus } from '@keymanapp/common-types'; +import { compilerTestCallbacks, loadSectionFixture, testCompilationCases } from './helpers/index.js'; +import { KMXPlus, Constants } from '@keymanapp/common-types'; import { CompilerMessages } from '../src/compiler/messages.js'; +const K = Constants.USVirtualKeyCodes; + import Keys = KMXPlus.Keys; import { constants } from '@keymanapp/ldml-keyboard-constants'; @@ -18,42 +20,101 @@ describe('keys', function () { assert.equal(keys.keys.length, 1); }); - it('should compile escaped keys data', function() { - let keys = loadSectionFixture(KeysCompiler, 'sections/keys/escaped.xml', compilerTestCallbacks) as Keys; - assert.isNotNull(keys); - assert.equal(compilerTestCallbacks.messages.length, 0); - assert.equal(keys.keys.length, 1); - assert.equal(keys.keys[0].to.value, String.fromCodePoint(0x1faa6)); - }); - - it('should compile a hardware layer', function() { - let keys = loadSectionFixture(KeysCompiler, 'sections/keys/hardware.xml', compilerTestCallbacks) as Keys; - assert.isNotNull(keys); - assert.equal(compilerTestCallbacks.messages.length, 0); - assert.equal(keys.keys.length, 4); - assert.sameDeepMembers(keys.keys.map(({vkey, to, mod}) => ({vkey, to: to.value, mod})), [ - { - vkey: 192, - to: 'qqq', - mod: constants.keys_mod_none, + testCompilationCases(KeysCompiler, [ + { + subpath: 'sections/keys/escaped.xml', + callback: (keys, subpath, callbacks) => { + assert.isNotNull(keys); + assert.equal((keys).keys.length, 1); + assert.equal((keys).keys[0].to.value, String.fromCodePoint(0x1faa6)); }, - { - vkey: '1'.charCodeAt(0), - to: 'www', - mod: constants.keys_mod_none, + }, + { + subpath: 'sections/keys/hardware.xml', + callback: (sect, subpath, callbacks) => { + const keys = sect as Keys; + assert.isNotNull(keys); + assert.equal(compilerTestCallbacks.messages.length, 0); + assert.equal(keys.keys.length, 4); + assert.sameDeepMembers(keys.keys.map(({vkey, to, mod}) => ({vkey, to: to.value, mod})), [ + { + vkey: K.K_BKQUOTE, + to: 'qqq', + mod: constants.keys_mod_none, + }, + { + vkey: K.K_1, + to: 'www', + mod: constants.keys_mod_none, + }, + { + vkey: K.K_BKQUOTE, + to: 'QQQ', + mod: constants.keys_mod_shift, + }, + { + vkey: K.K_1, + to: 'WWW', + mod: constants.keys_mod_shift, + }, + ]); }, - { - vkey: 192, - to: 'QQQ', - mod: constants.keys_mod_shift, + }, + { + subpath: 'sections/keys/hardware_us.xml', + callback: (sect, subpath, callbacks) => { + const keys = sect as Keys; + assert.isNotNull(keys); + assert.includeDeepMembers(keys.keys.map(({vkey, to, mod}) => ({vkey, to: to.value, mod})), [ + { + vkey: K.K_BKSLASH, + to: '\\', + mod: constants.keys_mod_none, + }, + { + vkey: K.K_Z, + to: 'z', + mod: constants.keys_mod_none, + }, + { + vkey: K.K_BKQUOTE, + to: '`', + mod: constants.keys_mod_none, + }, + ]); }, - { - vkey: '1'.charCodeAt(0), - to: 'WWW', - mod: constants.keys_mod_shift, + }, + { + subpath: 'sections/keys/hardware_iso.xml', + callback: (sect, subpath, callbacks) => { + const keys = sect as Keys; + assert.isNotNull(keys); + assert.includeDeepMembers(keys.keys.map(({vkey, to, mod}) => ({vkey, to: to.value, mod})), [ + { + vkey: K.K_oE2, + to: '\\', + mod: constants.keys_mod_none, + }, + { + vkey: 'Z'.charCodeAt(0), + to: 'z', + mod: constants.keys_mod_none, + }, + { + vkey: 192, + to: '`', + mod: constants.keys_mod_none, + }, + ]); }, - ]); - }); + }, + { + subpath: 'sections/keys/invalid-bad-modifier.xml', + errors: [ + CompilerMessages.Error_InvalidModifier({layer:'base',modifier:'altR-shift'}), + ] + }, + ]); it('should reject structurally invalid layers', function() { let keys = loadSectionFixture(KeysCompiler, 'sections/keys/invalid-missing-layer.xml', compilerTestCallbacks) as Keys; @@ -76,7 +137,7 @@ describe('keys', function () { assert.isNull(keys); assert.equal(compilerTestCallbacks.messages.length, 1); - assert.deepEqual(compilerTestCallbacks.messages[0], CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({row: 1})); + assert.deepEqual(compilerTestCallbacks.messages[0], CompilerMessages.Error_RowOnHardwareLayerHasTooManyKeys({row: 1, hardware: 'us'})); }); it('should reject layouts with undefined keys', function() { @@ -98,5 +159,4 @@ describe('keys', function () { assert.equal(compilerTestCallbacks.messages.length, 0); assert.equal(keys.keys.length, 4); }); - // TODO-LDML: { } }); }); + describe('isValidModifier()', () => { + it('should treat falsy values as valid', () => { + for(let str of [ + null, undefined, '', 'none' + ]) { + assert.ok(validModifier(str), `validModifier(${JSON.stringify(str)})`); + } + }); + it('should treat bad values as invalid', () => { + for(let str of [ + 'asdfasdf', 'shift asdfasdf', 'altR-shift', 'altR-shift shift' + ]) { + assert.notOk(validModifier(str), `validModifier(${JSON.stringify(str)})`); + } + }); + }); }); From f74a419bc6ad8a99c156130e60cc1b106a976b78 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Fri, 3 Feb 2023 19:03:26 -0600 Subject: [PATCH 2/3] chore: Review comment --- common/web/types/src/consts/virtual-key-constants.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/web/types/src/consts/virtual-key-constants.ts b/common/web/types/src/consts/virtual-key-constants.ts index 38e1412e5d..199060312e 100644 --- a/common/web/types/src/consts/virtual-key-constants.ts +++ b/common/web/types/src/consts/virtual-key-constants.ts @@ -162,7 +162,7 @@ export const ISOVirtualKeyMap: KeyMap = [ ]; /** - * Map from a hardware constnat to a keymap + * Map from a hardware constant to a keymap * For the 'key' see constants.layr_list_hardware_map */ export const HardwareToKeymap: Map = new Map( From 2a5d38e19f68cc1e89a603ce36240d4b804199b5 Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Mon, 6 Feb 2023 11:26:26 -0600 Subject: [PATCH 3/3] Update developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml Co-authored-by: Marc Durdin --- .../test/fixtures/sections/keys/invalid-bad-modifier.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml index e61c3c18e2..ea2eaedb44 100644 --- a/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml +++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml @@ -9,7 +9,7 @@ - +