From 77f8f41c21bb2cd8c623d6c7463fcb930d467eee Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Tue, 1 Nov 2022 16:55:58 -0500
Subject: [PATCH] =?UTF-8?q?feat(developer):=20improve=20validation=20and?=
 =?UTF-8?q?=20gap/switch=20=F0=9F=99=80?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- keys: error if to, gap, AND switch are missing
- add parts to LKKey
- add err message and test case

#7401
---
 .../src/ldml-keyboard/ldml-keyboard-xml.ts    |  2 ++
 .../src/kmc-keyboard/src/compiler/keys.ts     |  5 ++++
 .../src/kmc-keyboard/src/compiler/messages.ts |  4 +++
 .../fixtures/sections/keys/gap-switch.xml     | 26 +++++++++++++++++++
 .../keys/invalid-key-missing-attrs.xml        | 19 ++++++++++++++
 developer/src/kmc-keyboard/test/test-keys.ts  | 13 ++++++++++
 6 files changed, 69 insertions(+)
 create mode 100644 developer/src/kmc-keyboard/test/fixtures/sections/keys/gap-switch.xml
 create mode 100644 developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-key-missing-attrs.xml

diff --git a/common/web/types/src/ldml-keyboard/ldml-keyboard-xml.ts b/common/web/types/src/ldml-keyboard/ldml-keyboard-xml.ts
index 0787ada9bc..568d186ae6 100644
--- a/common/web/types/src/ldml-keyboard/ldml-keyboard-xml.ts
+++ b/common/web/types/src/ldml-keyboard/ldml-keyboard-xml.ts
@@ -71,6 +71,8 @@ export interface LKKeys {
 export interface LKKey {
   id?: string;
   to?: string;
+  gap?: boolean;
+  switch?: string;
 };
 
 export interface LKLayers {
diff --git a/developer/src/kmc-keyboard/src/compiler/keys.ts b/developer/src/kmc-keyboard/src/compiler/keys.ts
index 733e98049c..4e54bbf8a6 100644
--- a/developer/src/kmc-keyboard/src/compiler/keys.ts
+++ b/developer/src/kmc-keyboard/src/compiler/keys.ts
@@ -38,6 +38,11 @@ export class KeysCompiler extends SectionCompiler {
           valid = false;
           continue;
         }
+        if (!keydef.to && !keydef.gap && !keydef.switch) {
+          this.callbacks.reportMessage(CompilerMessages.Error_KeyMissingToGapOrSwitch({keyId: key}));
+          valid = false;
+          continue;
+        }
       }
     }
 
diff --git a/developer/src/kmc-keyboard/src/compiler/messages.ts b/developer/src/kmc-keyboard/src/compiler/messages.ts
index 7f6c37f57e..0a6801f9d2 100644
--- a/developer/src/kmc-keyboard/src/compiler/messages.ts
+++ b/developer/src/kmc-keyboard/src/compiler/messages.ts
@@ -77,6 +77,10 @@ export class CompilerMessages {
     m(this.ERROR_DisplayIsRepeated, `display to='${o.to}' has more than one display entry.`);
   static ERROR_DisplayIsRepeated = SevError | 0x0010;
 
+  static Error_KeyMissingToGapOrSwitch = (o:{keyId: string}) =>
+  m(this.ERROR_KeyMissingToGapOrSwitch, `key id='${o.keyId}' must have either to=, gap=, or switch=.`);
+static ERROR_KeyMissingToGapOrSwitch = SevError | 0x0011;
+
   static severityName(code: number): string {
     let severity = code & CompilerErrorSeverity.Severity_Mask;
     switch(severity) {
diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/gap-switch.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/gap-switch.xml
new file mode 100644
index 0000000000..7701b7ca36
--- /dev/null
+++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/gap-switch.xml
@@ -0,0 +1,26 @@
+<?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-gap-switch" />
+  </names>
+
+  <keys>
+    <key id="Q" gap="true" />
+    <key id="W" switch="shift" />
+    <key id="q" to="q" />
+    <key id="w" to="w" />
+  </keys>
+
+  <layers form="hardware">
+    <layer id="base">
+      <!-- beware: this is mapping ` and 1! -->
+      <row keys="Q W" />
+    </layer>
+    <layer id="shift">
+      <!-- beware: this is mapping ` and 1! -->
+      <row keys="q w" />
+    </layer>
+  </layers>
+</keyboard>
diff --git a/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-key-missing-attrs.xml b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-key-missing-attrs.xml
new file mode 100644
index 0000000000..21c798d8f9
--- /dev/null
+++ b/developer/src/kmc-keyboard/test/fixtures/sections/keys/invalid-key-missing-attrs.xml
@@ -0,0 +1,19 @@
+<?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-missing-attrs" />
+  </names>
+
+  <keys>
+    <key id="Q" /> <!-- missing to, gap, and switch -->
+  </keys>
+
+  <layers form="hardware">
+    <layer id="base">
+      <!-- beware: this is mapping `! -->
+      <row keys="Q" />
+    </layer>
+  </layers>
+</keyboard>
diff --git a/developer/src/kmc-keyboard/test/test-keys.ts b/developer/src/kmc-keyboard/test/test-keys.ts
index cdc2fbfaac..d435a3d628 100644
--- a/developer/src/kmc-keyboard/test/test-keys.ts
+++ b/developer/src/kmc-keyboard/test/test-keys.ts
@@ -55,4 +55,17 @@ describe('keys', function () {
 
     assert.deepEqual(compilerTestCallbacks.messages[0], CompilerMessages.Error_KeyNotFoundInKeyBag({col: 1, form: 'hardware', keyId: 'foo', layer: 'base', row: 1}));
   });
+  it('should reject layouts with invalid keys', function() {
+    let keys = loadSectionFixture(KeysCompiler, 'sections/keys/invalid-key-missing-attrs.xml', compilerTestCallbacks) as Keys;
+    assert.isNull(keys);
+    assert.equal(compilerTestCallbacks.messages.length, 1);
+
+    assert.deepEqual(compilerTestCallbacks.messages[0], CompilerMessages.Error_KeyMissingToGapOrSwitch({keyId: 'Q'}));
+  });
+  it('should accept layouts with gap/switch keys', function() {
+    let keys = loadSectionFixture(KeysCompiler, 'sections/keys/gap-switch.xml', compilerTestCallbacks) as Keys;
+    assert.isNotNull(keys);
+    assert.equal(compilerTestCallbacks.messages.length, 0);
+    assert.equal(keys.keys.length, 2);
+  });
 });