From dd11a25b2731c0431baa6114a275ece4dd9e7ab6 Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Wed, 6 Nov 2024 09:30:18 -0600
Subject: [PATCH 01/13] feat(core): ldml: scaffolding for testing
 get_key_list()

- move some test utils to statics
- add a new test action type and `@@key-list` keyword
- print warning on unhanded @-commands

Fixes: #12298
---
 .../unit/ldml/keyboards/k_004_tinyshift.xml   |  1 +
 core/tests/unit/ldml/ldml.cpp                 | 49 +++++++++++++++++
 core/tests/unit/ldml/ldml_test_source.cpp     | 36 +++++++++----
 core/tests/unit/ldml/ldml_test_source.hpp     | 53 +++++++++++++++++--
 4 files changed, 123 insertions(+), 16 deletions(-)

diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index 4346fa801b..620ecf7c32 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -4,6 +4,7 @@
 
 @@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE]
 @@expected: \u0037\u1790\u17B6\u0127
+@@keylist: [SHIFT K_BKQUOTE][SHIFT K_1][K_BKQUOTE][K_1]
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index 12e0c94399..86d98658d6 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -16,6 +16,7 @@
 #include <sstream>
 #include <string>
 #include <type_traits>
+#include <set>
 
 #include "path.hpp"
 #include "state.hpp"
@@ -267,6 +268,44 @@ verify_context(std::u16string &text_store, km_core_state *&test_state, std::vect
   delete[] buf;
 }
 
+
+bool
+verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_event> &expected) {
+  bool equals = true;
+  for(const auto &akey : actual) {
+    if (expected.count(akey) == 0) {
+      equals = false;
+      std::u16string dump = convert<char, char16_t>(akey.dump());  // akey.dump()
+      std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had extra key " << dump << console_color::reset() << std::endl;
+    }
+  }
+  for(const auto &ekey : expected) {
+    if (actual.count(ekey) == 0) {
+      equals = false;
+      std::u16string dump = convert<char, char16_t>(ekey.dump());  // akey.dump()
+      std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had missing key " << dump << console_color::reset() << std::endl;
+    }
+  }
+  return equals;
+}
+
+bool
+verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &expected_list) {
+  std::set<km::tests::key_event> actual, expected;
+  std::string expected_str = convert<char16_t, char>(expected_list);
+  // convert actual list
+  while (actual_list != nullptr && actual_list->key != 0 && actual_list->modifier_flag != 0) {
+    actual.emplace(actual_list->key, (uint16_t)actual_list->modifier_flag);
+    actual_list++; // advance pointer
+  }
+  // parse expected_str
+  while (!expected_str.empty()) {
+    km::tests::key_event event = km::tests::LdmlEmbeddedTestSource::parse_next_key(expected_str);
+    expected.insert(event);
+  }
+  return verify_key_list(actual, expected);
+}
+
 int
 run_test(const km::core::path &source, const km::core::path &compiled, km::tests::LdmlTestSource& test_source) {
   km_core_keyboard * test_kb = nullptr;
@@ -372,6 +411,16 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
         errorLine = __LINE__;
       }
     } break;
+    case km::tests::LDML_ACTION_CHECK_KEYLIST: {
+      std::cout << "- checking keylist " << action.string << std::endl;
+      // get keylist from kbd
+      const km_core_keyboard_key* actual_list = test_kb->get_key_list();
+      if (!verify_key_list(actual_list, action.string)) {
+        errorLine = __LINE__;
+      } else {
+        std::cout << " .. matches." << std::endl;
+      }
+    } break;
     case km::tests::LDML_ACTION_FAIL: {
       // test requested failure
       std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL: " << action.string << console_color::reset()
diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index f2cca9ee5f..511b50bb67 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -269,6 +269,7 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path ) {
   const std::string s_context = "@@context: ";
   const std::string s_capsLock = "@@capsLock: ";
   const std::string s_expecterror = "@@expect-error: ";
+  const std::string s_keylist = "@@keylist: ";
 
   // Parse out the header statements in file.kmn that tell us (a) environment, (b) key sequence, (c) start context, (d) expected
   // result
@@ -300,6 +301,10 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path ) {
       context = parse_source_string(line);
     } else if (is_token(s_capsLock, line)) {
       set_caps_lock_on(parse_source_string(line).compare(u"1") == 0);
+    } else if (is_token(s_keylist, line)) {
+      set_keylist(line);
+    } else if (line[0] == '@') {
+      std::cerr << path << " warning, unknown @-command " << line << std::endl;
     }
   }
 
@@ -356,7 +361,7 @@ LdmlTestSource::char_to_event(char ch) {
 }
 
 uint16_t
-LdmlTestSource::get_modifier(std::string const m) {
+LdmlTestSource::get_modifier(std::string const &m) {
   for (int i = 0; km::core::kmx::s_modifier_names[i].name; i++) {
     if (m == km::core::kmx::s_modifier_names[i].name) {
       return km::core::kmx::s_modifier_names[i].modifier;
@@ -365,6 +370,12 @@ LdmlTestSource::get_modifier(std::string const m) {
   return 0;
 }
 
+std::string key_event::dump() const {
+  std::stringstream f;
+  f  << "Key: {" << km::core::kmx::Debug_VirtualKey(vk) << ", " << km::core::kmx::Debug_ModifierName(modifier_state) << "}";
+  return f.str();
+}
+
 key_event
 LdmlEmbeddedTestSource::vkey_to_event(std::string const &vk_event) {
   // vkey format is MODIFIER MODIFIER K_NAME
@@ -400,34 +411,37 @@ LdmlEmbeddedTestSource::vkey_to_event(std::string const &vk_event) {
 
 void
 LdmlEmbeddedTestSource::next_action(ldml_action &fillin) {
-  if (is_done || keys.empty()) {
-    // We were already done. return done.
-    fillin.type = LDML_ACTION_DONE;
-    return;
+  if (keys.empty()) {
+    // #3 we are almost done, let's run the key check if needed
+    if (!check_keylist.empty()) {
+      fillin.type = LDML_ACTION_CHECK_KEYLIST;
+      fillin.string = check_keylist;
+      check_keylist.clear();
+    } else {
+      fillin.type = LDML_ACTION_DONE;
+    }
   } else if(keys[0].empty()) {
+    // #2. Then, when we finish a key set, we check the 'expected' at the end of it.
     // Got to the end of a key set. time to check
     fillin.type = LDML_ACTION_CHECK_EXPECTED;
     fillin.string = expected[0]; // copy expected
     expected.pop_front();
     keys.pop_front();
-    if (keys.empty()) {
-      is_done = true; // so we get DONE next time
-    }
   } else {
+    // #1 First, we process each key
     fillin.type = LDML_ACTION_KEY_EVENT;
     fillin.k = next_key();
   }
 }
 
-
 key_event
 LdmlEmbeddedTestSource::next_key() {
   // mutate this->keys
-  return next_key(keys[0]);
+  return parse_next_key(keys[0]);
 }
 
 key_event
-LdmlEmbeddedTestSource::next_key(std::string &keys) {
+LdmlEmbeddedTestSource::parse_next_key(std::string &keys) {
   // Parse the next element of the string, chop it off, and return it
   // mutates keys
   if (keys.length() == 0)
diff --git a/core/tests/unit/ldml/ldml_test_source.hpp b/core/tests/unit/ldml/ldml_test_source.hpp
index 4637af9bd4..5992724352 100644
--- a/core/tests/unit/ldml/ldml_test_source.hpp
+++ b/core/tests/unit/ldml/ldml_test_source.hpp
@@ -14,6 +14,34 @@ namespace tests {
 struct key_event {
   km_core_virtual_key vk;
   uint16_t modifier_state;
+  public:
+    key_event() : vk(0), modifier_state(0) {
+    }
+    key_event(km_core_virtual_key k, uint16_t m) :vk(k), modifier_state(m) {
+    }
+    std::string dump() const;
+  int compare(const key_event &other) const {
+    if (vk < other.vk) return -1;
+    if (vk > other.vk) return 1;
+    if (modifier_state < other.modifier_state) return -1;
+    if (modifier_state > other.modifier_state) return 1;
+    return 0;
+  }
+
+  bool operator<(const key_event &other) const {
+    return compare(other) < 0;
+  }
+  bool operator>(const key_event &other) const {
+    return compare(other) > 0;
+  }
+  bool operator==(const key_event &other) const {
+    return compare(other) == 0;
+  }
+  /** true if unset (null) key */
+  bool empty() const {
+    return vk == 0 && modifier_state == 0;
+  }
+
 };
 
 enum ldml_action_type {
@@ -37,6 +65,10 @@ enum ldml_action_type {
    * expected text
   */
   LDML_ACTION_CHECK_EXPECTED,
+  /**
+   * string - keylist to check
+   */
+  LDML_ACTION_CHECK_KEYLIST,
   // TODO-LDML: gestures, etc? Depends on touch.
 
   /**
@@ -80,7 +112,7 @@ public:
 
   // helper functions
   static key_event char_to_event(char ch);
-  static uint16_t get_modifier(std::string const m);
+  static uint16_t get_modifier(std::string const &m);
   static std::u16string parse_source_string(std::string const &s);
   static std::u16string parse_u8_source_string(std::string const &s);
 
@@ -143,18 +175,29 @@ public:
   virtual void next_action(ldml_action &fillin);
 
 private:
-
-  bool is_token(const std::string token, std::string &line);
-  key_event vkey_to_event(std::string const &vk_event);
-  key_event next_key(std::string &keys);
   key_event next_key();
 
+  void set_keylist(std::string const &s) {
+    check_keylist = parse_source_string(s);
+  }
+
+  std::u16string check_keylist;
   std::deque<std::string> keys;
   std::deque<std::u16string> expected;
   std::u16string context = u"";
   bool expected_beep = false;
   bool expected_error = false;
   bool is_done = false;
+
+  /** returns false on fail and updates the message */
+  bool handle_check_keylist(std::string &message) const;
+
+// utility
+  static key_event vkey_to_event(std::string const &vk_event);
+  static bool is_token(const std::string token, std::string &line);
+
+public:
+  static key_event parse_next_key(std::string &keys);
 };
 
 }  // namespace tests

From 382b511dce7239fe42d4dbe21dcb41cad691d16d Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Thu, 7 Nov 2024 09:34:30 -0600
Subject: [PATCH 02/13] feat(core): ldml: implementation for testing
 get_key_list()

- ldml::vkeys class updated to keep a set<> of keys
- fix the test case to not leak!

Fixes: #12298
---
 core/src/ldml/ldml_processor.cpp |  3 +--
 core/src/ldml/ldml_vkeys.cpp     | 16 ++++++++++++++++
 core/src/ldml/ldml_vkeys.hpp     |  7 +++++++
 core/tests/unit/ldml/ldml.cpp    |  6 ++++--
 4 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/core/src/ldml/ldml_processor.cpp b/core/src/ldml/ldml_processor.cpp
index fcadda629c..4e42c3acea 100644
--- a/core/src/ldml/ldml_processor.cpp
+++ b/core/src/ldml/ldml_processor.cpp
@@ -348,8 +348,7 @@ km_core_attr const & ldml_processor::attributes() const {
 }
 
 km_core_keyboard_key  * ldml_processor::get_key_list() const {
-  km_core_keyboard_key* key_list = new km_core_keyboard_key(KM_CORE_KEYBOARD_KEY_LIST_END);
-  return key_list;
+  return keys.get_key_list();
 }
 
 km_core_keyboard_imx  * ldml_processor::get_imx_list() const {
diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
index f79f82141f..5ab5082b73 100644
--- a/core/src/ldml/ldml_vkeys.cpp
+++ b/core/src/ldml/ldml_vkeys.cpp
@@ -22,6 +22,22 @@ vkeys::add(km_core_virtual_key vk, km_core_ldml_modifier_state modifier_state, s
   const vkey_id id(vk, modifier_state);
   // assign the string
   vkey_to_string[id] = output;
+  if (!output.empty()) {
+    // empty string = gap key, etc.
+    all_vkeys.insert(id);
+  }
+}
+
+km_core_keyboard_key  *
+vkeys::get_key_list() const {
+  km_core_keyboard_key *list = new km_core_keyboard_key[all_vkeys.size() + 1];
+  std::size_t n = 0;
+  for (const auto &k : all_vkeys) {
+    list[n  ].key           = k.first;
+    list[n++].modifier_flag = k.second;
+  }
+  list[n++] = KM_CORE_KEYBOARD_KEY_LIST_END;
+  return list;
 }
 
 static const uint16_t BOTH_ALT  = LALTFLAG  | RALTFLAG;
diff --git a/core/src/ldml/ldml_vkeys.hpp b/core/src/ldml/ldml_vkeys.hpp
index 6a54d77065..916d805e2c 100644
--- a/core/src/ldml/ldml_vkeys.hpp
+++ b/core/src/ldml/ldml_vkeys.hpp
@@ -12,6 +12,7 @@
 #include <unordered_map>
 #include <utility>
 #include <vector>
+#include <set>
 
 #include "keyman_core.h"
 
@@ -37,6 +38,7 @@ typedef std::pair<km_core_virtual_key, km_core_ldml_modifier_state> vkey_id;
 class vkeys {
 private:
   std::map<vkey_id, std::u16string> vkey_to_string;
+  std::set<vkey_id> all_vkeys;
 
 public:
   vkeys();
@@ -53,6 +55,11 @@ public:
   std::u16string
   lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) const;
 
+  /**
+   * For implementing ldml_processor::get_key_list()
+   */
+  km_core_keyboard_key* get_key_list() const;
+
 private:
   /**
    * Non-recursive internal lookup of a specific ID
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index 86d98658d6..daf9dd7d1d 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -294,8 +294,9 @@ verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &e
   std::set<km::tests::key_event> actual, expected;
   std::string expected_str = convert<char16_t, char>(expected_list);
   // convert actual list
-  while (actual_list != nullptr && actual_list->key != 0 && actual_list->modifier_flag != 0) {
-    actual.emplace(actual_list->key, (uint16_t)actual_list->modifier_flag);
+  while (actual_list != nullptr && !(actual_list->key == 0 && actual_list->modifier_flag == 0)) {
+    km::tests::key_event k(actual_list->key, (uint16_t)actual_list->modifier_flag);
+    actual.insert(k);
     actual_list++; // advance pointer
   }
   // parse expected_str
@@ -420,6 +421,7 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
       } else {
         std::cout << " .. matches." << std::endl;
       }
+      delete [] actual_list;
     } break;
     case km::tests::LDML_ACTION_FAIL: {
       // test requested failure

From 2e5b6019e1d0787d34b20c677e1dcfeaf3ba3da9 Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Sat, 23 Nov 2024 12:51:24 -0800
Subject: [PATCH 03/13] feat(core): get_key_list needs to include all keys

Fixes: #12298
---
 core/src/ldml/ldml_vkeys.cpp | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
index 5ab5082b73..90ec6acdef 100644
--- a/core/src/ldml/ldml_vkeys.cpp
+++ b/core/src/ldml/ldml_vkeys.cpp
@@ -22,10 +22,8 @@ vkeys::add(km_core_virtual_key vk, km_core_ldml_modifier_state modifier_state, s
   const vkey_id id(vk, modifier_state);
   // assign the string
   vkey_to_string[id] = output;
-  if (!output.empty()) {
-    // empty string = gap key, etc.
-    all_vkeys.insert(id);
-  }
+  // includes all keys - including gaps.
+  all_vkeys.insert(id);
 }
 
 km_core_keyboard_key  *

From 6d1035a4adf8b9b24ffa39ecf841f1d2440d720f Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Mon, 25 Nov 2024 12:54:02 -0600
Subject: [PATCH 04/13] feat(core): test improvements for get_key_list()

- move kmxplus processing into the base LdmlTestSource class
- add a function to traverse the layer list looking for keys to add
- The @@keylist keyword only has one example from each modifier set

Fixes: #12298
---
 core/src/kmx/kmx_plus.h                       |   6 +
 .../unit/ldml/keyboards/k_004_tinyshift.xml   |   3 +-
 core/tests/unit/ldml/ldml.cpp                 |  17 +-
 core/tests/unit/ldml/ldml_test_source.cpp     | 173 ++++++++++++++----
 core/tests/unit/ldml/ldml_test_source.hpp     |  18 +-
 5 files changed, 168 insertions(+), 49 deletions(-)

diff --git a/core/src/kmx/kmx_plus.h b/core/src/kmx/kmx_plus.h
index a8e5c1f0e4..e3a510b76d 100644
--- a/core/src/kmx/kmx_plus.h
+++ b/core/src/kmx/kmx_plus.h
@@ -455,7 +455,13 @@ public:
   bool setLayr(const COMP_KMXPLUS_LAYR *newLayr);
   bool valid() const;
 
+  /**
+   * @param list index from 0 to layr->listCount
+   */
   const COMP_KMXPLUS_LAYR_LIST  *getList(KMX_DWORD list) const;
+  /**
+   * @param entry index value: COMP_KMXPLUS_LAYR_LIST.layer but less than COMP_KMXPLUS_LAYR_LIST.layer+COMP_KMXPLUS_LAYR_LIST.count
+   */
   const COMP_KMXPLUS_LAYR_ENTRY *getEntry(KMX_DWORD entry) const;
   const COMP_KMXPLUS_LAYR_ROW   *getRow(KMX_DWORD row) const;
   const COMP_KMXPLUS_LAYR_KEY   *getKey(KMX_DWORD key) const;
diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index 620ecf7c32..35aef528c7 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -4,7 +4,8 @@
 
 @@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE]
 @@expected: \u0037\u1790\u17B6\u0127
-@@keylist: [SHIFT K_BKQUOTE][SHIFT K_1][K_BKQUOTE][K_1]
+'keylist' is a list with one example key per modifier. In this case, shifted and base.
+@@keylist: [SHIFT K_BKQUOTE][K_BKQUOTE]
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index daf9dd7d1d..f31925e7ba 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -290,7 +290,7 @@ verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_
 }
 
 bool
-verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &expected_list) {
+verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &expected_list, const km::tests::LdmlTestSource &test) {
   std::set<km::tests::key_event> actual, expected;
   std::string expected_str = convert<char16_t, char>(expected_list);
   // convert actual list
@@ -301,8 +301,17 @@ verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &e
   }
   // parse expected_str
   while (!expected_str.empty()) {
+    // we ignore the vkey here, just need modifier state
     km::tests::key_event event = km::tests::LdmlEmbeddedTestSource::parse_next_key(expected_str);
-    expected.insert(event);
+    auto modifier = event.modifier_state;
+
+    // now, fetch the keylist from KMX+
+    std::vector<km_core_virtual_key> keys;
+    assert(test.get_vkey_table(keys, modifier));
+    for(const km_core_virtual_key vkey : keys) {
+      // list of vkeys -> list of events
+      expected.emplace(vkey, modifier);
+    }
   }
   return verify_key_list(actual, expected);
 }
@@ -416,7 +425,7 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
       std::cout << "- checking keylist " << action.string << std::endl;
       // get keylist from kbd
       const km_core_keyboard_key* actual_list = test_kb->get_key_list();
-      if (!verify_key_list(actual_list, action.string)) {
+      if (!verify_key_list(actual_list, action.string, test_source)) {
         errorLine = __LINE__;
       } else {
         std::cout << " .. matches." << std::endl;
@@ -473,7 +482,7 @@ int run_all_tests(const km::core::path &source, const km::core::path &compiled,
 
   std::vector<std::string> failures; // track failures for summary
 
-  int embedded_result = embedded_test_source.load_source(source);
+  int embedded_result = embedded_test_source.load_source(source, compiled);
 
   if (!filter.empty()) {
     // Always skip the embedded test if there's a filter.
diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index 511b50bb67..5d64a3dfa8 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -126,6 +126,112 @@ bool LdmlTestSource::get_expected_beep() const {
   return false;
 }
 
+int LdmlTestSource::load_kmx_plus(const km::core::path &compiled) {
+    // check and load the KMX (yes, once again)
+  if(!km::core::ldml_processor::is_kmxplus_file(compiled, rawdata)) {
+    std::cerr << "Reading KMX for test purposes failed: " << compiled << std::endl;
+    return __LINE__;
+  }
+
+  auto comp_keyboard = (const km::core::kmx::COMP_KEYBOARD*)rawdata.data();
+  // initialize the kmxplus object with our copy
+  kmxplus.reset(new km::core::kmx::kmx_plus(comp_keyboard, rawdata.size()));
+
+  if (!kmxplus->is_valid()) {
+    std::cerr << "kmx_plus invalid" << std::endl;
+    return __LINE__;
+  }
+
+  if (!kmxplus->key2Helper.valid()) {
+    std::cerr << "kmx_plus invalid" << std::endl;
+    return __LINE__;
+  }
+
+  return 0; // success
+}
+
+bool LdmlTestSource::get_vkey_table(std::vector<km_core_virtual_key> &fillin, uint16_t modifier) const {
+  if (!kmxplus || !kmxplus->is_valid()) {
+    return false; // fail
+  }
+
+  // find the 'touch' id so we can avoid it
+  const std::string hardwareTouchStr(LDML_LAYR_LIST_HARDWARE_TOUCH);
+  const std::u16string hardwareTouch(convert<char, char16_t>(hardwareTouchStr));
+
+  // find the string 'touch' - it may not exist if there's no touch layer.
+  // in which case the ID here will be 0. Otherwise it's a string id.
+  const KMX_DWORD hardwareTouchId = kmxplus->strs->find(hardwareTouch);
+
+  // find the hardware list
+  const km::core::kmx::COMP_KMXPLUS_LAYR_LIST* hwList = nullptr;
+  for (KMX_DWORD listIdx = 0; listIdx < kmxplus->layr->listCount; listIdx++) {
+    const km::core::kmx::COMP_KMXPLUS_LAYR_LIST* list = kmxplus->layrHelper.getList(listIdx);
+    if (list == nullptr) {
+      return false; // hit a bad list
+    }
+
+    if (hardwareTouchId != 0 && list->hardware == hardwareTouchId) {
+      continue; // skip this list, it's the touch list.
+      // (there may not be a touch list.)
+    }
+    hwList = list;
+    break; // found it
+  }
+  if (hwList == nullptr) {
+    return false; // no list available
+  }
+  // now, look for the modifier set.
+  const km::core::kmx::COMP_KMXPLUS_LAYR_ENTRY *entry = nullptr;
+  for (KMX_DWORD layrIdx = 0; layrIdx < hwList->count; layrIdx++) {
+    const km::core::kmx::COMP_KMXPLUS_LAYR_ENTRY *e = kmxplus->layrHelper.getEntry(hwList->layer + layrIdx);
+    if (e != nullptr && e->mod == modifier) {
+      entry = e;
+      break;
+    }
+  }
+  if(entry == nullptr) {
+    return false; // mod not found
+  }
+
+  // now we have the layer with this mod, now get all rows and populate vkeys
+  for (KMX_DWORD rowIdx = 0; rowIdx < entry->count; rowIdx++) {
+    const km::core::kmx::COMP_KMXPLUS_LAYR_ROW *row = kmxplus->layrHelper.getRow(entry->row + rowIdx);
+    if (row == nullptr) {
+      return false;
+    }
+    for (KMX_DWORD keyIdx = 0; keyIdx < row->count; keyIdx++) {
+      const km::core::kmx::COMP_KMXPLUS_LAYR_KEY *rowKey = kmxplus->layrHelper.getKey(row->key + keyIdx);
+      if (rowKey == nullptr) {
+        return false;
+      }
+      // we may not actually need the key but just its ID, but we get it in case
+      KMX_DWORD keyId = 0;
+      const km::core::kmx::COMP_KMXPLUS_KEYS_KEY *key = kmxplus->key2Helper.findKeyByStringId(rowKey->key, keyId);
+      if (key == nullptr) {
+        return false;
+      }
+      // Now, linear search the kmap table looking for the key
+      bool found_kmap = false;
+      for (KMX_DWORD kmapIdx = 0; kmapIdx < kmxplus->key2->kmapCount; kmapIdx++) {
+        const km::core::kmx::COMP_KMXPLUS_KEYS_KMAP *kmap = kmxplus->key2Helper.getKmap(kmapIdx);
+        if (kmap == nullptr) {
+          return false;
+        }
+        if (kmap->key == keyId) {
+          fillin.emplace_back(kmap->vkey);
+          found_kmap = true;
+          break;
+        }
+      }
+      if (!found_kmap) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
 // String trim functions from https://stackoverflow.com/a/217605/1836776
 // trim from start (in place)
 static inline void
@@ -263,7 +369,7 @@ LdmlEmbeddedTestSource::is_token(const std::string token, std::string &line) {
 }
 
 int
-LdmlEmbeddedTestSource::load_source( const km::core::path &path ) {
+LdmlEmbeddedTestSource::load_source( const km::core::path &path, const km::core::path &compiled ) {
   const std::string s_keys = "@@keys: ";
   const std::string s_expected = "@@expected: ";
   const std::string s_context = "@@context: ";
@@ -320,7 +426,13 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path ) {
     return __LINE__;
   }
 
-  return 0;
+  // and load KMX+ for keylist
+  if (!expected_error) {
+    // don't attempt to load KMX+ on expected error
+    return load_kmx_plus(compiled);
+  } else {
+    return 0;
+  }
 }
 
 km_core_status
@@ -466,10 +578,10 @@ LdmlEmbeddedTestSource::parse_next_key(std::string &keys) {
 
 class LdmlJsonTestSource : public LdmlTestSource {
 public:
-  LdmlJsonTestSource(const std::string &path, km::core::kmx::kmx_plus *kmxplus);
+  LdmlJsonTestSource(const std::string &path);
   virtual ~LdmlJsonTestSource();
   virtual const std::u16string &get_context();
-  int load(const nlohmann::json &test);
+  int load(const nlohmann::json &test, const km::core::path &compiled);
   virtual void next_action(ldml_action &fillin);
 private:
   std::string path;
@@ -480,14 +592,13 @@ private:
    * Which action are we on?
   */
   std::size_t action_index = -1;
-  const km::core::kmx::kmx_plus *kmxplus;
   /** @return false if not found */
   bool set_key_from_id(key_event& k, const std::u16string& id);
   bool loaded_context = false;
 };
 
-LdmlJsonTestSource::LdmlJsonTestSource(const std::string &path, km::core::kmx::kmx_plus *k)
-:path(path), kmxplus(k) {
+LdmlJsonTestSource::LdmlJsonTestSource(const std::string &path)
+:path(path) {
 
 }
 
@@ -599,19 +710,21 @@ LdmlJsonTestSource::get_context() {
   return context;
 }
 
-int LdmlJsonTestSource::load(const nlohmann::json &data) {
+int LdmlJsonTestSource::load(const nlohmann::json &data, const km::core::path &compiled) {
   this->data        = data;
-  // TODO-LDML: validate here?
-  return 0;
+  // TODO-LDML: validate JSON here?
+
+  // load up the kmx_plus
+  return load_kmx_plus(compiled);
 }
 
 #if defined(HAVE_ICU4C)
 class LdmlJsonRepertoireTestSource : public LdmlTestSource {
 public:
-  LdmlJsonRepertoireTestSource(const std::string &path, km::core::kmx::kmx_plus *kmxplus);
+  LdmlJsonRepertoireTestSource(const std::string &path );
   virtual ~LdmlJsonRepertoireTestSource();
   virtual const std::u16string &get_context();
-  int load(const nlohmann::json &test);
+  int load(const nlohmann::json &test, const km::core::path &compiled);
   virtual void next_action(ldml_action &fillin);
 private:
   std::string path;
@@ -623,11 +736,10 @@ private:
   std::unique_ptr<icu::UnicodeSet> uset;
   std::unique_ptr<icu::UnicodeSetIterator> iterator;
   bool need_check = false; // set this after each char
-  const km::core::kmx::kmx_plus *kmxplus;
 };
 
-LdmlJsonRepertoireTestSource::LdmlJsonRepertoireTestSource(const std::string &path, km::core::kmx::kmx_plus *k)
-:path(path), kmxplus(k){
+LdmlJsonRepertoireTestSource::LdmlJsonRepertoireTestSource(const std::string &path)
+:path(path) {
 
 }
 
@@ -715,7 +827,7 @@ LdmlJsonRepertoireTestSource::get_context() {
   return context; // no context needed
 }
 
-int LdmlJsonRepertoireTestSource::load(const nlohmann::json &data) {
+int LdmlJsonRepertoireTestSource::load(const nlohmann::json &data, const km::core::path &compiled) {
   this->data = data;  // TODO-LDML
   // Need an update to json.hpp to use contains()
   // if (data.contains("/type"_json_pointer)) {
@@ -746,7 +858,7 @@ int LdmlJsonRepertoireTestSource::load(const nlohmann::json &data) {
   // }
   // #endif
   iterator = std::unique_ptr<icu::UnicodeSetIterator>(new icu::UnicodeSetIterator(*uset));
-  return 0;
+  return load_kmx_plus(compiled);
 }
 #endif // HAVE_ICU4C
 
@@ -770,25 +882,6 @@ int LdmlJsonTestSourceFactory::load(const km::core::path &compiled, const km::co
     return __LINE__; // empty
   }
 
-  // check and load the KMX (yes, once again)
-  if(!km::core::ldml_processor::is_kmxplus_file(compiled, rawdata)) {
-    std::cerr << "Reading KMX for test purposes failed: " << compiled << std::endl;
-    return __LINE__;
-  }
-
-  auto comp_keyboard = (const km::core::kmx::COMP_KEYBOARD*)rawdata.data();
-  // initialize the kmxplus object with our copy
-  kmxplus.reset(new km::core::kmx::kmx_plus(comp_keyboard, rawdata.size()));
-
-  if (!kmxplus->is_valid()) {
-    std::cerr << "kmx_plus invalid" << std::endl;
-    return __LINE__;
-  }
-
-  if (!kmxplus->key2Helper.valid()) {
-    std::cerr << "kmx_plus invalid" << std::endl;
-    return __LINE__;
-  }
 
   auto conformsTo = data["/keyboardTest3/conformsTo"_json_pointer].get<std::string>();
   assert_or_return(std::string(LDML_CLDR_TEST_VERSION_LATEST) == conformsTo);
@@ -811,8 +904,8 @@ int LdmlJsonTestSourceFactory::load(const km::core::path &compiled, const km::co
       test_path.append(info_name).append("/tests/").append(tests_name).append("/").append(test_name);
       // std::cout << "JSON: reading " << info_name << "/" << test_path << std::endl;
 
-      std::unique_ptr<LdmlJsonTestSource> subtest(new LdmlJsonTestSource(test_path, kmxplus.get()));
-      assert_or_return(subtest->load(test) == 0);
+      std::unique_ptr<LdmlJsonTestSource> subtest(new LdmlJsonTestSource(test_path));
+      assert_or_return(subtest->load(test, compiled) == 0);
       test_map[test_path] = std::unique_ptr<LdmlTestSource>(subtest.release());
     }
   }
@@ -827,8 +920,8 @@ int LdmlJsonTestSourceFactory::load(const km::core::path &compiled, const km::co
     std::string test_path;
     test_path.append(info_name).append("/repertoire/").append(rep_name);
 
-    std::unique_ptr<LdmlJsonRepertoireTestSource> reptest(new LdmlJsonRepertoireTestSource(test_path, kmxplus.get()));
-    assert_or_return(reptest->load(rep) == 0);
+    std::unique_ptr<LdmlJsonRepertoireTestSource> reptest(new LdmlJsonRepertoireTestSource(test_path));
+    assert_or_return(reptest->load(rep, compiled) == 0);
     test_map[test_path] = std::unique_ptr<LdmlTestSource>(reptest.release());
   }
 
diff --git a/core/tests/unit/ldml/ldml_test_source.hpp b/core/tests/unit/ldml/ldml_test_source.hpp
index 5992724352..9a3db60034 100644
--- a/core/tests/unit/ldml/ldml_test_source.hpp
+++ b/core/tests/unit/ldml/ldml_test_source.hpp
@@ -109,6 +109,13 @@ public:
   virtual km_core_status get_expected_load_status();
   virtual const std::u16string &get_context() = 0;
   virtual bool get_expected_beep() const;
+  /**
+   * fillin a list
+   * @param fillin key list to be filled in with all vkeys in the hardware map.
+   * @param modifier modifier to load key list for
+   * @returns false on load fail, true on OK
+   */
+  bool get_vkey_table(std::vector<km_core_virtual_key> &fillin, uint16_t modifier) const;
 
   // helper functions
   static key_event char_to_event(char ch);
@@ -134,6 +141,12 @@ private:
 
 private:
   bool _caps_lock_on = false;
+protected:
+  /** populate rawdata and kmxplus */
+  int load_kmx_plus(const km::core::path &compiled);
+  // copy of the kbd data, for lookups
+  std::vector<uint8_t> rawdata;
+  std::unique_ptr<km::core::kmx::kmx_plus> kmxplus;
 };
 
 typedef std::map<std::string, std::unique_ptr<km::tests::LdmlTestSource>> JsonTestMap;
@@ -152,9 +165,6 @@ class LdmlJsonTestSourceFactory {
     const JsonTestMap& get_tests() const;
   private:
     JsonTestMap test_map;
-    // copy of the kbd data, for lookups
-    std::vector<uint8_t> rawdata;
-    std::unique_ptr<km::core::kmx::kmx_plus> kmxplus;
 };
 
 
@@ -166,7 +176,7 @@ public:
   /**
    * Load the test_source from comments in the .xml source
    */
-  int load_source(const km::core::path &path);
+  int load_source(const km::core::path &path, const km::core::path &compiled);
 
   virtual km_core_status get_expected_load_status();
   virtual const std::u16string &get_context();

From f9862c3bdf92fd541da54a4ddd5c19753ed7b9fe Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Tue, 26 Nov 2024 09:36:11 -0600
Subject: [PATCH 05/13] feat(core): test improvements for get_key_list()

- just compare the key list to the key2.kmap table
- check the keylist for all LdmlTestSource instances - no syntax needed

Fixes: #12298
---
 .../unit/ldml/keyboards/k_004_tinyshift.xml   |  2 -
 core/tests/unit/ldml/ldml.cpp                 | 25 ++----
 core/tests/unit/ldml/ldml_test_source.cpp     | 88 +++----------------
 core/tests/unit/ldml/ldml_test_source.hpp     | 10 +--
 4 files changed, 22 insertions(+), 103 deletions(-)

diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index 35aef528c7..4346fa801b 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -4,8 +4,6 @@
 
 @@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE]
 @@expected: \u0037\u1790\u17B6\u0127
-'keylist' is a list with one example key per modifier. In this case, shifted and base.
-@@keylist: [SHIFT K_BKQUOTE][K_BKQUOTE]
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index f31925e7ba..27e0e98ebd 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -286,33 +286,22 @@ verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_
       std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had missing key " << dump << console_color::reset() << std::endl;
     }
   }
+  if (equals) {
+      std::wcout << console_color::fg(console_color::GREEN) << " " << actual.size() << " vkeys OK " << console_color::reset() << std::endl;
+  }
   return equals;
 }
 
 bool
-verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &expected_list, const km::tests::LdmlTestSource &test) {
+verify_key_list(const km_core_keyboard_key *actual_list, const km::tests::LdmlTestSource &test) {
   std::set<km::tests::key_event> actual, expected;
-  std::string expected_str = convert<char16_t, char>(expected_list);
   // convert actual list
   while (actual_list != nullptr && !(actual_list->key == 0 && actual_list->modifier_flag == 0)) {
     km::tests::key_event k(actual_list->key, (uint16_t)actual_list->modifier_flag);
     actual.insert(k);
     actual_list++; // advance pointer
   }
-  // parse expected_str
-  while (!expected_str.empty()) {
-    // we ignore the vkey here, just need modifier state
-    km::tests::key_event event = km::tests::LdmlEmbeddedTestSource::parse_next_key(expected_str);
-    auto modifier = event.modifier_state;
-
-    // now, fetch the keylist from KMX+
-    std::vector<km_core_virtual_key> keys;
-    assert(test.get_vkey_table(keys, modifier));
-    for(const km_core_virtual_key vkey : keys) {
-      // list of vkeys -> list of events
-      expected.emplace(vkey, modifier);
-    }
-  }
+  assert(test.get_vkey_table(expected));
   return verify_key_list(actual, expected);
 }
 
@@ -422,10 +411,10 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
       }
     } break;
     case km::tests::LDML_ACTION_CHECK_KEYLIST: {
-      std::cout << "- checking keylist " << action.string << std::endl;
+      std::cout << "- checking keylist" << std::endl;
       // get keylist from kbd
       const km_core_keyboard_key* actual_list = test_kb->get_key_list();
-      if (!verify_key_list(actual_list, action.string, test_source)) {
+      if (!verify_key_list(actual_list, test_source)) {
         errorLine = __LINE__;
       } else {
         std::cout << " .. matches." << std::endl;
diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index 5d64a3dfa8..92d67bce67 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -150,84 +150,21 @@ int LdmlTestSource::load_kmx_plus(const km::core::path &compiled) {
   return 0; // success
 }
 
-bool LdmlTestSource::get_vkey_table(std::vector<km_core_virtual_key> &fillin, uint16_t modifier) const {
+bool LdmlTestSource::get_vkey_table(std::set<key_event> &fillin) const {
   if (!kmxplus || !kmxplus->is_valid()) {
     return false; // fail
   }
 
-  // find the 'touch' id so we can avoid it
-  const std::string hardwareTouchStr(LDML_LAYR_LIST_HARDWARE_TOUCH);
-  const std::u16string hardwareTouch(convert<char, char16_t>(hardwareTouchStr));
-
-  // find the string 'touch' - it may not exist if there's no touch layer.
-  // in which case the ID here will be 0. Otherwise it's a string id.
-  const KMX_DWORD hardwareTouchId = kmxplus->strs->find(hardwareTouch);
-
-  // find the hardware list
-  const km::core::kmx::COMP_KMXPLUS_LAYR_LIST* hwList = nullptr;
-  for (KMX_DWORD listIdx = 0; listIdx < kmxplus->layr->listCount; listIdx++) {
-    const km::core::kmx::COMP_KMXPLUS_LAYR_LIST* list = kmxplus->layrHelper.getList(listIdx);
-    if (list == nullptr) {
-      return false; // hit a bad list
-    }
-
-    if (hardwareTouchId != 0 && list->hardware == hardwareTouchId) {
-      continue; // skip this list, it's the touch list.
-      // (there may not be a touch list.)
-    }
-    hwList = list;
-    break; // found it
-  }
-  if (hwList == nullptr) {
-    return false; // no list available
-  }
-  // now, look for the modifier set.
-  const km::core::kmx::COMP_KMXPLUS_LAYR_ENTRY *entry = nullptr;
-  for (KMX_DWORD layrIdx = 0; layrIdx < hwList->count; layrIdx++) {
-    const km::core::kmx::COMP_KMXPLUS_LAYR_ENTRY *e = kmxplus->layrHelper.getEntry(hwList->layer + layrIdx);
-    if (e != nullptr && e->mod == modifier) {
-      entry = e;
-      break;
-    }
-  }
-  if(entry == nullptr) {
-    return false; // mod not found
-  }
-
-  // now we have the layer with this mod, now get all rows and populate vkeys
-  for (KMX_DWORD rowIdx = 0; rowIdx < entry->count; rowIdx++) {
-    const km::core::kmx::COMP_KMXPLUS_LAYR_ROW *row = kmxplus->layrHelper.getRow(entry->row + rowIdx);
-    if (row == nullptr) {
+  // just dump the kmap table
+  for (KMX_DWORD kmapIdx = 0; kmapIdx < kmxplus->key2->kmapCount; kmapIdx++) {
+    const km::core::kmx::COMP_KMXPLUS_KEYS_KMAP *kmap = kmxplus->key2Helper.getKmap(kmapIdx);
+    if (kmap == nullptr) {
       return false;
     }
-    for (KMX_DWORD keyIdx = 0; keyIdx < row->count; keyIdx++) {
-      const km::core::kmx::COMP_KMXPLUS_LAYR_KEY *rowKey = kmxplus->layrHelper.getKey(row->key + keyIdx);
-      if (rowKey == nullptr) {
-        return false;
-      }
-      // we may not actually need the key but just its ID, but we get it in case
-      KMX_DWORD keyId = 0;
-      const km::core::kmx::COMP_KMXPLUS_KEYS_KEY *key = kmxplus->key2Helper.findKeyByStringId(rowKey->key, keyId);
-      if (key == nullptr) {
-        return false;
-      }
-      // Now, linear search the kmap table looking for the key
-      bool found_kmap = false;
-      for (KMX_DWORD kmapIdx = 0; kmapIdx < kmxplus->key2->kmapCount; kmapIdx++) {
-        const km::core::kmx::COMP_KMXPLUS_KEYS_KMAP *kmap = kmxplus->key2Helper.getKmap(kmapIdx);
-        if (kmap == nullptr) {
-          return false;
-        }
-        if (kmap->key == keyId) {
-          fillin.emplace_back(kmap->vkey);
-          found_kmap = true;
-          break;
-        }
-      }
-      if (!found_kmap) {
-        return false;
-      }
+    if (kmap->vkey > 0xFF) {
+      continue; // synthetic key- skip
     }
+    fillin.insert(key_event(kmap->vkey, kmap->mod));
   }
   return true;
 }
@@ -375,7 +312,6 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path, const km::core:
   const std::string s_context = "@@context: ";
   const std::string s_capsLock = "@@capsLock: ";
   const std::string s_expecterror = "@@expect-error: ";
-  const std::string s_keylist = "@@keylist: ";
 
   // Parse out the header statements in file.kmn that tell us (a) environment, (b) key sequence, (c) start context, (d) expected
   // result
@@ -407,8 +343,6 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path, const km::core:
       context = parse_source_string(line);
     } else if (is_token(s_capsLock, line)) {
       set_caps_lock_on(parse_source_string(line).compare(u"1") == 0);
-    } else if (is_token(s_keylist, line)) {
-      set_keylist(line);
     } else if (line[0] == '@') {
       std::cerr << path << " warning, unknown @-command " << line << std::endl;
     }
@@ -525,10 +459,10 @@ void
 LdmlEmbeddedTestSource::next_action(ldml_action &fillin) {
   if (keys.empty()) {
     // #3 we are almost done, let's run the key check if needed
-    if (!check_keylist.empty()) {
+    if (check_keylist) {
       fillin.type = LDML_ACTION_CHECK_KEYLIST;
-      fillin.string = check_keylist;
-      check_keylist.clear();
+      // no params
+      check_keylist = false;
     } else {
       fillin.type = LDML_ACTION_DONE;
     }
diff --git a/core/tests/unit/ldml/ldml_test_source.hpp b/core/tests/unit/ldml/ldml_test_source.hpp
index 9a3db60034..96320bdf13 100644
--- a/core/tests/unit/ldml/ldml_test_source.hpp
+++ b/core/tests/unit/ldml/ldml_test_source.hpp
@@ -4,6 +4,7 @@
 #include "path.hpp"
 
 #include <map>
+#include <set>
 #include <memory>
 
 #include "kmx/kmx_plus.h"
@@ -115,7 +116,7 @@ public:
    * @param modifier modifier to load key list for
    * @returns false on load fail, true on OK
    */
-  bool get_vkey_table(std::vector<km_core_virtual_key> &fillin, uint16_t modifier) const;
+  bool get_vkey_table(std::set<key_event> &fillin) const;
 
   // helper functions
   static key_event char_to_event(char ch);
@@ -187,17 +188,14 @@ public:
 private:
   key_event next_key();
 
-  void set_keylist(std::string const &s) {
-    check_keylist = parse_source_string(s);
-  }
-
-  std::u16string check_keylist;
   std::deque<std::string> keys;
   std::deque<std::u16string> expected;
   std::u16string context = u"";
   bool expected_beep = false;
   bool expected_error = false;
   bool is_done = false;
+  /** did we check the keylist yet? */
+  bool check_keylist = true;
 
   /** returns false on fail and updates the message */
   bool handle_check_keylist(std::string &message) const;

From 2d81b195f001692ec06f37e291545e445dd605c6 Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Thu, 28 Nov 2024 12:48:32 -0600
Subject: [PATCH 06/13] feat(core): update k_004_tinyshift

- add a ctrl and an other layer

Fixes: #12298
---
 core/tests/unit/ldml/keyboards/k_004_tinyshift.xml | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index 4346fa801b..ec7c92ce95 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -2,8 +2,8 @@
 
 <!--
 
-@@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE]
-@@expected: \u0037\u1790\u17B6\u0127
+@@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE][LCTRL K_BKQUOTE][RALT K_BKQUOTE]
+@@expected: \u0037\u1790\u17B6\u0127\u1790\u17B6\u0065
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
@@ -23,5 +23,11 @@
     <layer id="shift" modifiers="shift">
       <row keys="seven eee" /> <!-- number row -->
     </layer>
+    <layer id="control" modifiers="ctrl">
+      <row keys="that gap" /> <!-- number row -->
+    </layer>
+    <layer id="catchall" modifiers="other">
+      <row keys="eee gap" /> <!-- number row -->
+    </layer>
   </layers>
 </keyboard3>

From 29db34f25222518cea136f3ff3505ccaa97755ff Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Fri, 29 Nov 2024 15:40:48 -0600
Subject: [PATCH 07/13] chore(core): update to ldmL_test_source.cpp

---
 core/tests/unit/ldml/ldml_test_source.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index 2ded08c900..79b6ac55f9 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -127,7 +127,7 @@ bool LdmlTestSource::get_expected_beep() const {
 
 int LdmlTestSource::load_kmx_plus(const km::core::path &compiled) {
     // check and load the KMX (yes, once again)
-  if(!km::core::ldml_processor::is_kmxplus_file(compiled, rawdata)) {
+  if(!km::core::ldml_processor::is_kmxplus_file(rawdata)) {
     std::cerr << "Reading KMX for test purposes failed: " << compiled << std::endl;
     return __LINE__;
   }

From d92dc175c20adb904413b23998794b2072e98eea Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Fri, 29 Nov 2024 15:47:30 -0600
Subject: [PATCH 08/13] chore(core): update to ldmL_test_source.cpp

---
 core/src/ldml/ldml_processor.hpp          | 4 ----
 core/tests/unit/ldml/ldml_test_source.cpp | 3 ++-
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/core/src/ldml/ldml_processor.hpp b/core/src/ldml/ldml_processor.hpp
index 82f59292c3..6bf8a8fa31 100644
--- a/core/src/ldml/ldml_processor.hpp
+++ b/core/src/ldml/ldml_processor.hpp
@@ -34,10 +34,6 @@ class ldml_processor : public abstract_processor {
       const std::vector<uint8_t> & data
     );
 
-    static bool is_kmxplus_file(
-      const std::vector<uint8_t> & data
-    );
-
     km_core_status
     process_event(
       km_core_state *state,
diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index 79b6ac55f9..2ce2430574 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -127,7 +127,8 @@ bool LdmlTestSource::get_expected_beep() const {
 
 int LdmlTestSource::load_kmx_plus(const km::core::path &compiled) {
     // check and load the KMX (yes, once again)
-  if(!km::core::ldml_processor::is_kmxplus_file(rawdata)) {
+  rawdata = km::tests::load_kmx_file(compiled);
+  if(!km::core::ldml_processor::is_handled(rawdata)) {
     std::cerr << "Reading KMX for test purposes failed: " << compiled << std::endl;
     return __LINE__;
   }

From 7f3bd309613b3565ba9848d1dc753036ae38b80a Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Fri, 29 Nov 2024 17:25:31 -0600
Subject: [PATCH 09/13] feat(core): improvements for get_key_list()

- expand OTHER and ALT / CTRL appropriately
- add KM_CORE_MODIFIER_NONE=0
- disable test of get_key_list() for now

Fixes: #12298
---
 core/include/keyman/keyman_core_api_vkeys.h |   1 +
 core/src/ldml/ldml_vkeys.cpp                | 103 +++++++++++++++++++-
 core/tests/unit/ldml/ldml.cpp               |   4 +
 3 files changed, 105 insertions(+), 3 deletions(-)

diff --git a/core/include/keyman/keyman_core_api_vkeys.h b/core/include/keyman/keyman_core_api_vkeys.h
index 7e4a6d181a..7e65d8ad91 100644
--- a/core/include/keyman/keyman_core_api_vkeys.h
+++ b/core/include/keyman/keyman_core_api_vkeys.h
@@ -16,6 +16,7 @@
 #pragma once
 
 enum km_core_modifier_state {
+  KM_CORE_MODIFIER_NONE        = 0,
   KM_CORE_MODIFIER_LCTRL       = 1 << 0,
   KM_CORE_MODIFIER_RCTRL       = 1 << 1,
   KM_CORE_MODIFIER_LALT        = 1 << 2,
diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
index 90ec6acdef..d341fa8c21 100644
--- a/core/src/ldml/ldml_vkeys.cpp
+++ b/core/src/ldml/ldml_vkeys.cpp
@@ -8,6 +8,8 @@
 #include "ldml_vkeys.hpp"
 #include "kmx_file.h"
 #include <ldml/keyman_core_ldml.h>
+#include <set>
+#include <assert.h>
 
 namespace km {
 namespace core {
@@ -28,13 +30,108 @@ vkeys::add(km_core_virtual_key vk, km_core_ldml_modifier_state modifier_state, s
 
 km_core_keyboard_key  *
 vkeys::get_key_list() const {
-  km_core_keyboard_key *list = new km_core_keyboard_key[all_vkeys.size() + 1];
+  // prescan to find out which modifier flags are used
+
+  std::size_t other_key_count = 0;        // number of 'OTHER' keys, which will need to expand to all of the other_state set ( so other_state.size())
+  std::size_t both_alt_key_count = 0;     // number of 'ALT' keys,   which will need to expand to LALT+RALT   (so 2)
+  std::size_t both_ctrl_key_count = 0;    // number of 'CTRL' keys,  which will need to expand to LCTRL+RCTRL (so 2)
+
+  std::set<km::core::ldml::km_core_ldml_modifier_state> all_modifiers;
+  for (const auto &k : all_vkeys) {
+    const auto mod = k.second;
+    if (mod == LDML_KEYS_MOD_OTHER) {
+      other_key_count++;
+    } else if (mod == LDML_KEYS_MOD_ALT) {
+      both_alt_key_count++;
+    } else if (mod == LDML_KEYS_MOD_CTRL) {
+      both_ctrl_key_count++;
+    }
+    all_modifiers.insert(mod);
+  }
+  std::set<km_core_modifier_state> other_state;
+
+  // Alt
+  if (all_modifiers.count(LDML_KEYS_MOD_ALT) == 0 && all_modifiers.count(LDML_KEYS_MOD_ALTL) == 0 && all_modifiers.count(LDML_KEYS_MOD_ALTR) == 0) {
+    // no ALT keys were seen, so OTHER includes ALT
+    other_state.insert(KM_CORE_MODIFIER_LALT);
+    other_state.insert(KM_CORE_MODIFIER_RALT);
+  } else if(all_modifiers.count(LDML_KEYS_MOD_ALTL) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_RALT);
+  } else if(all_modifiers.count(LDML_KEYS_MOD_ALTR) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_LALT);
+  }
+
+  // ctrl
+  if (all_modifiers.count(LDML_KEYS_MOD_CTRL) == 0 && all_modifiers.count(LDML_KEYS_MOD_CTRLL) == 0 && all_modifiers.count(LDML_KEYS_MOD_CTRLR) == 0) {
+    // no CTRL keys were seen, so OTHER includes CTRL
+    other_state.insert(KM_CORE_MODIFIER_LCTRL);
+    other_state.insert(KM_CORE_MODIFIER_RCTRL);
+  } else if(all_modifiers.count(LDML_KEYS_MOD_CTRLL) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_RCTRL);
+  } else if(all_modifiers.count(LDML_KEYS_MOD_CTRLR) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_LCTRL);
+  }
+
+  // shift
+  if (all_modifiers.count(LDML_KEYS_MOD_SHIFT) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_SHIFT);
+  }
+
+  // caps
+  if (all_modifiers.count(LDML_KEYS_MOD_CAPS) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_CAPS);
+  }
+
+  // none- it's possible there is no 'none' layer
+  if (all_modifiers.count(LDML_KEYS_MOD_NONE) == 0) {
+    other_state.insert(KM_CORE_MODIFIER_NONE);
+  }
+
+  const std::size_t new_list_size = all_vkeys.size() // original size
+    + (other_key_count * (other_state.size() - 1))
+    + (both_alt_key_count * 1)
+    + (both_ctrl_key_count * 1)
+    + 1; // terminator
+  km_core_keyboard_key *list = new km_core_keyboard_key[new_list_size];
   std::size_t n = 0;
   for (const auto &k : all_vkeys) {
-    list[n  ].key           = k.first;
-    list[n++].modifier_flag = k.second;
+    const auto vkey = k.first;
+    const auto mod  = k.second;
+
+    if (mod == LDML_KEYS_MOD_OTHER) {
+      // expand to all of other_state
+      for (const auto expanded_mod : other_state) {
+        list[n].key             = vkey;
+        list[n++].modifier_flag = expanded_mod;
+        assert(n <= new_list_size);
+      }
+    } else if (mod == LDML_KEYS_MOD_ALT) {
+      // expand to 'both'
+      list[n].key             = vkey;
+      list[n++].modifier_flag = KM_CORE_MODIFIER_LALT;
+      assert(n <= new_list_size);
+
+      list[n].key             = vkey;
+      list[n++].modifier_flag = KM_CORE_MODIFIER_RALT;
+      assert(n <= new_list_size);
+    } else if (mod == LDML_KEYS_MOD_CTRL) {
+      // expand to 'both'
+      list[n].key             = vkey;
+      list[n++].modifier_flag = KM_CORE_MODIFIER_LCTRL;
+      assert(n <= new_list_size);
+
+      list[n].key             = vkey;
+      list[n++].modifier_flag = KM_CORE_MODIFIER_RCTRL;
+      assert(n <= new_list_size);
+    } else {
+      assert(mod <= KM_CORE_MODIFIER_MASK_ALL); // that no LDMLisms escape
+      list[n].key             = vkey;
+      list[n++].modifier_flag = mod;
+      assert(n <= new_list_size);
+    }
   }
   list[n++] = KM_CORE_KEYBOARD_KEY_LIST_END;
+  assert(n == new_list_size);
   return list;
 }
 
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index 1e8a17eaac..77eceeeeb4 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -413,6 +413,7 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
       }
     } break;
     case km::tests::LDML_ACTION_CHECK_KEYLIST: {
+#if 0
       std::cout << "- checking keylist" << std::endl;
       // get keylist from kbd
       const km_core_keyboard_key* actual_list = test_kb->get_key_list();
@@ -422,6 +423,9 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
         std::cout << " .. matches." << std::endl;
       }
       delete [] actual_list;
+#else
+  std::cout << "skipping: check keylist" << std::endl;
+#endif
     } break;
     case km::tests::LDML_ACTION_FAIL: {
       // test requested failure

From 9e2b8d0d8c8c428c1d6820d811a2d2a6e42c3f2b Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Fri, 29 Nov 2024 18:28:05 -0600
Subject: [PATCH 10/13] feat(core): improvements for get_key_list()

- reintroduce example keycaps

Fixes: #12298
---
 core/src/ldml/ldml_vkeys.cpp                  | 28 +-------------
 .../unit/ldml/keyboards/k_004_tinyshift.xml   |  1 +
 core/tests/unit/ldml/ldml.cpp                 | 38 +++++++++++++------
 core/tests/unit/ldml/ldml_test_source.cpp     | 15 +++++++-
 core/tests/unit/ldml/ldml_test_source.hpp     |  6 +++
 5 files changed, 48 insertions(+), 40 deletions(-)

diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
index d341fa8c21..2815eb2ab4 100644
--- a/core/src/ldml/ldml_vkeys.cpp
+++ b/core/src/ldml/ldml_vkeys.cpp
@@ -33,18 +33,12 @@ vkeys::get_key_list() const {
   // prescan to find out which modifier flags are used
 
   std::size_t other_key_count = 0;        // number of 'OTHER' keys, which will need to expand to all of the other_state set ( so other_state.size())
-  std::size_t both_alt_key_count = 0;     // number of 'ALT' keys,   which will need to expand to LALT+RALT   (so 2)
-  std::size_t both_ctrl_key_count = 0;    // number of 'CTRL' keys,  which will need to expand to LCTRL+RCTRL (so 2)
 
   std::set<km::core::ldml::km_core_ldml_modifier_state> all_modifiers;
   for (const auto &k : all_vkeys) {
     const auto mod = k.second;
     if (mod == LDML_KEYS_MOD_OTHER) {
       other_key_count++;
-    } else if (mod == LDML_KEYS_MOD_ALT) {
-      both_alt_key_count++;
-    } else if (mod == LDML_KEYS_MOD_CTRL) {
-      both_ctrl_key_count++;
     }
     all_modifiers.insert(mod);
   }
@@ -89,8 +83,6 @@ vkeys::get_key_list() const {
 
   const std::size_t new_list_size = all_vkeys.size() // original size
     + (other_key_count * (other_state.size() - 1))
-    + (both_alt_key_count * 1)
-    + (both_ctrl_key_count * 1)
     + 1; // terminator
   km_core_keyboard_key *list = new km_core_keyboard_key[new_list_size];
   std::size_t n = 0;
@@ -105,26 +97,8 @@ vkeys::get_key_list() const {
         list[n++].modifier_flag = expanded_mod;
         assert(n <= new_list_size);
       }
-    } else if (mod == LDML_KEYS_MOD_ALT) {
-      // expand to 'both'
-      list[n].key             = vkey;
-      list[n++].modifier_flag = KM_CORE_MODIFIER_LALT;
-      assert(n <= new_list_size);
-
-      list[n].key             = vkey;
-      list[n++].modifier_flag = KM_CORE_MODIFIER_RALT;
-      assert(n <= new_list_size);
-    } else if (mod == LDML_KEYS_MOD_CTRL) {
-      // expand to 'both'
-      list[n].key             = vkey;
-      list[n++].modifier_flag = KM_CORE_MODIFIER_LCTRL;
-      assert(n <= new_list_size);
-
-      list[n].key             = vkey;
-      list[n++].modifier_flag = KM_CORE_MODIFIER_RCTRL;
-      assert(n <= new_list_size);
     } else {
-      assert(mod <= KM_CORE_MODIFIER_MASK_ALL); // that no LDMLisms escape
+      assert(mod <= KM_CORE_MODIFIER_CAPS); // that no LDMLisms escape
       list[n].key             = vkey;
       list[n++].modifier_flag = mod;
       assert(n <= new_list_size);
diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index ec7c92ce95..a2f300dc10 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -4,6 +4,7 @@
 
 @@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE][LCTRL K_BKQUOTE][RALT K_BKQUOTE]
 @@expected: \u0037\u1790\u17B6\u0127\u1790\u17B6\u0065
+@@keylist: [SHIFT K_BKQUOTE][SHIFT K_1][K_BKQUOTE][K_1][CTRL-do-not-use K_1][LALT K_BKQUOTE][RALT K_BKQUOTE]
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index 77eceeeeb4..ca2e95cc27 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -270,16 +270,23 @@ verify_context(std::u16string &text_store, km_core_state *&test_state, std::vect
 }
 
 
+/**
+ * @param actual the list from get_key_list()
+ * @param expected optional list with keys to check, can be empty - not exhaistive
+ * @returns true if passing
+ */
 bool
 verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_event> &expected) {
   bool equals = true;
+  // error if any bad modifier keys
   for(const auto &akey : actual) {
-    if (expected.count(akey) == 0) {
+    if (akey.modifier_state > KM_CORE_MODIFIER_CAPS) {
       equals = false;
       std::u16string dump = convert<char, char16_t>(akey.dump());  // akey.dump()
-      std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had extra key " << dump << console_color::reset() << std::endl;
+      std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had key with bad modifier " << akey.modifier_state << ": " << dump << console_color::reset() << std::endl;
     }
   }
+  // error if any expected keys missing (note expected may be empty)
   for(const auto &ekey : expected) {
     if (actual.count(ekey) == 0) {
       equals = false;
@@ -288,13 +295,19 @@ verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_
     }
   }
   if (equals) {
-      std::wcout << console_color::fg(console_color::GREEN) << " " << actual.size() << " vkeys OK " << console_color::reset() << std::endl;
+      std::wcout << console_color::fg(console_color::GREEN) << " " << actual.size() << " vkeys OK, verified  " << expected.size() << console_color::reset() << std::endl;
   }
   return equals;
 }
 
+/**
+ * @param actual_list the list from get_key_list()
+ * @param keylist optional string with keys to check, can be empty
+ * @param test the LDML test source, for additional data
+ * @returns true if passing
+ */
 bool
-verify_key_list(const km_core_keyboard_key *actual_list, const km::tests::LdmlTestSource &test) {
+verify_key_list(const km_core_keyboard_key *actual_list, const std::u16string &expected_list, const km::tests::LdmlTestSource &test) {
   std::set<km::tests::key_event> actual, expected;
   // convert actual list
   while (actual_list != nullptr && !(actual_list->key == 0 && actual_list->modifier_flag == 0)) {
@@ -302,7 +315,14 @@ verify_key_list(const km_core_keyboard_key *actual_list, const km::tests::LdmlTe
     actual.insert(k);
     actual_list++; // advance pointer
   }
-  assert(test.get_vkey_table(expected));
+  // parse the expected list
+  std::string keylist = convert<char16_t, char>(expected_list);
+  while (!keylist.empty() && keylist[0] == '[') {
+    const km::tests::key_event k = km::tests::LdmlEmbeddedTestSource::parse_next_key(keylist);
+    if (!k.empty()) {
+      expected.emplace(k);
+    }
+  }
   return verify_key_list(actual, expected);
 }
 
@@ -413,19 +433,15 @@ run_test(const km::core::path &source, const km::core::path &compiled, km::tests
       }
     } break;
     case km::tests::LDML_ACTION_CHECK_KEYLIST: {
-#if 0
       std::cout << "- checking keylist" << std::endl;
       // get keylist from kbd
       const km_core_keyboard_key* actual_list = test_kb->get_key_list();
-      if (!verify_key_list(actual_list, test_source)) {
+      if (!verify_key_list(actual_list, action.string, test_source)) {
         errorLine = __LINE__;
       } else {
-        std::cout << " .. matches." << std::endl;
+        std::cout << " .. passes." << std::endl;
       }
       delete [] actual_list;
-#else
-  std::cout << "skipping: check keylist" << std::endl;
-#endif
     } break;
     case km::tests::LDML_ACTION_FAIL: {
       // test requested failure
diff --git a/core/tests/unit/ldml/ldml_test_source.cpp b/core/tests/unit/ldml/ldml_test_source.cpp
index 2ce2430574..51f38af83a 100644
--- a/core/tests/unit/ldml/ldml_test_source.cpp
+++ b/core/tests/unit/ldml/ldml_test_source.cpp
@@ -312,6 +312,7 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path, const km::core:
   const std::string s_context = "@@context: ";
   const std::string s_capsLock = "@@capsLock: ";
   const std::string s_expecterror = "@@expect-error: ";
+  const std::string s_keylist = "@@keylist: ";
 
   // Parse out the header statements in file.kmn that tell us (a) environment, (b) key sequence, (c) start context, (d) expected
   // result
@@ -343,6 +344,8 @@ LdmlEmbeddedTestSource::load_source( const km::core::path &path, const km::core:
       context = parse_source_string(line);
     } else if (is_token(s_capsLock, line)) {
       set_caps_lock_on(parse_source_string(line).compare(u"1") == 0);
+    } else if (is_token(s_keylist, line)) {
+      set_keylist(line);
     } else if (line[0] == '@') {
       std::cerr << path << " warning, unknown @-command " << line << std::endl;
     }
@@ -458,10 +461,10 @@ LdmlEmbeddedTestSource::vkey_to_event(std::string const &vk_event) {
 void
 LdmlEmbeddedTestSource::next_action(ldml_action &fillin) {
   if (keys.empty()) {
-    // #3 we are almost done, let's run the key check if needed
+    // #3 we are almost done, let's run the key check
     if (check_keylist) {
       fillin.type = LDML_ACTION_CHECK_KEYLIST;
-      // no params
+      fillin.string = expected_keylist; // could be empty
       check_keylist = false;
     } else {
       fillin.type = LDML_ACTION_DONE;
@@ -529,6 +532,7 @@ private:
   /** @return false if not found */
   bool set_key_from_id(key_event& k, const std::u16string& id);
   bool loaded_context = false;
+  bool check_keys = true;
 };
 
 LdmlJsonTestSource::LdmlJsonTestSource(const std::string &path)
@@ -577,6 +581,13 @@ bool LdmlJsonTestSource::set_key_from_id(key_event& k, const std::u16string& id)
 void
 LdmlJsonTestSource::next_action(ldml_action &fillin) {
   if ((action_index+1) >= data["/actions"_json_pointer].size()) {
+    // add check keylist
+    if (check_keys) {
+      fillin.type = LDML_ACTION_CHECK_KEYLIST;
+      fillin.string.clear();
+      check_keys = false;
+      return;
+    }
     // at end, done
     fillin.type = LDML_ACTION_DONE;
     return;
diff --git a/core/tests/unit/ldml/ldml_test_source.hpp b/core/tests/unit/ldml/ldml_test_source.hpp
index 96320bdf13..c8e7061ed6 100644
--- a/core/tests/unit/ldml/ldml_test_source.hpp
+++ b/core/tests/unit/ldml/ldml_test_source.hpp
@@ -196,6 +196,12 @@ private:
   bool is_done = false;
   /** did we check the keylist yet? */
   bool check_keylist = true;
+  /** set the expected keys in the keylist */
+  void set_keylist(std::string const& s) {
+    expected_keylist = parse_source_string(s);
+  }
+
+  std::u16string expected_keylist;
 
   /** returns false on fail and updates the message */
   bool handle_check_keylist(std::string &message) const;

From feba97979a11b0ba506e4f9516edbb4246036acd Mon Sep 17 00:00:00 2001
From: "Steven R. Loomis" <srl295@gmail.com>
Date: Fri, 6 Dec 2024 12:24:53 -0600
Subject: [PATCH 11/13] fix(core): update get_key_list to account for other
 combinatorics

- yes, expand 'other' to all possible combinations
- use ALT  and CTRL instead of RALT,LALT and RCTRL,LCTRL in the key list (reduce expansions up to 4x)

Fixes: #12298
---
 core/src/ldml/ldml_vkeys.cpp                  | 43 +++++++++++++++----
 .../unit/ldml/keyboards/k_004_tinyshift.xml   |  2 +-
 core/tests/unit/ldml/ldml.cpp                 |  2 +-
 3 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp
index 2815eb2ab4..dc3f22f18b 100644
--- a/core/src/ldml/ldml_vkeys.cpp
+++ b/core/src/ldml/ldml_vkeys.cpp
@@ -47,8 +47,7 @@ vkeys::get_key_list() const {
   // Alt
   if (all_modifiers.count(LDML_KEYS_MOD_ALT) == 0 && all_modifiers.count(LDML_KEYS_MOD_ALTL) == 0 && all_modifiers.count(LDML_KEYS_MOD_ALTR) == 0) {
     // no ALT keys were seen, so OTHER includes ALT
-    other_state.insert(KM_CORE_MODIFIER_LALT);
-    other_state.insert(KM_CORE_MODIFIER_RALT);
+    other_state.insert(KM_CORE_MODIFIER_ALT);
   } else if(all_modifiers.count(LDML_KEYS_MOD_ALTL) == 0) {
     other_state.insert(KM_CORE_MODIFIER_RALT);
   } else if(all_modifiers.count(LDML_KEYS_MOD_ALTR) == 0) {
@@ -58,8 +57,7 @@ vkeys::get_key_list() const {
   // ctrl
   if (all_modifiers.count(LDML_KEYS_MOD_CTRL) == 0 && all_modifiers.count(LDML_KEYS_MOD_CTRLL) == 0 && all_modifiers.count(LDML_KEYS_MOD_CTRLR) == 0) {
     // no CTRL keys were seen, so OTHER includes CTRL
-    other_state.insert(KM_CORE_MODIFIER_LCTRL);
-    other_state.insert(KM_CORE_MODIFIER_RCTRL);
+    other_state.insert(KM_CORE_MODIFIER_CTRL);
   } else if(all_modifiers.count(LDML_KEYS_MOD_CTRLL) == 0) {
     other_state.insert(KM_CORE_MODIFIER_RCTRL);
   } else if(all_modifiers.count(LDML_KEYS_MOD_CTRLR) == 0) {
@@ -81,9 +79,35 @@ vkeys::get_key_list() const {
     other_state.insert(KM_CORE_MODIFIER_NONE);
   }
 
-  const std::size_t new_list_size = all_vkeys.size() // original size
-    + (other_key_count * (other_state.size() - 1))
-    + 1; // terminator
+  // We need ALL combinations of the other_state, except for 'all off'.
+  // The number of additions will be (2**(other_state.size())-1
+  // Also, since the LDML_KEYS_MOD_OTHER modifier is excluded, we
+  // will need to subtract 1 when calculating new_list_size
+  const std::size_t other_expanded_count = (1 << other_state.size()) - 1;
+
+  std::vector<uint32_t> other_expanded_mods(other_expanded_count);
+
+  // populate the expanded list.
+  // we start at 1 because 0 is "all bits off" (00000b)
+  for (std::size_t expansion = 1; expansion <= other_expanded_count; expansion++) {
+    uint32_t &expanded_mod = other_expanded_mods.at(expansion-1) = KM_CORE_MODIFIER_NONE;
+    std::size_t bit_mask = 1;
+    for (const auto mod : other_state) {
+      // Check if this modifier is on in this iteration of the expansion
+      // bit_mask will be 2^0 … 2^(other_state().size()-1)
+      if (bit_mask & expansion) {
+        // do we include this entry? check if this bit is on
+        expanded_mod |= mod;
+      }
+      // shift the bitmask over
+      assert(expanded_mod <= KM_CORE_MODIFIER_MASK_CAPS);
+      bit_mask <<= 1;
+    }
+  }
+
+  const std::size_t new_list_size = all_vkeys.size()                                // original size
+                                    + (other_key_count * (other_expanded_count - 1))// number of additional entries needed
+                                    + 1;                                            // terminator
   km_core_keyboard_key *list = new km_core_keyboard_key[new_list_size];
   std::size_t n = 0;
   for (const auto &k : all_vkeys) {
@@ -92,18 +116,19 @@ vkeys::get_key_list() const {
 
     if (mod == LDML_KEYS_MOD_OTHER) {
       // expand to all of other_state
-      for (const auto expanded_mod : other_state) {
+      for (const auto expanded_mod : other_expanded_mods) {
         list[n].key             = vkey;
         list[n++].modifier_flag = expanded_mod;
         assert(n <= new_list_size);
       }
     } else {
-      assert(mod <= KM_CORE_MODIFIER_CAPS); // that no LDMLisms escape
+      assert(mod <= KM_CORE_MODIFIER_MASK_CAPS); // that no LDMLisms escape
       list[n].key             = vkey;
       list[n++].modifier_flag = mod;
       assert(n <= new_list_size);
     }
   }
+  // add the list terminator
   list[n++] = KM_CORE_KEYBOARD_KEY_LIST_END;
   assert(n == new_list_size);
   return list;
diff --git a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
index a2f300dc10..2d152867a7 100644
--- a/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
+++ b/core/tests/unit/ldml/keyboards/k_004_tinyshift.xml
@@ -4,7 +4,7 @@
 
 @@keys: [SHIFT K_BKQUOTE][K_1][K_BKQUOTE][LCTRL K_BKQUOTE][RALT K_BKQUOTE]
 @@expected: \u0037\u1790\u17B6\u0127\u1790\u17B6\u0065
-@@keylist: [SHIFT K_BKQUOTE][SHIFT K_1][K_BKQUOTE][K_1][CTRL-do-not-use K_1][LALT K_BKQUOTE][RALT K_BKQUOTE]
+@@keylist: [SHIFT K_BKQUOTE][SHIFT K_1][K_BKQUOTE][K_1][CTRL-do-not-use K_1][ALT-do-not-use K_BKQUOTE][CAPS K_BKQUOTE][ALT-do-not-use CAPS K_BKQUOTE]
 
 -->
 <keyboard3 xmlns="https://schemas.unicode.org/cldr/45/keyboard3" locale="mt" conformsTo="45">
diff --git a/core/tests/unit/ldml/ldml.cpp b/core/tests/unit/ldml/ldml.cpp
index fed2cefac1..f0b1eac4fe 100644
--- a/core/tests/unit/ldml/ldml.cpp
+++ b/core/tests/unit/ldml/ldml.cpp
@@ -280,7 +280,7 @@ verify_key_list(std::set<km::tests::key_event> &actual, std::set<km::tests::key_
   bool equals = true;
   // error if any bad modifier keys
   for(const auto &akey : actual) {
-    if (akey.modifier_state > KM_CORE_MODIFIER_CAPS) {
+    if (akey.modifier_state > KM_CORE_MODIFIER_MASK_CAPS) {
       equals = false;
       std::u16string dump = convert<char, char16_t>(akey.dump());  // akey.dump()
       std::wcout << console_color::fg(console_color::BRIGHT_RED) << "- FAIL - key_map had key with bad modifier " << akey.modifier_state << ": " << dump << console_color::reset() << std::endl;

From cb8ce445f97f964df6d6d4fa3fa005a95a7476f9 Mon Sep 17 00:00:00 2001
From: Marc Durdin <marc@durdin.net>
Date: Mon, 9 Dec 2024 10:43:53 +0700
Subject: [PATCH 12/13] chore(developer): add some docs for language examples
 in kmp.json

Fixes: #12657
---
 .../help/reference/file-types/metadata.md     | 51 +++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/developer/docs/help/reference/file-types/metadata.md b/developer/docs/help/reference/file-types/metadata.md
index 8a3e50deae..1a0d631820 100644
--- a/developer/docs/help/reference/file-types/metadata.md
+++ b/developer/docs/help/reference/file-types/metadata.md
@@ -243,6 +243,12 @@ The `Keyboard` object describes an individual keyboard in the Keyman package. A
 
   The filename of the font for the OSK
 
+`examples`
+
+: `Array`
+
+  An array of [`Example`](#obj-example) objects linked to the keyboard.
+
 ### The Language object
 
 The `Language` object describes the language that can be typed with the keyboard
@@ -259,6 +265,51 @@ The `Language` object describes the language that can be typed with the keyboard
 
   [BCP 47 language code](../bcp-47)
 
+### The Example object
+
+The `Example` object describes a keying sequence example text for the keyboard.
+This can be the easiest way for a new user to start using a keyboard, and is
+particularly helpful when the keyboard makes use of keying sequences that may
+not be immediately obvious.
+
+`id`
+
+: `string`
+
+  The BCP 47 language code for the example.
+
+`keys`
+
+: `string`
+
+  The key sequence to type the example. The key sequence must list each key
+  combination, separated by space. The actual text for each key is reasonably
+  arbitrary, to allow you to provide examples for touch keyboards as well as
+  desktop keyboards. There are three special kinds of key strings:
+  * Modifier keys may be specified with the `+` character, e.g. `shift+e` or
+    `right-alt+k`. Use lower case keys. The suggested standard modifiers are:
+    `shift`, `ctrl`, `alt`, `left-alt`, `right-alt`, `left-ctrl`, `right-ctrl`,
+    and `option` (mac).
+  * The <kbd>space</kbd> key itself may be specified with `space`.
+  * To avoid confusion with modifier keys, the <kbd>+</kbd> key can be specified
+    with `plus`.
+
+  For example, the key sequence <kbd>x</kbd>, <kbd>j</kbd>, <kbd>m</kbd>,
+  <kbd>Shift</kbd>+<kbd>e</kbd>, <kbd>r</kbd> may be specified as
+  `x j m shift+e r` or `x j m E r`.
+
+`text`
+
+: `string`
+
+  The expected output when the `keys` are typed
+
+`note`
+
+: `string`
+
+  A brief explanation of the example, e.g. "Name of language"
+
 ### The LexicalModel object
 
 The `LexicalModel` object describes an individual model in the Keyman package. A package cannot contain both lexical models and keyboards.

From ff9ee3dfee3ab8dcd566f1a3e7bcb8d137ff6dcd Mon Sep 17 00:00:00 2001
From: Keyman Build Agent <buildagent@keyman.com>
Date: Mon, 9 Dec 2024 13:02:17 -0500
Subject: [PATCH 13/13] auto: increment master version to 18.0.157

---
 HISTORY.md | 10 ++++++++++
 VERSION.md |  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/HISTORY.md b/HISTORY.md
index d008db3bb7..6cffd0589a 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -1,5 +1,15 @@
 # Keyman Version History
 
+## 18.0.156 alpha 2024-12-09
+
+* fix(developer): remove platforms from kmc-generate LM readme (#12803)
+* test(developer): add test for ERROR_DescriptionIsMissing to kmc-keyboard-info (#12804)
+* chore(developer): verify bundled node version when building installer (#12806)
+* fix(developer): support hint property in displaymap (#12807)
+* fix(common/web): delete replaceExtension in types/src/util/file-types.ts (#12762)
+* chore(developer): add some docs for language examples in kmp.json (#12805)
+* fix(core): implement ldml_processor::get_key_list() (#12644)
+
 ## 18.0.155 alpha 2024-12-07
 
 * chore(android,windows): Update crowdin for Czech (#12792)
diff --git a/VERSION.md b/VERSION.md
index d8dc6af9c7..4f4515d945 100644
--- a/VERSION.md
+++ b/VERSION.md
@@ -1 +1 @@
-18.0.156
\ No newline at end of file
+18.0.157
\ No newline at end of file