Merge pull request #8172 from keymanapp/feat/developer/7965-morehardware-epic-ldml

feat(common): ldml different hardware support 🙀

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 constant to a keymap
+ * For the 'key' see constants.layr_list_hardware_map
+ */
+export const HardwareToKeymap: Map<string, KeyMap> = 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
  */

core/tests/unit/ldml/keyboards/k_011_mt_iso.xml

@@ -0,0 +1,91 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+@@keys: [SHIFT K_B][K_O][K_N][K_LBRKT][K_U][K_SPACE][K_S][K_A][K_RBRKT][K_RBRKT][K_A]
+@@expected: \u0042\u006f\u006e\u0121\u0075\u0020\u0073\u0061\u0127\u0127\u0061
+
+Note:
+    "Bonġu saħħa".split('').map(s=>`\\u${s.codePointAt(0).toString(16)}`).join('')
+Gets part of the way,
+
+Exact copy of mt.xml from CLDR, but with:
+  - an updated DTD path
+  - test case
+  - changed 'altR-shift' to 'altR shift'
+-->
+<!DOCTYPE keyboard SYSTEM "../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard.dtd">
+<keyboard locale="mt" conformsTo="techpreview">
+    <locales>
+        <!-- English is also an official language in Malta.-->
+        <locale id="en" />
+    </locales>
+    <info author="Steven R. Loomis" normalization="NFC" layout="QWERTY" indicator="MT" />
+
+    <names>
+        <name value="Maltese (48-key)" />
+        <!-- <name value="MSA 100:2002" /> -->
+    </names>
+
+    <keys>
+        <!-- imports -->
+        <import base="cldr" path="techpreview/keys-Zyyy-punctuation.xml"/>
+
+        <!-- accent grave -->
+        <key id="a-grave" to="à" />
+        <key id="A-grave" to="À" />
+        <key id="e-grave" to="è" />
+        <key id="E-grave" to="È" />
+        <key id="i-grave" to="ì" />
+        <key id="I-grave" to="Ì" />
+        <key id="o-grave" to="ò" />
+        <key id="O-grave" to="Ò" />
+        <key id="u-grave" to="ù" />
+        <key id="U-grave" to="Ù" />
+
+        <!-- tikka and maqtua -->
+        <key id="c-tikka" to="ċ" />
+        <key id="C-tikka" to="Ċ" />
+        <key id="g-tikka" to="ġ" />
+        <key id="G-tikka" to="Ġ" />
+        <key id="h-maqtugha" to="ħ" /> <!-- maqtugħa, i.e. cut -->
+        <key id="H-maqtugha" to="Ħ" /> <!-- maqtugħa, i.e. cut -->
+        <key id="z-tikka" to="ż" />
+        <key id="Z-tikka" to="Ż" />
+
+        <!-- Cedilla -->
+        <key id="c-cedilla" to="ç" />
+    </keys>
+
+    <layers form="hardware" hardware="iso">
+        <layer modifier="none">
+            <row keys="c-tikka 1 2 3 4 5 6 7 8 9 0 hyphen equal" />
+            <row keys="q w e r t y u i o p g-tikka h-maqtugha" />
+            <row keys="a s d f g h j k l semi-colon hash" />
+            <row keys="z-tikka z x c v b n m comma period slash" />
+            <row keys="space" />
+        </layer>
+
+        <layer modifier="shift">
+            <row keys="C-tikka bang double-quote euro dollar percent caret amp open-paren close-paren underscore plus" />
+            <row keys="Q W E R T Y U I O P G-tikka H-maqtugha" />
+            <row keys="A S D F G H J K L colon at tilde" />
+            <row keys="Z-tikka Z X C V B N M open-angle close-angle question" />
+            <row keys="space" />
+        </layer>
+
+        <layer modifier="altR">
+            <row keys="grave gap gap pound gap gap gap gap gap gap gap gap gap" />
+            <row keys="gap gap e-grave gap gap gap u-grave i-grave o-grave gap open-square close-square" />
+            <row keys="a-grave gap gap gap gap gap gap gap gap gap gap gap" />
+            <row keys="backslash gap gap gap gap gap gap gap gap gap gap" />
+            <row keys="space" />
+        </layer>
+
+        <layer modifier="altR shift">
+            <row keys="not gap gap gap gap gap gap gap gap gap gap gap gap" />
+            <row keys="gap gap E-grave gap gap gap U-grave I-grave O-grave gap open-curly close-curly" />
+            <row keys="A-grave gap gap gap gap gap gap gap gap gap gap gap" />
+            <row keys="pipe gap gap gap gap gap gap gap gap gap gap" />
+            <row keys="space" />
+        </layer>
+    </layers>
+</keyboard>

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',

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

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) {

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
         });
       }
     }

developer/src/kmc-keyboard/src/util/util.ts

@@ -73,3 +73,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;
+}
+
+

developer/src/kmc-keyboard/test/fixtures/sections/keys/escaped.xml

@@ -10,7 +10,7 @@
     <key id="grave" to="\u{1faa6}" />
   </keys>
 
-  <layers form="hardware">
+  <layers form="hardware" hardware="us">
     <layer id="base">
       <row keys="grave" />
     </layer>

developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_iso.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard.dtd">
+<keyboard locale="mt" conformsTo="techpreview">
+  <names>
+    <name value="hardware-minimal" />
+  </names>
+
+  <keys>
+    <import base="cldr" path="techpreview/keys-Zyyy-punctuation.xml"/>
+  </keys>
+
+  <layers form="hardware" hardware="iso">
+    <layer id="base" modifier="none">
+      <row keys="grave 1 2 3 4 5 6 7 8 9 0 hyphen equal" />
+      <row keys="q w e r t y u i o p open-square close-square" />
+      <row keys="a s d f g h j k l semi-colon apos" />
+      <row keys="backslash z x c v b n m comma period slash" />
+      <row keys="space" />
+    </layer>
+  </layers>
+</keyboard>

developer/src/kmc-keyboard/test/fixtures/sections/keys/hardware_us.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard.dtd">
+<keyboard locale="mt" conformsTo="techpreview">
+  <names>
+    <name value="hardware-minimal" />
+  </names>
+
+  <keys>
+    <import base="cldr" path="techpreview/keys-Zyyy-punctuation.xml"/>
+  </keys>
+
+  <layers form="hardware" hardware="us">
+    <layer id="base" modifier="none">
+      <row keys="grave 1 2 3 4 5 6 7 8 9 0 hyphen equal" />
+      <row keys="q w e r t y u i o p open-square close-square backslash" />
+      <row keys="a s d f g h j k l semi-colon apos" />
+      <row keys="z x c v b n m comma period slash" />
+      <row keys="space" />
+    </layer>
+  </layers>
+</keyboard>

developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-bad-modifier.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!DOCTYPE keyboard SYSTEM "../../../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard.dtd">
+<keyboard locale="mt" conformsTo="techpreview">
+  <names>
+    <name value="keys-minimal" />
+  </names>
+
+  <keys />
+
+  <layers form="hardware" hardware="us">
+    <layer id="base" modifier="altR-shift"> <!-- invalid, should be "altR shift" -->
+      <row keys="a b c" />
+    </layer>
+  </layers>
+
+</keyboard>

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).keys.length, 1);
+        assert.equal((<Keys>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:  <modifier="altR-shift" should throw a message
 });

developer/src/kmc-keyboard/test/test-utils.ts

@@ -1,6 +1,6 @@
 import 'mocha';
 import {assert} from 'chai';
-import { isValidEnumValue, calculateUniqueKeys, allUsedKeyIdsInLayers, translateLayerAttrToModifier } from '../src/util/util.js';
+import { isValidEnumValue, calculateUniqueKeys, allUsedKeyIdsInLayers, translateLayerAttrToModifier, validModifier } from '../src/util/util.js';
 import { constants } from "@keymanapp/ldml-keyboard-constants";
 import { LDMLKeyboard } from '@keymanapp/common-types';
 
@@ -145,4 +145,20 @@ describe('test of util/util.ts', () => {
       }
     });
   });
+  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)})`);
+      }
+    });
+  });
 });