From 796a2d4c859522476d712381db7b5ab5d054a999 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 26 Jul 2021 15:23:20 +1000 Subject: [PATCH 1/5] chore(common/core): handle deletion of markers in actions Fixes #5488. This updates Keyman Core, corresponding tests, and Keyman Engine for Linux to support deletion of markers through an action, ensuring that the action queue does not desynchronize with the context. --- .../include/keyman/keyboardprocessor.h.in | 13 ++- .../core/desktop/src/km_kbp_context_api.cpp | 5 ++ common/core/desktop/src/kmx/kmx_processor.cpp | 41 +++++++-- .../core/desktop/src/mock/mock_processor.cpp | 2 +- common/core/desktop/src/state.hpp | 9 +- .../tests/unit/kmnkbd/action_items.hpp | 10 ++- .../desktop/tests/unit/kmnkbd/debug_api.cpp | 47 ++++++++-- .../unit/kmx/019 - multiple deadkeys.kmn | 18 ++-- .../unit/kmx/019 - multiple deadkeys.kmx | Bin 1862 -> 2264 bytes common/core/desktop/tests/unit/kmx/kmx.cpp | 82 +++++++++++++++--- linux/ibus-keyman/src/engine.c | 6 +- 11 files changed, 188 insertions(+), 45 deletions(-) diff --git a/common/core/desktop/include/keyman/keyboardprocessor.h.in b/common/core/desktop/include/keyman/keyboardprocessor.h.in index 430dc0b455..138ce065ef 100644 --- a/common/core/desktop/include/keyman/keyboardprocessor.h.in +++ b/common/core/desktop/include/keyman/keyboardprocessor.h.in @@ -471,13 +471,20 @@ services framework to transform the text store in the Client Application, among other actions. ```c */ + +typedef struct { + uint8_t expected_type; /// one of KM_KBP_IT_CHAR, KM_KBP_IT_MARKER, KM_KBP_IT_END (when type to delete is unknown) + uintptr_t expected_value; /// used mainly in unit tests +} km_kbp_backspace_item; + typedef struct { uint8_t type; uint8_t _reserved[sizeof(void*)-sizeof(uint8_t)]; union { - uintptr_t marker; // MARKER type - km_kbp_option_item const * option; // OPT types - km_kbp_usv character; // CHAR type + uintptr_t marker; // MARKER type + km_kbp_option_item const * option; // OPT types + km_kbp_usv character; // CHAR type + km_kbp_backspace_item backspace; }; } km_kbp_action_item; diff --git a/common/core/desktop/src/km_kbp_context_api.cpp b/common/core/desktop/src/km_kbp_context_api.cpp index 964f7867b4..f70017ae96 100644 --- a/common/core/desktop/src/km_kbp_context_api.cpp +++ b/common/core/desktop/src/km_kbp_context_api.cpp @@ -76,6 +76,11 @@ namespace { *sz_ptr = buf_size - (e - i); + // Skip over any final markers - they are execluded from context + while(ci->type == KM_KBP_CT_MARKER) { + ci++; + } + return ci->type == KM_KBP_CT_END ? KM_KBP_STATUS_OK : KM_KBP_STATUS_INSUFFICENT_BUFFER; diff --git a/common/core/desktop/src/kmx/kmx_processor.cpp b/common/core/desktop/src/kmx/kmx_processor.cpp index 18ec00d067..c2746cc31c 100644 --- a/common/core/desktop/src/kmx/kmx_processor.cpp +++ b/common/core/desktop/src/kmx/kmx_processor.cpp @@ -135,24 +135,47 @@ namespace km { case BK_DEFAULT: // This only happens if we know we have context to delete. Last item must be a character assert(!state->context().empty() && state->context().back().type != KM_KBP_IT_MARKER); - if (!state->context().empty()) state->context().pop_back(); - state->actions().push_backspace(); + if(!state->context().empty()) { + auto item = state->context().back(); + state->context().pop_back(); + state->actions().push_backspace(KM_KBP_IT_CHAR, item.character); + } else { + // Note: only runs on non-debug build, fail safe + state->actions().push_backspace(KM_KBP_IT_END); + } break; case BK_DEADKEY: // This only happens if we know we have context to delete. Last item must be a deadkey assert(!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER); - if (!state->context().empty()) state->context().pop_back(); + if(!state->context().empty()) { + auto item = state->context().back(); + state->context().pop_back(); + state->actions().push_backspace(KM_KBP_IT_MARKER, item.marker); + } else { + // Note: only runs on non-debug build, fail safe + state->actions().push_backspace(KM_KBP_IT_END); + } + break; case BK_BACKSPACE: // User-initiated backspace. We need to delete deadkeys from context, both sides of the character deleted - while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) state->context().pop_back(); - if (!state->context().empty()) { + while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { + state->actions().push_backspace(KM_KBP_IT_MARKER, state->context().back().marker); + state->context().pop_back(); + } + if (!state->context().empty()) { + state->actions().push_backspace(KM_KBP_IT_CHAR, state->context().back().character); + state->context().pop_back(); + } else { + // Even if context is empty, we send the backspace event, because we may not + // know the context. + state->actions().push_backspace(KM_KBP_IT_END); + } + // Delete deadkey markers prior to char + while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { + state->actions().push_backspace(KM_KBP_IT_MARKER, state->context().back().marker); state->context().pop_back(); - while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) state->context().pop_back(); } - // Even if context is empty, we send the backspace event, because we may not - // know the context. - state->actions().push_backspace(); break; default: assert(false); diff --git a/common/core/desktop/src/mock/mock_processor.cpp b/common/core/desktop/src/mock/mock_processor.cpp index d12e8e302b..1494512a64 100644 --- a/common/core/desktop/src/mock/mock_processor.cpp +++ b/common/core/desktop/src/mock/mock_processor.cpp @@ -119,7 +119,7 @@ namespace km { { case KM_KBP_VKEY_BKSP: state->context().pop_back(); - state->actions().push_backspace(); + state->actions().push_backspace(KM_KBP_IT_END); // Assuming we don't know the character break; case KM_KBP_VKEY_F2: diff --git a/common/core/desktop/src/state.hpp b/common/core/desktop/src/state.hpp index 95ec85e7a3..c95d2545df 100644 --- a/common/core/desktop/src/state.hpp +++ b/common/core/desktop/src/state.hpp @@ -39,7 +39,7 @@ public: void push_character(km_kbp_usv usv); void push_marker(uintptr_t marker); void push_alert(); - void push_backspace(); + void push_backspace(km_kbp_action_type expected_type, uintptr_t expected_value = 0); void push_persist(option const &); void push_persist(option const &&); void push_emit_keystroke(km_kbp_virtual_key vk=0); @@ -81,9 +81,12 @@ void actions::push_alert() { inline -void actions::push_backspace() { +void actions::push_backspace(km_kbp_action_type expected_type, uintptr_t expected_value) { assert(empty() || (!empty() && back().type != KM_KBP_IT_END)); - emplace_back(km_kbp_action_item {KM_KBP_IT_BACK, {0,}, {0}}); + km_kbp_action_item item = {KM_KBP_IT_BACK}; + item.backspace.expected_type = expected_type; + item.backspace.expected_value = expected_value; + emplace_back(item); } diff --git a/common/core/desktop/tests/unit/kmnkbd/action_items.hpp b/common/core/desktop/tests/unit/kmnkbd/action_items.hpp index 3edcdc400f..a5cf414231 100644 --- a/common/core/desktop/tests/unit/kmnkbd/action_items.hpp +++ b/common/core/desktop/tests/unit/kmnkbd/action_items.hpp @@ -38,6 +38,13 @@ void print_action_item(const char *title, km_kbp_action_item const & item) { case KM_KBP_IT_CHAR: std::cout << " char: '" << std::u32string(1, item.character) << "' (" << item.character << ")" << std::endl; break; + case KM_KBP_IT_BACK: + std::cout << " delete: " << + (item.backspace.expected_type == KM_KBP_IT_CHAR ? "char" : + item.backspace.expected_type == KM_KBP_IT_MARKER ? "marker" : + "unknown") << " (" << + item.backspace.expected_value << ")" << std::endl; + break; case KM_KBP_IT_PERSIST_OPT: std::cout << " option: key: " << item.option->key << std::endl @@ -58,7 +65,8 @@ bool operator==(km_kbp_action_item const & lhs, case KM_KBP_IT_CHAR: result = lhs.character == rhs.character; break; case KM_KBP_IT_MARKER: result = lhs.marker == rhs.marker; break; case KM_KBP_IT_ALERT: break; - case KM_KBP_IT_BACK: break; + case KM_KBP_IT_BACK: result = lhs.backspace.expected_type == rhs.backspace.expected_type && + lhs.backspace.expected_value == rhs.backspace.expected_value; break; case KM_KBP_IT_PERSIST_OPT: result = *lhs.option == *rhs.option; break; case KM_KBP_IT_EMIT_KEYSTROKE: break; case KM_KBP_IT_INVALIDATE_CONTEXT: break; diff --git a/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp b/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp index e6353e6cbd..f764a8c715 100644 --- a/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp +++ b/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp @@ -174,9 +174,17 @@ void test_basic_rule_matches() { km_kbp_state_debug_item{KM_KBP_DEBUG_END, 0, {}, {u"", nullptr, nullptr, {}, 5 /* from above */ }}, })); + km_kbp_action_item bksp_d = {KM_KBP_IT_BACK}; + bksp_d.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_d.backspace.expected_value = 'D'; + + km_kbp_action_item bksp_e = {KM_KBP_IT_BACK}; + bksp_e.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_e.backspace.expected_value = 'E'; + assert(action_items(test_state, { - {KM_KBP_IT_BACK}, - {KM_KBP_IT_BACK}, + bksp_e, + bksp_d, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv(u'\u0E04')}}, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv(u'\u0E05')}}, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv(u'\u0E06')}}, @@ -219,9 +227,13 @@ void test_multiple_groups() { km_kbp_state_debug_item{KM_KBP_DEBUG_END, 0, {}, {u"", nullptr, nullptr, {}, 3}}, // action item will emit a 'b' })); + km_kbp_action_item bksp_a = {KM_KBP_IT_BACK}; + bksp_a.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_a.backspace.expected_value = 'a'; + assert(action_items(test_state, { {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('a')}}, - {KM_KBP_IT_BACK}, + bksp_a, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('b')}}, {KM_KBP_IT_END} })); @@ -248,8 +260,12 @@ void test_multiple_groups() { km_kbp_state_debug_item{KM_KBP_DEBUG_END, 0, {}, {u"", nullptr, nullptr, {}, 4}}, // action item will "abc" })); + km_kbp_action_item bksp_b = {KM_KBP_IT_BACK}; + bksp_b.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_b.backspace.expected_value = 'b'; + assert(action_items(test_state, { - {KM_KBP_IT_BACK}, + bksp_b, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('a')}}, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('b')}}, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('c')}}, @@ -306,11 +322,26 @@ void test_store_offsets() { km_kbp_state_debug_item{KM_KBP_DEBUG_END, 0, {}, {u"", nullptr, nullptr, {}, 6}}, })); + km_kbp_action_item bksp[] = { + {KM_KBP_IT_BACK}, + {KM_KBP_IT_BACK}, + {KM_KBP_IT_BACK}, + {KM_KBP_IT_BACK} + }; + bksp[0].backspace.expected_type = KM_KBP_IT_CHAR; + bksp[1].backspace.expected_type = KM_KBP_IT_CHAR; + bksp[2].backspace.expected_type = KM_KBP_IT_CHAR; + bksp[3].backspace.expected_type = KM_KBP_IT_CHAR; + bksp[0].backspace.expected_value = 'y'; + bksp[1].backspace.expected_value = 'a'; + bksp[2].backspace.expected_value = 'x'; + bksp[3].backspace.expected_value = 'e'; + assert(action_items(test_state, { - {KM_KBP_IT_BACK}, - {KM_KBP_IT_BACK}, - {KM_KBP_IT_BACK}, - {KM_KBP_IT_BACK}, + bksp[0], + bksp[1], + bksp[2], + bksp[3], {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('e')}}, {KM_KBP_IT_CHAR, {0,}, {km_kbp_usv('x')}}, {KM_KBP_IT_END} diff --git a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn index 07c4ad072a..ce3f46e940 100644 --- a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn +++ b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn @@ -7,15 +7,16 @@ c 5. One char and two deadkeys in context 'a' dk(5a) dk(5b) + '5' c 6. One char and two deadkeys and one char in context 'a' dk(6a) dk(6b) 'b' + '6' c 7. Two deadkeys and one char in context dk(7a) dk(7b) 'a' + '7' c 8. Three deadkeys in context dk(8a) dk(8b) dk(8c) + '8' -c keys: [K_1][K_X][K_2][K_X][K_3][K_X][K_4][K_X][K_5][K_X][K_6][K_X][K_7][K_X][K_8][K_X] -c expected: 1=OK 2=OK 3=OK 4=OK 5=OK 6=OK 7=OK 8=OK -c context: +c 9. Replacing deadkeys in context via rule [9][x][x] +c keys: [K_1][K_X][K_2][K_X][K_3][K_X][K_4][K_X][K_5][K_X][K_6][K_X][K_7][K_X][K_8][K_X][K_9][K_X][K_X] +c expected: 1=OK 2=OK 3=OK 4=OK 5=OK 6=OK 7=OK 8=OK 9=OK +c context: store(&VERSION) '9.0' begin unicode > use(main) -group(main) using keys +group(main) using keys + '1' > dk(1) dk(1) + 'x' > '1=OK ' @@ -41,10 +42,17 @@ dk(7a) 'a' + 'x' > '7=Fail1 ' dk(7b) 'a' + 'x' > '7=Fail2 ' + '8' > dk(8a) dk(8b) dk(8c) -dk(8a) dk(8b) dk(8c) + 'x' > '8=OK' +dk(8a) dk(8b) dk(8c) + 'x' > '8=OK ' dk(8a) + 'x' > '8=Fail1 ' dk(8b) + 'x' > '8=Fail2 ' dk(8c) + 'x' > '8=Fail3 ' dk(8a) dk(8b) + 'x' > '8=Fail4 ' dk(8a) dk(8c) + 'x' > '8=Fail5 ' dk(8b) dk(8c) + 'x' > '8=Fail6 ' + +c [K_9] [K_X] [K_X] -> test that + ++ '9' > '<' dk(9a) dk(9b) '>' dk(9c) dk(9d) + +dk(9b) '>' dk(9c) dk(9d) + 'x' > dk(9e) '>' dk(9f) dk(9g) +'<' dk(9a) dk(9e) '>' dk(9f) dk(9g) + 'x' > '9=OK' \ No newline at end of file diff --git a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmx b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmx index 5d21325f874b6bcfb0def7f93743193a119c3722..fb1b8c3a6887be580a34cc6707727fa352b90ea2 100644 GIT binary patch literal 2264 zcmZvd%S%;J6vmg2>{eE0_M)h3l$n%u)ccAGF?(THq)CGa?Wxd1EUchGgg9^%4Z=Y+ zaum^^fkZ^32nQk##XAPsc+T(*Vz=QzV!z>G;uFK8#OH?J5XTIEB90rLB2F0oPMkG7PyAuHl>Yu0-VCn9 zb)Nf(HHMpshYWWT+YR>;uNZzz95nolIAVB=_}cIX;(NoB#IJZ2&-zK6#gEB=T$diX z&-I!l<$?5pdqHg|kK{JFJFr&RAbv~kNV?F)*X&1SI71n)G?S@y^;QGkP zO|<{-exj838_(NTKoPF*Reonh^s82@FB3`Qx^tVI!q*CK!FBg^v|1~?9d80}cM4xJ z{4$<`SDV%i@5j^dU8W7dhj4vw_Lw#de}QM=`%D{!zs2=!)LON7@Jak3Sar22_>5`0 zLrE6k#mfups^E2|?XWK4zMpo@(#lA5Gg4O7aCh zZQ5SbzQN~At26C4yqu#`d($sTZ6mzaG~E-mH2jolx+iK~@LtpOR8|{=KQ*mBlw<_{ z(zFAnjlsuF3(pFX&+ut{aWP?kh#rTF-56C%+`ArDl)Bt9mkaC4oI*obs3e@S!hH?l zsFhAx<=v;;DXZOejT?K|;kE8sAxB+Y=dP75pOYh;T5Vvza=lenxk`Pv(^i)RFP)F} zvviEAOP`rqa;K=*jF;2VeuvpuF5YW{myg!E(Nm*UZ;GkW`s)l%&|x3{2|dm7E8PR~ zb9$Y0{d7g9b*SYt-LzhQ}rAcJI;owK1 z)#-<}6;pfyYi|%)qXF03;HD62-U||sVcWp-_&IDl_!WKui-DK$ud#OUC;WTE-|(x3 z>+41S814YKV4A-d-(@(3PZ}=b3x-GW7Yvv2V}@t(Gln1I9~qv*KR3LHe`$Cb|JLvd z{)6G~_%GNN*3(8keuGcSv|N*t%i!v-{Dv-gs>f`C1!R-LD7&r<`f%Ks{!;A~S^rR@OJVS1L8YKFH! zGdebkbYfa9z46+*;60dDtkWqX3C+RM@Li^*;R9F(9yhHBAHnpz_L(*cFJn1)k7?K8 zvzVUtA=4hfpJ4ak-KIT-&zZKzv_<%mY5Prk2VXHQY1&u#Pt&vt+8P_#Gp2LwbPD^0 zCrsOIS_)n;En(VO_!ZL*m{x|*nik%5k%#bSrs=KJwg7))n$}v|GW@e?`U+`Vh5s<^ za45+ycr&k+uDRE=4tTd|M@&n?3)tFP%wEBg!}Tt%YJd~T=w$}8~cO>kiEBFgfN9Wf7 diff --git a/common/core/desktop/tests/unit/kmx/kmx.cpp b/common/core/desktop/tests/unit/kmx/kmx.cpp index f2f1b3b6ee..eb414ce416 100644 --- a/common/core/desktop/tests/unit/kmx/kmx.cpp +++ b/common/core/desktop/tests/unit/kmx/kmx.cpp @@ -23,13 +23,16 @@ #include "state.hpp" #include "utfcodec.hpp" -#define try_status(expr) \ +#include "../test_assert.h" +#include "../test_color.h" + +/*#define try_status(expr) \ {auto __s = (expr); if (__s != KM_KBP_STATUS_OK) std::exit(100*__LINE__+__s);} #ifdef assert #undef assert #endif -#define assert(expr) {if (!(expr)) std::exit(100*__LINE__); } +#define assert(expr) {if (!(expr)) std::exit(100*__LINE__); }*/ namespace { @@ -159,7 +162,7 @@ key_event next_key(std::string &keys) { } } -void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16string & text_store, kmx_options &options) { +void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16string & text_store, std::vector & context, kmx_options &options) { switch (act.type) { case KM_KBP_IT_END: @@ -170,6 +173,7 @@ void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16 //std::cout << "beep" << std::endl; break; case KM_KBP_IT_CHAR: + context.push_back(km_kbp_context_item{KM_KBP_CT_CHAR, {0,}, {act.character}}); if (Uni_IsSMP(act.character)) { text_store.push_back(Uni_UTF32ToSurrogate1(act.character)); text_store.push_back(Uni_UTF32ToSurrogate2(act.character)); @@ -181,6 +185,7 @@ void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16 break; case KM_KBP_IT_MARKER: //std::cout << "deadkey(" << act.marker << ")" << std::endl; + context.push_back(km_kbp_context_item{KM_KBP_CT_MARKER, {0,}, {(uint32_t)act.marker}}); break; case KM_KBP_IT_BACK: // It is valid for a backspace to be received with an empty text store @@ -189,17 +194,29 @@ void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16 // processing at start of a text store, e.g. delete from a previous cell // in a table. Or, if Keyman has a cached context, then there may be // additional text in the text store that Keyman can't see. - if (text_store.length() > 0) { - auto ch = text_store.back(); + if(act.backspace.expected_type == KM_KBP_IT_MARKER) { + assert(!context.empty()); + assert(context.back().type == KM_KBP_CT_MARKER); + context.pop_back(); + } + else if (text_store.length() > 0) { + assert(!context.empty() && !text_store.empty()); + km_kbp_usv ch = text_store.back(); text_store.pop_back(); if(text_store.length() > 0 && Uni_IsSurrogate2(ch)) { - ch = text_store.back(); - if(Uni_IsSurrogate1(ch)) { + auto ch1 = text_store.back(); + if(Uni_IsSurrogate1(ch1)) { // We'll only pop the next character off it is actually a // surrogate pair + ch = Uni_SurrogateToUTF32(ch1, ch); text_store.pop_back(); } } + assert(ch == act.backspace.expected_value); + + assert(context.back().type == KM_KBP_CT_CHAR); + assert(context.back().character == ch); + context.pop_back(); } break; case KM_KBP_IT_PERSIST_OPT: @@ -306,6 +323,13 @@ int run_test(const km::kbp::path & source, const km::kbp::path & compiled) { km_kbp_context_item *citems = nullptr; try_status(km_kbp_context_items_from_utf16(context.c_str(), &citems)); try_status(km_kbp_context_set(km_kbp_state_context(test_state), citems)); + + // Make a copy of the setup context for the test + std::vector test_context; + for(km_kbp_context_item *ci = citems; ci->type != KM_KBP_CT_END; ci++) { + test_context.emplace_back(*ci); + } + km_kbp_context_items_dispose(citems); // Setup baseline text store @@ -316,7 +340,24 @@ int run_test(const km::kbp::path & source, const km::kbp::path & compiled) { try_status(km_kbp_process_event(test_state, p.vk, p.modifier_state)); for (auto act = km_kbp_state_action_items(test_state, nullptr); act->type != KM_KBP_IT_END; act++) { - apply_action(test_state, *act, text_store, options); + apply_action(test_state, *act, text_store, test_context, options); + } + + // Compare context and text store at each step - should be identical + size_t n = 0; + try_status(km_kbp_context_get(km_kbp_state_context(test_state), &citems)); + try_status(km_kbp_context_items_to_utf16(citems, nullptr, &n)); + std::cout << "n=" << n << std::endl; + km_kbp_cp *buf = new km_kbp_cp[n]; + + try_status(km_kbp_context_items_to_utf16(citems, buf, &n)); + + km_kbp_context_items_dispose(citems); + if (text_store != buf) { + std::cerr << "text store has diverged from buf" << std::endl; + std::cerr << "text store: " << string_to_hex(text_store) << " [" << text_store << "]" << std::endl; + std::cerr << "context : " << string_to_hex(buf) << " [" << buf << "]" << std::endl; + assert(false); } } @@ -493,21 +534,34 @@ int load_source(const km::kbp::path & path, std::string & keys, std::u16string & } constexpr const auto help_str = "\ -kmx \n\ +kmx [--color] \n\ help:\n\ \tKMN_FILE:\tThe source file for the keyboard under test.\n\ \tKMX_FILE:\tThe corresponding compiled kmx file produced from KMN_FILE.\n"; } // namespace -int main(int argc, char *argv[]) -{ - if (argc < 3) - { +int error_args() { std::cerr << "kmx: Not enough arguments." << std::endl; std::cout << help_str; return 1; +} + +int main(int argc, char *argv[]) { + int first_arg = 1; + + if (argc < 3) { + return error_args(); } + auto arg_color = std::string(argv[1]) == "--color"; + if(arg_color) { + first_arg++; + if(argc < 4) { + return error_args(); + } + } + console_color::enabled = console_color::isaterminal() || arg_color; + km::kbp::kmx::g_debug_ToConsole = TRUE; - return run_test(argv[1], argv[2]); + return run_test(argv[first_arg], argv[first_arg+1]); } diff --git a/linux/ibus-keyman/src/engine.c b/linux/ibus-keyman/src/engine.c index 6853aa4149..87ae3ac68b 100644 --- a/linux/ibus-keyman/src/engine.c +++ b/linux/ibus-keyman/src/engine.c @@ -766,7 +766,11 @@ ibus_keyman_engine_process_key_event (IBusEngine *engine, break; case KM_KBP_IT_BACK: g_message("BACK action %d/%d", i+1, (int)num_action_items); - if (keyman->char_buffer != NULL) + if (action_items[i].backspace.expected_type == KM_KBP_IT_MARKER) + { + g_message("skipping marker type"); + } + else if (keyman->char_buffer != NULL) { // ibus_keyman_engine_commit_string(keyman, keyman->char_buffer); g_message("removing one utf8 char from CHAR buffer"); From 91cde81d35a496a890b70a9a2f1e174d50714b8f Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 26 Jul 2021 15:29:16 +1000 Subject: [PATCH 2/5] chore(common/core): extra test for context consistency --- common/core/desktop/tests/unit/kmx/kmx.cpp | 25 +++++++++++++--------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/common/core/desktop/tests/unit/kmx/kmx.cpp b/common/core/desktop/tests/unit/kmx/kmx.cpp index eb414ce416..92db43353b 100644 --- a/common/core/desktop/tests/unit/kmx/kmx.cpp +++ b/common/core/desktop/tests/unit/kmx/kmx.cpp @@ -26,14 +26,6 @@ #include "../test_assert.h" #include "../test_color.h" -/*#define try_status(expr) \ -{auto __s = (expr); if (__s != KM_KBP_STATUS_OK) std::exit(100*__LINE__+__s);} - -#ifdef assert -#undef assert -#endif -#define assert(expr) {if (!(expr)) std::exit(100*__LINE__); }*/ - namespace { bool g_beep_found = false; @@ -347,11 +339,16 @@ int run_test(const km::kbp::path & source, const km::kbp::path & compiled) { size_t n = 0; try_status(km_kbp_context_get(km_kbp_state_context(test_state), &citems)); try_status(km_kbp_context_items_to_utf16(citems, nullptr, &n)); - std::cout << "n=" << n << std::endl; km_kbp_cp *buf = new km_kbp_cp[n]; - try_status(km_kbp_context_items_to_utf16(citems, buf, &n)); + // Verify that both our local test_context and the core's test_state.context have + // not diverged + auto ci = citems; + for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END && test_ci->type != KM_KBP_CT_END; ci++, test_ci++) { + assert(test_ci->type == ci->type && test_ci->marker == ci->marker); + } + km_kbp_context_items_dispose(citems); if (text_store != buf) { std::cerr << "text store has diverged from buf" << std::endl; @@ -370,6 +367,14 @@ int run_test(const km::kbp::path & source, const km::kbp::path & compiled) { try_status(km_kbp_context_items_to_utf16(citems, nullptr, &n)); km_kbp_cp *buf = new km_kbp_cp[n]; try_status(km_kbp_context_items_to_utf16(citems, buf, &n)); + + // Verify that both our local test_context and the core's test_state.context have + // not diverged + auto ci = citems; + for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END && test_ci->type != KM_KBP_CT_END; ci++, test_ci++) { + assert(test_ci->type == ci->type && test_ci->marker == ci->marker); + } + km_kbp_context_items_dispose(citems); std::cout << "expected : " << string_to_hex(expected) << " [" << expected << "]" << std::endl; From 75ce10510a27ab3ba6c60f660ac06c75a306b22b Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 27 Jul 2021 07:08:30 +1000 Subject: [PATCH 3/5] chore(common/core): address review comments Also introduce a specific enum for backspace type rather than inappropriately using the action item type enum. --- .../include/keyman/keyboardprocessor.h.in | 11 +++++-- common/core/desktop/src/kmx/kmx_processor.cpp | 31 ++++++++++-------- .../core/desktop/src/mock/mock_processor.cpp | 2 +- common/core/desktop/src/state.hpp | 4 +-- .../tests/unit/kmnkbd/action_items.hpp | 4 +-- .../unit/kmx/019 - multiple deadkeys.kmn | 6 ++-- .../unit/kmx/019 - multiple deadkeys.kmx | Bin 2264 -> 2264 bytes common/core/desktop/tests/unit/kmx/kmx.cpp | 2 +- 8 files changed, 35 insertions(+), 25 deletions(-) diff --git a/common/core/desktop/include/keyman/keyboardprocessor.h.in b/common/core/desktop/include/keyman/keyboardprocessor.h.in index 138ce065ef..7fd9122c53 100644 --- a/common/core/desktop/include/keyman/keyboardprocessor.h.in +++ b/common/core/desktop/include/keyman/keyboardprocessor.h.in @@ -473,10 +473,17 @@ other actions. */ typedef struct { - uint8_t expected_type; /// one of KM_KBP_IT_CHAR, KM_KBP_IT_MARKER, KM_KBP_IT_END (when type to delete is unknown) - uintptr_t expected_value; /// used mainly in unit tests + uint8_t expected_type; // km_kbp_backspace_type + uintptr_t expected_value; // used mainly in unit tests } km_kbp_backspace_item; +enum km_kbp_backspace_type { + KM_KBP_BT_UNKNOWN = 0, // Used at beginning of context; user-initiated backspace + KM_KBP_BT_CHAR = 1, // Deleting a character prior to insertion point + KM_KBP_BT_MARKER = 2, // Deleting a marker prior to insertion point + KM_KBP_BT_MAX_TYPE_ID +}; + typedef struct { uint8_t type; uint8_t _reserved[sizeof(void*)-sizeof(uint8_t)]; diff --git a/common/core/desktop/src/kmx/kmx_processor.cpp b/common/core/desktop/src/kmx/kmx_processor.cpp index c2746cc31c..6d06dd55e3 100644 --- a/common/core/desktop/src/kmx/kmx_processor.cpp +++ b/common/core/desktop/src/kmx/kmx_processor.cpp @@ -71,6 +71,15 @@ namespace km { return option(scope, key, value); } + void pop_context_push_backspace_action(km_kbp_state *state) { + assert(!state->context().empty()); + auto item = state->context().back(); + state->context().pop_back(); + state->actions().push_backspace( + item.type == KM_KBP_CT_MARKER ? KM_KBP_BT_MARKER : KM_KBP_BT_CHAR, + item.type == KM_KBP_CT_MARKER ? item.marker : item.character); + } + km_kbp_status kmx_processor::process_event(km_kbp_state *state, km_kbp_virtual_key vk, uint16_t modifier_state) { // Construct a context buffer from the items @@ -136,45 +145,39 @@ namespace km { // This only happens if we know we have context to delete. Last item must be a character assert(!state->context().empty() && state->context().back().type != KM_KBP_IT_MARKER); if(!state->context().empty()) { - auto item = state->context().back(); - state->context().pop_back(); - state->actions().push_backspace(KM_KBP_IT_CHAR, item.character); + pop_context_push_backspace_action(state); } else { // Note: only runs on non-debug build, fail safe - state->actions().push_backspace(KM_KBP_IT_END); + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); } break; case BK_DEADKEY: // This only happens if we know we have context to delete. Last item must be a deadkey assert(!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER); if(!state->context().empty()) { - auto item = state->context().back(); - state->context().pop_back(); - state->actions().push_backspace(KM_KBP_IT_MARKER, item.marker); + pop_context_push_backspace_action(state); } else { // Note: only runs on non-debug build, fail safe - state->actions().push_backspace(KM_KBP_IT_END); + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); } break; case BK_BACKSPACE: // User-initiated backspace. We need to delete deadkeys from context, both sides of the character deleted while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { - state->actions().push_backspace(KM_KBP_IT_MARKER, state->context().back().marker); - state->context().pop_back(); + pop_context_push_backspace_action(state); } if (!state->context().empty()) { - state->actions().push_backspace(KM_KBP_IT_CHAR, state->context().back().character); + state->actions().push_backspace(KM_KBP_BT_CHAR, state->context().back().character); state->context().pop_back(); } else { // Even if context is empty, we send the backspace event, because we may not // know the context. - state->actions().push_backspace(KM_KBP_IT_END); + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); } // Delete deadkey markers prior to char while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { - state->actions().push_backspace(KM_KBP_IT_MARKER, state->context().back().marker); - state->context().pop_back(); + pop_context_push_backspace_action(state); } break; default: diff --git a/common/core/desktop/src/mock/mock_processor.cpp b/common/core/desktop/src/mock/mock_processor.cpp index 1494512a64..2f2bf53a12 100644 --- a/common/core/desktop/src/mock/mock_processor.cpp +++ b/common/core/desktop/src/mock/mock_processor.cpp @@ -119,7 +119,7 @@ namespace km { { case KM_KBP_VKEY_BKSP: state->context().pop_back(); - state->actions().push_backspace(KM_KBP_IT_END); // Assuming we don't know the character + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); // Assuming we don't know the character break; case KM_KBP_VKEY_F2: diff --git a/common/core/desktop/src/state.hpp b/common/core/desktop/src/state.hpp index c95d2545df..90af6f6988 100644 --- a/common/core/desktop/src/state.hpp +++ b/common/core/desktop/src/state.hpp @@ -39,7 +39,7 @@ public: void push_character(km_kbp_usv usv); void push_marker(uintptr_t marker); void push_alert(); - void push_backspace(km_kbp_action_type expected_type, uintptr_t expected_value = 0); + void push_backspace(km_kbp_backspace_type expected_type, uintptr_t expected_value = 0); void push_persist(option const &); void push_persist(option const &&); void push_emit_keystroke(km_kbp_virtual_key vk=0); @@ -81,7 +81,7 @@ void actions::push_alert() { inline -void actions::push_backspace(km_kbp_action_type expected_type, uintptr_t expected_value) { +void actions::push_backspace(km_kbp_backspace_type expected_type, uintptr_t expected_value) { assert(empty() || (!empty() && back().type != KM_KBP_IT_END)); km_kbp_action_item item = {KM_KBP_IT_BACK}; item.backspace.expected_type = expected_type; diff --git a/common/core/desktop/tests/unit/kmnkbd/action_items.hpp b/common/core/desktop/tests/unit/kmnkbd/action_items.hpp index a5cf414231..8176f72ba5 100644 --- a/common/core/desktop/tests/unit/kmnkbd/action_items.hpp +++ b/common/core/desktop/tests/unit/kmnkbd/action_items.hpp @@ -40,8 +40,8 @@ void print_action_item(const char *title, km_kbp_action_item const & item) { break; case KM_KBP_IT_BACK: std::cout << " delete: " << - (item.backspace.expected_type == KM_KBP_IT_CHAR ? "char" : - item.backspace.expected_type == KM_KBP_IT_MARKER ? "marker" : + (item.backspace.expected_type == KM_KBP_BT_CHAR ? "char" : + item.backspace.expected_type == KM_KBP_BT_MARKER ? "marker" : "unknown") << " (" << item.backspace.expected_value << ")" << std::endl; break; diff --git a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn index ce3f46e940..c6b19700a0 100644 --- a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn +++ b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmn @@ -52,7 +52,7 @@ dk(8b) dk(8c) + 'x' > '8=Fail6 ' c [K_9] [K_X] [K_X] -> test that -+ '9' > '<' dk(9a) dk(9b) '>' dk(9c) dk(9d) ++ '9' > '{' dk(9a) dk(9b) '}' dk(9c) dk(9d) -dk(9b) '>' dk(9c) dk(9d) + 'x' > dk(9e) '>' dk(9f) dk(9g) -'<' dk(9a) dk(9e) '>' dk(9f) dk(9g) + 'x' > '9=OK' \ No newline at end of file +dk(9b) '}' dk(9c) dk(9d) + 'x' > dk(9e) '}' dk(9f) dk(9g) +'{' dk(9a) dk(9e) '}' dk(9f) dk(9g) + 'x' > '9=OK' \ No newline at end of file diff --git a/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmx b/common/core/desktop/tests/unit/kmx/019 - multiple deadkeys.kmx index fb1b8c3a6887be580a34cc6707727fa352b90ea2..44113e2d350ffba99dc565b5a32717b810f9d0c2 100644 GIT binary patch delta 90 zcmca1cteoKJ0c{Qfs=vZrR2(M8+jJ7@mDka|Ifj|4<-c|YA5eylSUB}28umrGZwD} Ts}KQ`q6`c$^_vaZmoowYWkem4 delta 90 zcmca1cteoKJ0c{Qfs=vZgyP(`jXaCk_-z>e|L0)f2a^H}c9ZwANu!7f1I3=R8H?M2 SRfvE|Q3eK>`pt&y%NYR(fg7U$ diff --git a/common/core/desktop/tests/unit/kmx/kmx.cpp b/common/core/desktop/tests/unit/kmx/kmx.cpp index 92db43353b..3695cb64fc 100644 --- a/common/core/desktop/tests/unit/kmx/kmx.cpp +++ b/common/core/desktop/tests/unit/kmx/kmx.cpp @@ -186,7 +186,7 @@ void apply_action(km_kbp_state const *, km_kbp_action_item const & act, std::u16 // processing at start of a text store, e.g. delete from a previous cell // in a table. Or, if Keyman has a cached context, then there may be // additional text in the text store that Keyman can't see. - if(act.backspace.expected_type == KM_KBP_IT_MARKER) { + if(act.backspace.expected_type == KM_KBP_BT_MARKER) { assert(!context.empty()); assert(context.back().type == KM_KBP_CT_MARKER); context.pop_back(); From 60b1cc7545a58272a04ac1aeb3fff5d63bf98826 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 27 Jul 2021 08:32:02 +1000 Subject: [PATCH 4/5] chore: fixup damaged merge --- common/core/desktop/src/kmx/kmx_processor.cpp | 85 ++++++++---------- .../tests/unit/kmx/033 - caps always off.kmx | Bin 516 -> 648 bytes .../047 - caps always off initially on.kmx | Bin 516 -> 648 bytes 3 files changed, 38 insertions(+), 47 deletions(-) diff --git a/common/core/desktop/src/kmx/kmx_processor.cpp b/common/core/desktop/src/kmx/kmx_processor.cpp index f77a699a25..42017eeb85 100644 --- a/common/core/desktop/src/kmx/kmx_processor.cpp +++ b/common/core/desktop/src/kmx/kmx_processor.cpp @@ -146,54 +146,45 @@ kmx_processor::process_event( state->actions().push_alert(); break; case QIT_BACK: - switch (a.dwData) { - case BK_DEFAULT: - // This only happens if we know we have context to delete. Last item must be a character - assert(!state->context().empty() && state->context().back().type != KM_KBP_IT_MARKER); - if(!state->context().empty()) { - pop_context_push_backspace_action(state); - } else { - // Note: only runs on non-debug build, fail safe - state->actions().push_backspace(KM_KBP_BT_UNKNOWN); - } - break; - case BK_DEADKEY: - // This only happens if we know we have context to delete. Last item must be a deadkey - assert(!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER); - if(!state->context().empty()) { - pop_context_push_backspace_action(state); - } else { - // Note: only runs on non-debug build, fail safe - state->actions().push_backspace(KM_KBP_BT_UNKNOWN); - } - - break; - case BK_BACKSPACE: - // User-initiated backspace. We need to delete deadkeys from context, both sides of the character deleted - while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { - pop_context_push_backspace_action(state); - } - if (!state->context().empty()) { - state->actions().push_backspace(KM_KBP_BT_CHAR, state->context().back().character); - state->context().pop_back(); - } else { - // Even if context is empty, we send the backspace event, because we may not - // know the context. - state->actions().push_backspace(KM_KBP_BT_UNKNOWN); - } - // Delete deadkey markers prior to char - while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { - pop_context_push_backspace_action(state); - } - break; - default: - assert(false); - } - break; + switch (a.dwData) { + case BK_DEFAULT: + // This only happens if we know we have context to delete. Last item must be a character + assert(!state->context().empty() && state->context().back().type != KM_KBP_IT_MARKER); + if(!state->context().empty()) { + pop_context_push_backspace_action(state); + } else { + // Note: only runs on non-debug build, fail safe + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); + } + break; + case BK_DEADKEY: + // This only happens if we know we have context to delete. Last item must be a deadkey + assert(!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER); + if(!state->context().empty()) { + pop_context_push_backspace_action(state); + } else { + // Note: only runs on non-debug build, fail safe + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); + } + + break; + case BK_BACKSPACE: + // User-initiated backspace. We need to delete deadkeys from context, both sides of the character deleted + while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { + pop_context_push_backspace_action(state); + } + if (!state->context().empty()) { + state->actions().push_backspace(KM_KBP_BT_CHAR, state->context().back().character); + state->context().pop_back(); + } else { + // Even if context is empty, we send the backspace event, because we may not + // know the context. + state->actions().push_backspace(KM_KBP_BT_UNKNOWN); + } + // Delete deadkey markers prior to char + while (!state->context().empty() && state->context().back().type == KM_KBP_IT_MARKER) { + pop_context_push_backspace_action(state); } - // Even if context is empty, we send the backspace event, because we may not - // know the context. - state->actions().push_backspace(); break; default: assert(false); diff --git a/common/core/desktop/tests/unit/kmx/033 - caps always off.kmx b/common/core/desktop/tests/unit/kmx/033 - caps always off.kmx index 9b414f222652620a9c6d55c9295f37ef20b97de6..08bd6521fe6c458c83b95fb857eff1a91ff6eba6 100644 GIT binary patch literal 648 zcmbV}y-EX75QR^IgkYoik5#a+6r)B`##OKgir^0xK?s`Q%Ib!Vn8aGdM-VHY!P>X7 zu+;BtRtRF{T+YlnGjs3k-djICI@GxIQTbdHmb1f(z`O=tAYT|%V|Lh6ei&yRsJ{~C zKmncy^VYIPEE`q7|)zO;uCIAhd1tH0yHO=i2hGb6j)woXH(dZ!)uo zAHZX^S?*ZJ?{Y>-d(=YaJL{c+j!_}|WB#a>=KuPxx1972eyjko96e8(aCwXSBVu5wz1kinu{r2i0W_ H)qiW>cJoIg literal 516 zcmbV|y=nqM6oro>60A*ZeFAY+NXjIpvy4SrA&LwBv4(^^L3|wTLmnVlT8IxI!NMw} z67;*Xt4xu~JDhvYx%bY@?lhj7kIG3Oi}p-J$`X+Urf)$Ju_utqSRyj%m@i|}1ini2 z1}-Fe3tuF92XmE+ENA8uokQ0ZsjZe?$j>V3O(D9AYEXGspWXoXfO`c!4K;!zYUU%2 z*)eo8fodwL3@W;2nU;fpmK3-d>je(~?Xpk2Nm-8L_%61?#ho;z{|QdDd+7`R4JzK; o9KZBy%&qWiQ1#}1@V|bIxj%fGH}CKcA%9i5QR@mDaDoIFRqFUmtwU_m$eEmiX!-fiy#zjF%nHl#ah=QK7zRN8M^k} z^gBs}BCed|%$zeb_fB$e`~2isQ_^y290)7eVHIHB1n-b9462eH_FBy1yaV+Q!U8D6 zi(t{3STW!$#3jHeHHdXE4G)NCIZucYm?Qs2dWyfO*0rG2ig{@TYiXD^xDXZH>>hxidZ zR*%OWb^JbiRCPctWWKW-_?AvlA?suQsGH{h`mS+7`UF2#hFDJSmz|+@I8W}Q3H)s{ zUee-49|BLEb&kFxbdcwK&hEQF-+)b|@6de_`Z3S>F5UNpegVDok0MUc)<+F^GWFlu EH{1zE9smFU literal 516 zcmbV|y=nqM6oro>60A*ZeFAY+NXjIpvy4SrA&LwBv4(^^L3|wTLmnVlT8IxI!NMw} z67;*Xt4xu~JDhvYx%bY@?lhj7kIG3Oi}p-J$`X+Urf)$Ju_utqSRyj%m@i|}1ini2 z1}-Fe3tuF92XmE+ENA8uokQ0ZsjZe?$j>V3O(D9AYEXGspWXoXfO`c!4K;!zYUU%2 z*)eo8fodwL3@W;2nU;fpmK3-d>je(~?Xpk2Nm-8L_%61?#ho;z{|QdDd+7`R4JzK; o9KZBy%&qWiQ1#}1@V|bIxj%fGH}CKcA%9 Date: Wed, 28 Jul 2021 10:02:39 +1000 Subject: [PATCH 5/5] chore(common/core): unit test fixes Per review comments. --- .../desktop/tests/unit/kmnkbd/debug_api.cpp | 20 ++++++++----------- common/core/desktop/tests/unit/kmx/kmx.cpp | 6 ++++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp b/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp index 1d1c530252..a508b062f3 100644 --- a/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp +++ b/common/core/desktop/tests/unit/kmnkbd/debug_api.cpp @@ -152,7 +152,6 @@ void test_basic_rule_matches() { })); try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_E, KM_KBP_MODIFIER_SHIFT, 1)); - assert(debug_items(test_state, { km_kbp_state_debug_item{KM_KBP_DEBUG_BEGIN, KM_KBP_DEBUG_FLAG_UNICODE, {KM_KBP_VKEY_E, KM_KBP_MODIFIER_SHIFT, 'E'}}, km_kbp_state_debug_item{KM_KBP_DEBUG_GROUP_ENTER, 0, {}, {u"", &gp}}, @@ -166,7 +165,6 @@ void test_basic_rule_matches() { })); try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_F, KM_KBP_MODIFIER_SHIFT, 1)); - assert(debug_items(test_state, { km_kbp_state_debug_item{KM_KBP_DEBUG_BEGIN, KM_KBP_DEBUG_FLAG_UNICODE, {KM_KBP_VKEY_F, KM_KBP_MODIFIER_SHIFT, 'F'}}, km_kbp_state_debug_item{KM_KBP_DEBUG_GROUP_ENTER, 0, {}, {u"", &gp}}, @@ -177,11 +175,11 @@ void test_basic_rule_matches() { })); km_kbp_action_item bksp_d = {KM_KBP_IT_BACK}; - bksp_d.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_d.backspace.expected_type = KM_KBP_BT_CHAR; bksp_d.backspace.expected_value = 'D'; km_kbp_action_item bksp_e = {KM_KBP_IT_BACK}; - bksp_e.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_e.backspace.expected_type = KM_KBP_BT_CHAR; bksp_e.backspace.expected_value = 'E'; assert(action_items(test_state, { @@ -230,7 +228,7 @@ void test_multiple_groups() { })); km_kbp_action_item bksp_a = {KM_KBP_IT_BACK}; - bksp_a.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_a.backspace.expected_type = KM_KBP_BT_CHAR; bksp_a.backspace.expected_value = 'a'; assert(action_items(test_state, { @@ -241,7 +239,6 @@ void test_multiple_groups() { })); try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_2, 0, 1)); - assert(debug_items(test_state, { km_kbp_state_debug_item{KM_KBP_DEBUG_BEGIN, KM_KBP_DEBUG_FLAG_UNICODE, {KM_KBP_VKEY_2, 0, '2'}}, km_kbp_state_debug_item{KM_KBP_DEBUG_GROUP_ENTER, 0, {}, {u"", &gp}}, @@ -264,7 +261,7 @@ void test_multiple_groups() { })); km_kbp_action_item bksp_b = {KM_KBP_IT_BACK}; - bksp_b.backspace.expected_type = KM_KBP_IT_CHAR; + bksp_b.backspace.expected_type = KM_KBP_BT_CHAR; bksp_b.backspace.expected_value = 'b'; assert(action_items(test_state, { @@ -311,7 +308,6 @@ void test_store_offsets() { })); try_status(km_kbp_process_event(test_state, KM_KBP_VKEY_B, 0, 1)); - assert(debug_items(test_state, { km_kbp_state_debug_item{KM_KBP_DEBUG_BEGIN, KM_KBP_DEBUG_FLAG_UNICODE, {KM_KBP_VKEY_B, 0, 'b'}}, km_kbp_state_debug_item{KM_KBP_DEBUG_GROUP_ENTER, 0, {}, {u"", &gp}}, @@ -332,10 +328,10 @@ void test_store_offsets() { {KM_KBP_IT_BACK}, {KM_KBP_IT_BACK} }; - bksp[0].backspace.expected_type = KM_KBP_IT_CHAR; - bksp[1].backspace.expected_type = KM_KBP_IT_CHAR; - bksp[2].backspace.expected_type = KM_KBP_IT_CHAR; - bksp[3].backspace.expected_type = KM_KBP_IT_CHAR; + bksp[0].backspace.expected_type = KM_KBP_BT_CHAR; + bksp[1].backspace.expected_type = KM_KBP_BT_CHAR; + bksp[2].backspace.expected_type = KM_KBP_BT_CHAR; + bksp[3].backspace.expected_type = KM_KBP_BT_CHAR; bksp[0].backspace.expected_value = 'y'; bksp[1].backspace.expected_value = 'a'; bksp[2].backspace.expected_value = 'x'; diff --git a/common/core/desktop/tests/unit/kmx/kmx.cpp b/common/core/desktop/tests/unit/kmx/kmx.cpp index ecc3d29fc4..503547c99c 100644 --- a/common/core/desktop/tests/unit/kmx/kmx.cpp +++ b/common/core/desktop/tests/unit/kmx/kmx.cpp @@ -372,7 +372,8 @@ int run_test(const km::kbp::path &source, const km::kbp::path &compiled) { // Verify that both our local test_context and the core's test_state.context have // not diverged auto ci = citems; - for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END && test_ci->type != KM_KBP_CT_END; ci++, test_ci++) { + for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END || test_ci != test_context.end(); ci++, test_ci++) { + assert(ci->type != KM_KBP_CT_END && test_ci != test_context.end()); // Verify that both lists are same length assert(test_ci->type == ci->type && test_ci->marker == ci->marker); } @@ -398,7 +399,8 @@ int run_test(const km::kbp::path &source, const km::kbp::path &compiled) { // Verify that both our local test_context and the core's test_state.context have // not diverged auto ci = citems; - for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END && test_ci->type != KM_KBP_CT_END; ci++, test_ci++) { + for(auto test_ci = test_context.begin(); ci->type != KM_KBP_CT_END || test_ci != test_context.end(); ci++, test_ci++) { + assert(ci->type != KM_KBP_CT_END && test_ci != test_context.end()); // Verify that both lists are same length assert(test_ci->type == ci->type && test_ci->marker == ci->marker); }