From bfdc3afb4583e22eec0aedc21c3e2fe2c53849a5 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Thu, 29 Jul 2021 16:15:36 +1000 Subject: [PATCH] chore(common/core): refactor backspace handling Moves the responsibility for deletion of markers when the users presses the backspace key, from `kmx_processor::process_event` to `KMX_ProcessEvent::ProcessGroup`. This has no effect on usage of Keyman Core, but: 1. Simplifies the association between context and action further 2. Ensures that `m_actions` and `state->actions` will always be the same length, making future refactoring of these simpler 3. Allows us to use the index into the m_actions array for Debug events, which simplifies the interleaving of these for the upcoming Debugger. When the user presses backspace at start of context (where there may be markers in the cached context, but no characters), the kmx processor will delete any markers prior to insertion point before requesting that the consumer emit the backspace virtual key back to the application, to allow the application to handle backspace at start of text. --- common/core/desktop/src/kmx/kmx_actions.cpp | 6 --- common/core/desktop/src/kmx/kmx_file.h | 29 +++++++------ .../core/desktop/src/kmx/kmx_processevent.cpp | 21 +++++++++- common/core/desktop/src/kmx/kmx_processor.cpp | 42 +++++-------------- 4 files changed, 44 insertions(+), 54 deletions(-) diff --git a/common/core/desktop/src/kmx/kmx_actions.cpp b/common/core/desktop/src/kmx/kmx_actions.cpp index 7eea892b63..c0d7cdafef 100644 --- a/common/core/desktop/src/kmx/kmx_actions.cpp +++ b/common/core/desktop/src/kmx/kmx_actions.cpp @@ -49,13 +49,7 @@ KMX_BOOL KMX_Actions::QueueAction(int ItemType, KMX_DWORD dwData) break; case QIT_BACK: - if(dwData == BK_BACKSPACE) // User pressed backspace so delete deadkeys - while(m_context->CharIsDeadkey()) m_context->Delete(); - m_context->Delete(); - - if(dwData == BK_BACKSPACE) // User pressed backspace so delete deadkeys - while(m_context->CharIsDeadkey()) m_context->Delete(); break; } diff --git a/common/core/desktop/src/kmx/kmx_file.h b/common/core/desktop/src/kmx/kmx_file.h index 0a686b2567..15de5e985f 100644 --- a/common/core/desktop/src/kmx/kmx_file.h +++ b/common/core/desktop/src/kmx/kmx_file.h @@ -19,17 +19,17 @@ namespace kmx { #define KEYMANID_IGNORE 0xFFFFFFFE #define KEYMANID_INVALID 0xFFFFFFFD -/* Shift flags for hotkeys (version 1.0) */ +/* Shift flags for hotkeys (version 1.0) */ #define SHIFTFLAG 0x2000 #define CTRLFLAG 0x4000 #define ALTFLAG 0x8000 -/* Miscellaneous flags and defines */ +/* Miscellaneous flags and defines */ #define MAXGROUPS 128 -/* File version identifiers */ +/* File version identifiers */ #define VERSION_30 0x00000300 #define VERSION_31 0x00000301 @@ -52,7 +52,6 @@ namespace kmx { #define BK_DEFAULT 0 #define BK_DEADKEY 1 -#define BK_BACKSPACE 2 // Different begin types #define BEGIN_ANSI 0 @@ -127,7 +126,7 @@ namespace kmx { #define TSS__MAX 38 -/* wm_keyman_control_internal message control codes */ +/* wm_keyman_control_internal message control codes */ #define KMCI_SELECTKEYBOARD 3 // I3933 #define KMCI_SELECTKEYBOARD_TSF 4 // I3933 @@ -205,7 +204,7 @@ namespace kmx { #define K_CTRLFLAG 0x0020 // Either ctrl flag #define K_ALTFLAG 0x0040 // Either alt flag //#define K_METAFLAG 0x0080 // Either Meta-key flag (tentative). Not usable in keyboard rules; - // Used internally (currently, only by KMW) to ensure Meta-key + // Used internally (currently, only by KMW) to ensure Meta-key // shortcuts safely bypass rules // Meta key = Command key on macOS, Windows key on Windows #define CAPITALFLAG 0x0100 // Caps lock on @@ -222,7 +221,7 @@ namespace kmx { struct COMP_STORE { KMX_DWORD dwSystemID; - KMX_DWORD dpName; + KMX_DWORD dpName; KMX_DWORD dpString; }; @@ -237,30 +236,30 @@ struct COMP_KEY { struct COMP_GROUP { KMX_DWORD dpName; KMX_DWORD dpKeyArray; // [LPKEY] address of first item in key array - KMX_DWORD dpMatch; - KMX_DWORD dpNoMatch; + KMX_DWORD dpMatch; + KMX_DWORD dpNoMatch; KMX_DWORD cxKeyArray; // in array entries KMX_BOOL fUsingKeys; // group(xx) [using keys] <-- specified or not }; struct COMP_KEYBOARD { KMX_DWORD dwIdentifier; // 0000 Keyman compiled keyboard id - + KMX_DWORD dwFileVersion; // 0004 Version of the file - Keyman 4.0 is 0x0400 - + KMX_DWORD dwCheckSum; // 0008 As stored in keyboard KMX_DWORD KeyboardID; // 000C as stored in HKEY_LOCAL_MACHINE//system//currentcontrolset//control//keyboard layouts - KMX_DWORD IsRegistered; // 0010 + KMX_DWORD IsRegistered; // 0010 KMX_DWORD version; // 0014 keyboard version - + KMX_DWORD cxStoreArray; // 0018 in array entries KMX_DWORD cxGroupArray; // 001C in array entries KMX_DWORD dpStoreArray; // 0020 [LPSTORE] address of first item in store array KMX_DWORD dpGroupArray; // 0024 [LPGROUP] address of first item in group array - + KMX_DWORD StartGroup[2]; // 0028 index of starting groups [2 of them] - + KMX_DWORD dwFlags; // 0030 Flags for the keyboard file KMX_DWORD dwHotKey; // 0034 standard windows hotkey (hiword=shift/ctrl/alt stuff, loword=vkey) diff --git a/common/core/desktop/src/kmx/kmx_processevent.cpp b/common/core/desktop/src/kmx/kmx_processevent.cpp index 665947106b..1d9e2e46be 100644 --- a/common/core/desktop/src/kmx/kmx_processevent.cpp +++ b/common/core/desktop/src/kmx/kmx_processevent.cpp @@ -266,7 +266,16 @@ KMX_BOOL KMX_ProcessEvent::ProcessGroup(LPGROUP gp, KMX_BOOL *pOutputKeystroke) KMX_BOOL fIsBackspace = m_state.vkey == KM_KBP_VKEY_BKSP && (m_modifiers & (LCTRLFLAG|RCTRLFLAG|LALTFLAG|RALTFLAG)) == 0; // I4128 if(fIsBackspace) { // I4838 // I4933 + + // Delete deadkeys prior to insertion point PKMX_WCHAR pdeletecontext = m_context.Buf(1); // I4933 + while(pdeletecontext && *pdeletecontext == UC_SENTINEL) { + m_actions.QueueAction(QIT_BACK, BK_DEADKEY); // side effect: also removes last char from context + pdeletecontext = m_context.Buf(1); + } + + // If there is now no character in the context, we want to + // emit the backspace for application to use if(!pdeletecontext || *pdeletecontext == 0) { // I4933 m_actions.QueueAction(QIT_INVALIDATECONTEXT, 0); if(m_debug_items) { @@ -275,7 +284,17 @@ KMX_BOOL KMX_ProcessEvent::ProcessGroup(LPGROUP gp, KMX_BOOL *pOutputKeystroke) *pOutputKeystroke = TRUE; // I4933 return FALSE; // I4933 } - m_actions.QueueAction(QIT_BACK, BK_BACKSPACE); // I4933 + + // Emit a backspace to delete the character + m_actions.QueueAction(QIT_BACK, BK_DEFAULT); // side effect: also removes last char from context + + // And delete any deadkeys prior to insertion point again + pdeletecontext = m_context.Buf(1); + while(pdeletecontext && *pdeletecontext == UC_SENTINEL) { + m_actions.QueueAction(QIT_BACK, BK_DEADKEY); + pdeletecontext = m_context.Buf(1); // side effect: also removes last char from context + } + } else { // I4024 // I4128 // I4287 // I4290 DebugLog(" ... IsLegacy = FALSE; IsTIP = TRUE"); // I4128 m_actions.QueueAction(QIT_INVALIDATECONTEXT, 0); diff --git a/common/core/desktop/src/kmx/kmx_processor.cpp b/common/core/desktop/src/kmx/kmx_processor.cpp index 42017eeb85..2e0ac04b6b 100644 --- a/common/core/desktop/src/kmx/kmx_processor.cpp +++ b/common/core/desktop/src/kmx/kmx_processor.cpp @@ -74,15 +74,6 @@ kmx_processor::update_option( 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, @@ -149,9 +140,12 @@ kmx_processor::process_event( 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); + assert(!state->context().empty()); + assert(state->context().back().type != KM_KBP_IT_MARKER); if(!state->context().empty()) { - pop_context_push_backspace_action(state); + auto item = state->context().back(); + state->context().pop_back(); + state->actions().push_backspace(KM_KBP_BT_CHAR, item.character); } else { // Note: only runs on non-debug build, fail safe state->actions().push_backspace(KM_KBP_BT_UNKNOWN); @@ -159,32 +153,16 @@ kmx_processor::process_event( 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); + assert(!state->context().empty()); + assert(state->context().back().type == KM_KBP_IT_MARKER); if(!state->context().empty()) { - pop_context_push_backspace_action(state); + auto item = state->context().back(); + state->context().pop_back(); + state->actions().push_backspace(KM_KBP_BT_MARKER, item.marker); } 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);