feat(core): ldml improve key-not-found 🙀

- distinguish between unmapped and 0-length output strings
- don't do any processing for 0-length output strings
- no change to developer, that will be next
- remove a comment referencing transform=no

For: #9451

core/src/ldml/ldml_processor.cpp

@@ -252,13 +252,15 @@ ldml_processor::process_backspace(km_core_state *state) const {
 void
 ldml_processor::process_key(km_core_state *state, km_core_virtual_key vk, uint16_t modifier_state) const {
   // Look up the key
-  const std::u16string key_str = keys.lookup(vk, modifier_state);
+  bool found = false;
+  const std::u16string key_str = keys.lookup(vk, modifier_state, found);
 
-  if (key_str.empty()) {
+  if (!found) {
     // no key was found, so pass the keystroke on to the Engine
     state->actions().push_invalidate_context();
     state->actions().push_emit_keystroke();
-  } else {
+  } else if (!key_str.empty()) {
+    // TODO-LDML: skip processing empty (gap) keys?
     process_key_string(state, key_str);
   }
 }

core/src/ldml/ldml_vkeys.cpp

@@ -27,12 +27,12 @@ static const uint16_t BOTH_ALT  = LALTFLAG  | RALTFLAG;
 static const uint16_t BOTH_CTRL = LCTRLFLAG | RCTRLFLAG;
 
 std::u16string
-vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state) const {
+vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) const {
   const vkey_id id(vk, modifier_state);
 
   // try exact match first
-  std::u16string ret = lookup(id);
-  if (!ret.empty()) {
+  std::u16string ret = lookup(id, found);
+  if (found) {
     return ret;
   }
 
@@ -42,38 +42,41 @@ vkeys::lookup(km_core_virtual_key vk, uint16_t modifier_state) const {
   // look for a layer with "alt" (either)
   if (have_alt) {
     const vkey_id id_alt(vk, (modifier_state & ~(BOTH_ALT)) | K_ALTFLAG);
-    ret = lookup(id_alt);
-    if (!ret.empty()) {
-      return ret;
-    }
-  }
-  // look for a layer with "ctrl" (either)
-  if (have_ctrl) {
-    const vkey_id id_ctrl(vk, (modifier_state & ~(BOTH_CTRL)) | K_CTRLFLAG);
-    ret = lookup(id_ctrl);
-    if (!ret.empty()) {
-      return ret;
-    }
-  }
-  // look for a layer with "alt ctrl" (either)
-  if (have_ctrl && have_alt) {
-    const vkey_id id_ctrl_alt(vk, (modifier_state & ~(BOTH_ALT | BOTH_CTRL)) | K_CTRLFLAG | K_ALTFLAG);
-    ret = lookup(id_ctrl_alt);
-    if (!ret.empty()) {
+    ret = lookup(id_alt, found);
+    if (found) {
       return ret;
     }
   }
 
-  // default: return failure.
+  // look for a layer with "ctrl" (either)
+  if (have_ctrl) {
+    const vkey_id id_ctrl(vk, (modifier_state & ~(BOTH_CTRL)) | K_CTRLFLAG);
+    ret = lookup(id_ctrl, found);
+    if (found) {
+      return ret;
+    }
+  }
+
+  // look for a layer with "alt ctrl" (either)
+  if (have_ctrl && have_alt) {
+    const vkey_id id_ctrl_alt(vk, (modifier_state & ~(BOTH_ALT | BOTH_CTRL)) | K_CTRLFLAG | K_ALTFLAG);
+    ret = lookup(id_ctrl_alt, found);
+    if (found) {
+      return ret;
+    }
+  }
+  // default: return failure. found=false.
   return ret;
 }
 
 std::u16string
-vkeys::lookup(const vkey_id& id) const {
+vkeys::lookup(const vkey_id& id, bool &found) const {
   const auto key = vkey_to_string.find(id);
   if (key == vkey_to_string.end()) {
+    found = false;
     return std::u16string();  // TODO-LDML: optimize object construction?
   }
+  found = true;
   return key->second;
 }
 

core/src/ldml/ldml_vkeys.hpp

@@ -29,7 +29,6 @@ typedef std::pair<km_core_virtual_key, uint16_t> vkey_id;
  */
 class vkeys {
 private:
-  // TODO-LDML: store transform=no state
   std::map<vkey_id, std::u16string> vkey_to_string;
 
 public:
@@ -42,16 +41,18 @@ public:
 
   /**
    * Lookup a vkey, returns an empty string if not found
+   * @param found on exit: true if found
    */
   std::u16string
-  lookup(km_core_virtual_key vk, uint16_t modifier_state) const;
+  lookup(km_core_virtual_key vk, uint16_t modifier_state, bool &found) const;
 
 private:
   /**
    * Non-recursive internal lookup of a specific ID
-  */
+   * @param found on exit: true if found
+   */
   std::u16string
-  lookup(const vkey_id &id) const;
+  lookup(const vkey_id &id, bool &found) const;
 };
 
 }  // namespace ldml

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

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 
 <!--
-@@keys: [K_BKQUOTE]
+@@keys: [K_BKQUOTE][K_1][K_ENTER]
 @@expected: \u0061
 -->
 
@@ -11,7 +11,7 @@
   <!-- only implied keys - the <keys/> element is omitted. -->
   <layers formId="us">
     <layer id="base">
-      <row keys="a" /> <!-- number row -->
+      <row keys="a gap" /> <!-- number row -->
     </layer>
   </layers>
 

core/tests/unit/ldml/test_kmx_plus.cpp

@@ -85,61 +85,71 @@ int test_ldml_vkeys() {
   ADD_VKEY("K_E", K_ALTFLAG|RCTRLFLAG);
 
 #undef ADD_VKEY
+  vk.add(km::tests::get_vk("K_F"), 0, u""); // K_F as a 'gap' key
+
+  bool found = false;
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_A"), 0), u"K_A-0");
+    "K_F"), 0, found), u"");
+  assert_equal(found, true); // K_F found, but empty string (gap)
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_A"), LCTRLFLAG), u"K_A-LCTRLFLAG");
+    "K_ENTER"), 0, found), u"");
+  assert_equal(found, false); // K_ENTER not found, empty string
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_A"), RCTRLFLAG), u"K_A-RCTRLFLAG");
+    "K_A"), 0, found), u"K_A-0");
+  assert_equal(found, true); // expect
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_A"), LALTFLAG), u"K_A-LALTFLAG");
+    "K_A"), LCTRLFLAG, found), u"K_A-LCTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_A"), RALTFLAG), u"K_A-RALTFLAG");
+    "K_A"), RCTRLFLAG, found), u"K_A-RCTRLFLAG");
+  assert_equal(vk.lookup(km::tests::get_vk(
+    "K_A"), LALTFLAG, found), u"K_A-LALTFLAG");
+  assert_equal(vk.lookup(km::tests::get_vk(
+    "K_A"), RALTFLAG, found), u"K_A-RALTFLAG");
 
   // now try either-side keys :should get the same result with either or both
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), LCTRLFLAG), u"K_B-K_CTRLFLAG");
+    "K_B"), LCTRLFLAG, found), u"K_B-K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), RCTRLFLAG), u"K_B-K_CTRLFLAG");
+    "K_B"), RCTRLFLAG, found), u"K_B-K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), LCTRLFLAG|RCTRLFLAG), u"K_B-K_CTRLFLAG");
+    "K_B"), LCTRLFLAG|RCTRLFLAG, found), u"K_B-K_CTRLFLAG");
 
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), LALTFLAG), u"K_B-K_ALTFLAG");
+    "K_B"), LALTFLAG, found), u"K_B-K_ALTFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), RALTFLAG), u"K_B-K_ALTFLAG");
+    "K_B"), RALTFLAG, found), u"K_B-K_ALTFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_B"), LALTFLAG|RALTFLAG), u"K_B-K_ALTFLAG");
+    "K_B"), LALTFLAG|RALTFLAG, found), u"K_B-K_ALTFLAG");
 
   // OOOkay now try BOTH side
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_C"), LCTRLFLAG|LALTFLAG), u"K_C-K_ALTFLAG|K_CTRLFLAG");
+    "K_C"), LCTRLFLAG|LALTFLAG, found), u"K_C-K_ALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_C"), LCTRLFLAG|RALTFLAG), u"K_C-K_ALTFLAG|K_CTRLFLAG");
+    "K_C"), LCTRLFLAG|RALTFLAG, found), u"K_C-K_ALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_C"), RCTRLFLAG|LALTFLAG), u"K_C-K_ALTFLAG|K_CTRLFLAG");
+    "K_C"), RCTRLFLAG|LALTFLAG, found), u"K_C-K_ALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_C"), RCTRLFLAG|RALTFLAG), u"K_C-K_ALTFLAG|K_CTRLFLAG");
+    "K_C"), RCTRLFLAG|RALTFLAG, found), u"K_C-K_ALTFLAG|K_CTRLFLAG");
 
   // OOOkay now try either alt
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_D"), LCTRLFLAG|LALTFLAG), u"K_D-LALTFLAG|K_CTRLFLAG");
+    "K_D"), LCTRLFLAG|LALTFLAG, found), u"K_D-LALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_D"), LCTRLFLAG|RALTFLAG), u"K_D-RALTFLAG|K_CTRLFLAG");
+    "K_D"), LCTRLFLAG|RALTFLAG, found), u"K_D-RALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_D"), RCTRLFLAG|LALTFLAG), u"K_D-LALTFLAG|K_CTRLFLAG");
+    "K_D"), RCTRLFLAG|LALTFLAG, found), u"K_D-LALTFLAG|K_CTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_D"), RCTRLFLAG|RALTFLAG), u"K_D-RALTFLAG|K_CTRLFLAG");
+    "K_D"), RCTRLFLAG|RALTFLAG, found), u"K_D-RALTFLAG|K_CTRLFLAG");
 
   // OOOkay now try either ctrl
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_E"), LCTRLFLAG|LALTFLAG), u"K_E-K_ALTFLAG|LCTRLFLAG");
+    "K_E"), LCTRLFLAG|LALTFLAG, found), u"K_E-K_ALTFLAG|LCTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_E"), LCTRLFLAG|RALTFLAG), u"K_E-K_ALTFLAG|LCTRLFLAG");
+    "K_E"), LCTRLFLAG|RALTFLAG, found), u"K_E-K_ALTFLAG|LCTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_E"), RCTRLFLAG|LALTFLAG), u"K_E-K_ALTFLAG|RCTRLFLAG");
+    "K_E"), RCTRLFLAG|LALTFLAG, found), u"K_E-K_ALTFLAG|RCTRLFLAG");
   assert_equal(vk.lookup(km::tests::get_vk(
-    "K_E"), RCTRLFLAG|RALTFLAG), u"K_E-K_ALTFLAG|RCTRLFLAG");
+    "K_E"), RCTRLFLAG|RALTFLAG, found), u"K_E-K_ALTFLAG|RCTRLFLAG");
 
   return EXIT_SUCCESS;
 }