From 11334ed7faeba1e62ef04690fb8d6a71a8adeb7a Mon Sep 17 00:00:00 2001 From: "Steven R. Loomis" Date: Tue, 28 Nov 2023 18:07:28 -0600 Subject: [PATCH] =?UTF-8?q?feat(core):=20ldml=20improve=20key-not-found=20?= =?UTF-8?q?=F0=9F=99=80?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 | 8 ++- core/src/ldml/ldml_vkeys.cpp | 49 ++++++++-------- core/src/ldml/ldml_vkeys.hpp | 9 +-- .../unit/ldml/keyboards/k_102_keytest.xml | 4 +- core/tests/unit/ldml/test_kmx_plus.cpp | 56 +++++++++++-------- 5 files changed, 71 insertions(+), 55 deletions(-) diff --git a/core/src/ldml/ldml_processor.cpp b/core/src/ldml/ldml_processor.cpp index 0c36e5c0aa..7f6a0b0969 100644 --- a/core/src/ldml/ldml_processor.cpp +++ b/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); } } diff --git a/core/src/ldml/ldml_vkeys.cpp b/core/src/ldml/ldml_vkeys.cpp index 3d5123d709..156a4c3247 100644 --- a/core/src/ldml/ldml_vkeys.cpp +++ b/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; } diff --git a/core/src/ldml/ldml_vkeys.hpp b/core/src/ldml/ldml_vkeys.hpp index 2b85138f40..22dea817ef 100644 --- a/core/src/ldml/ldml_vkeys.hpp +++ b/core/src/ldml/ldml_vkeys.hpp @@ -29,7 +29,6 @@ typedef std::pair vkey_id; */ class vkeys { private: - // TODO-LDML: store transform=no state std::map 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 diff --git a/core/tests/unit/ldml/keyboards/k_102_keytest.xml b/core/tests/unit/ldml/keyboards/k_102_keytest.xml index 9441d980af..dd0e5d3533 100644 --- a/core/tests/unit/ldml/keyboards/k_102_keytest.xml +++ b/core/tests/unit/ldml/keyboards/k_102_keytest.xml @@ -1,7 +1,7 @@ @@ -11,7 +11,7 @@ - + diff --git a/core/tests/unit/ldml/test_kmx_plus.cpp b/core/tests/unit/ldml/test_kmx_plus.cpp index ce80c2bf2d..fc3f7e1ffe 100644 --- a/core/tests/unit/ldml/test_kmx_plus.cpp +++ b/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; }