From b6daac0bdb692548888e296fb8604081b85790c2 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Sat, 25 Nov 2023 13:33:54 +1000 Subject: [PATCH 1/2] feat(core): action struct to action items conversion Adds state->set_actions(). This sets the Core's action list to match the contents of the action struct. Note that markers are not supported and backspace expected_values will be empty, as this information is not available. As the intended consumer of the action struct does not need to know this information, this should be adequate. --- core/include/keyman/keyman_core_api.h | 6 +- core/src/action.cpp | 41 ++- core/src/action.hpp | 3 +- core/src/km_core_action_api.cpp | 8 +- core/src/state.hpp | 5 +- core/tests/unit/kmnkbd/action_api.cpp | 12 +- core/tests/unit/kmnkbd/action_set_api.cpp | 356 ++++++++++++++++++++++ core/tests/unit/kmnkbd/meson.build | 1 + 8 files changed, 415 insertions(+), 17 deletions(-) create mode 100644 core/tests/unit/kmnkbd/action_set_api.cpp diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 309fdb20e1b..c2aad8974f6 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -582,7 +582,7 @@ typedef struct { km_core_usv* output; // list of options to persist, terminated with KM_CORE_OPTIONS_END - km_core_option_item* persist_options; + km_core_option_item * persist_options; // issue a beep, 0 = no, 1 = yes km_core_bool do_alert; @@ -611,7 +611,7 @@ A pointer to a `km_core_actions` object, which must be freed with ```c */ KMN_API -km_core_actions* +km_core_actions const * km_core_state_get_actions( km_core_state const *state ); @@ -630,7 +630,7 @@ returned by `km_core_state_get_actions`. KMN_API km_core_status km_core_actions_dispose( - km_core_actions* actions + km_core_actions const * actions ); /* diff --git a/core/src/action.cpp b/core/src/action.cpp index 4c018cabbd9..baa627ed3da 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -17,7 +17,7 @@ #include "state.hpp" #include "option.hpp" -km_core_actions * km::core::action_item_list_to_actions_object( +km_core_actions const * km::core::action_item_list_to_actions_object( km_core_action_item const *action_items ) { assert(action_items != nullptr); @@ -138,3 +138,42 @@ km_core_actions * km::core::action_item_list_to_actions_object( return actions.release(); } + + +// TODO: this is effectively the inverse of action_item_list_to_actions_object, +// and perhaps we should consider changing that function to be a member +// of state also, so that we can move memory management into state? +bool km::core::state::set_actions( + km_core_actions const &actions +) { + _actions.clear(); + + // number of codepoints (not codeunits!) to delete from app context. + + for(unsigned int i = 0; i < actions.code_points_to_delete; i++) { + _actions.push_backspace(KM_CORE_BT_CHAR, 0); //(*end).character); + } + + for(auto output = actions.output; *output; output++) { + _actions.push_character(*output); + } + + for(auto opt = actions.persist_options; opt->scope; opt++) { + km::core::option opt0(static_cast(opt->scope), opt->key, opt->value); + _actions.push_persist(opt0); + } + + if(actions.do_alert) { + _actions.push_alert(); + } + + if(actions.emit_keystroke) { + _actions.push_emit_keystroke(); + } + + if(actions.new_caps_lock_state != KM_CORE_CAPS_UNCHANGED) { + _actions.push_capslock(actions.new_caps_lock_state == KM_CORE_CAPS_ON); + } + + return true; +} diff --git a/core/src/action.hpp b/core/src/action.hpp index 4cc0aabdf27..06038c31222 100644 --- a/core/src/action.hpp +++ b/core/src/action.hpp @@ -9,11 +9,12 @@ #pragma once #include +#include namespace km { namespace core { - km_core_actions* action_item_list_to_actions_object( + km_core_actions const *action_item_list_to_actions_object( km_core_action_item const *action_items ); } // namespace core diff --git a/core/src/km_core_action_api.cpp b/core/src/km_core_action_api.cpp index d552d410592..1177ec71fcc 100644 --- a/core/src/km_core_action_api.cpp +++ b/core/src/km_core_action_api.cpp @@ -19,7 +19,7 @@ using namespace km::core; -km_core_actions* km_core_state_get_actions( +km_core_actions const * km_core_state_get_actions( km_core_state const *state ) { assert(state); @@ -27,18 +27,16 @@ km_core_actions* km_core_state_get_actions( return nullptr; } - km_core_actions* actions = nullptr; auto action_items = km_core_state_action_items(state, nullptr); if(!action_items) { return nullptr; } - actions = action_item_list_to_actions_object(action_items); - return actions; + return action_item_list_to_actions_object(action_items); } km_core_status km_core_actions_dispose( - km_core_actions* actions + km_core_actions const * actions ) { if(actions == nullptr) { return KM_CORE_STATUS_OK; diff --git a/core/src/state.hpp b/core/src/state.hpp index fadfa61fc8b..3ff476bbfaa 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -152,8 +152,11 @@ class state void imx_deregister_callback(); void imx_callback(uint32_t imx_id); -}; + bool set_actions( + km_core_actions const &actions + ); + }; } // namespace core } // namespace km diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index e46d207194a..dd0e46f005e 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -34,7 +34,7 @@ void test_two_backspaces() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 1); assert(std::u32string(actions->output) == U""); @@ -65,7 +65,7 @@ void test_marker_text_interleaved() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U"ABD"); @@ -88,7 +88,7 @@ void test_alert() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -111,7 +111,7 @@ void test_emit_keystroke() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -135,7 +135,7 @@ void test_invalidate_context() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -164,7 +164,7 @@ void test_persist_opt() { end_action_item() }; - km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); + km_core_actions const *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); diff --git a/core/tests/unit/kmnkbd/action_set_api.cpp b/core/tests/unit/kmnkbd/action_set_api.cpp new file mode 100644 index 00000000000..12e0ce3195b --- /dev/null +++ b/core/tests/unit/kmnkbd/action_set_api.cpp @@ -0,0 +1,356 @@ +/* + Copyright: © 2018 SIL International. + Description: Tests for the context API family of functions. + Create Date: 23 Oct 2023 + Authors: Marc Durdin + History: 23 Oct 2023 - MCD - Initial implementation. +*/ +#include +#include + +#include "path.hpp" +#include "action.hpp" + +#include +#include "../emscripten_filesystem.h" + +const km_core_action_item alert_action_item(); +const km_core_action_item bksp_action_item(uint8_t type, uintptr_t value); +const km_core_action_item caps_action_item(uint8_t capsLock); +const km_core_action_item char_action_item(km_core_usv chr); +const km_core_action_item emit_keystroke_action_item(); +const km_core_action_item persist_opt_action_item(km_core_option_item const *option); +const km_core_action_item end_action_item(); +const km_core_action_item invalidate_context_action_item(); +const km_core_action_item marker_action_item(uint32_t marker); + +//------------------------------------------------------------------------------------- + +km_core_option_item test_env_opts[] = +{ + KM_CORE_OPTIONS_END +}; + +km_core_keyboard * test_kb = nullptr; +km_core_state * test_state = nullptr; +km_core_context_item * citems = nullptr; +km_core_usv test_empty_output[] = {0}; +std::string arg_path; + +void teardown() { + if(citems) { + km_core_context_items_dispose(citems); + citems = nullptr; + } + if(test_state) { + km_core_state_dispose(test_state); + test_state = nullptr; + } + if(test_kb) { + km_core_keyboard_dispose(test_kb); + test_kb = nullptr; + } +} + +void setup(const char *keyboard, const km_core_cp* context) { + teardown(); + + km::core::path path = km::core::path::join(arg_path, keyboard); + try_status(km_core_keyboard_load(path.native().c_str(), &test_kb)); + try_status(km_core_state_create(test_kb, test_env_opts, &test_state)); + try_status(km_core_context_items_from_utf16(context, &citems)); + try_status(km_core_context_set(km_core_state_context(test_state), citems)); +} + +void run_test(km_core_action_item const * action_items, const km_core_actions &actions) { + setup("k_000___null_keyboard.kmx", u""); + test_state->set_actions(actions); + auto set_actions = test_state->actions(); + + int n = 0; + for(auto act = set_actions.begin(); act != set_actions.end(); act++, n++) { + assert(act->type == action_items[n].type); + // TODO: all other fields + switch(act->type) { + case KM_CORE_IT_ALERT: + break; + case KM_CORE_IT_BACK: + assert(act->backspace.expected_type == action_items[n].backspace.expected_type); + assert(act->backspace.expected_value == action_items[n].backspace.expected_value); + break; + case KM_CORE_IT_CAPSLOCK: + assert(act->capsLock == action_items[n].capsLock); + break; + case KM_CORE_IT_CHAR: + assert(act->character == action_items[n].character); + break; + case KM_CORE_IT_EMIT_KEYSTROKE: + break; + case KM_CORE_IT_END: + break; + case KM_CORE_IT_INVALIDATE_CONTEXT: + break; + case KM_CORE_IT_MARKER: + assert(act->marker == action_items[n].marker); + break; + case KM_CORE_IT_PERSIST_OPT: + assert(act->option->scope == action_items[n].option->scope); + assert(std::u16string(act->option->key) == action_items[n].option->key); + assert(std::u16string(act->option->value) == action_items[n].option->value); + break; + default: + // Invalid action type + assert(false); + } + } +} + +//------------------------------------------------------------------------------------- +// Set Action tests +//------------------------------------------------------------------------------------- + +void test_two_backspaces() { + puts("test_two_backspaces"); + const km_core_action_item action_items[] = { + bksp_action_item(KM_CORE_BT_CHAR, 0), + bksp_action_item(KM_CORE_BT_CHAR, 0), + end_action_item() + }; + + const km_core_actions actions = { + 2, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +void test_character() { + puts("test_character"); + const km_core_action_item action_items[] = { + char_action_item(u'A'), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + (km_core_usv*) U"A", // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +//------------------------------------------------------------------------------------- + +void test_alert() { + puts("test_alert"); + const km_core_action_item action_items[] = { + alert_action_item(), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_TRUE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +//------------------------------------------------------------------------------------- + +void test_emit_keystroke() { + puts("test_emit_keystroke"); + const km_core_action_item action_items[] = { + emit_keystroke_action_item(), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_TRUE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +//------------------------------------------------------------------------------------- + +void test_invalidate_context() { + puts("test_invalidate_context"); + // note, this generates a no-op, DELETE? + const km_core_action_item action_items[] = { + // invalidate_context_action_item(), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +//------------------------------------------------------------------------------------- + +void test_persist_opt() { + puts("test_persist_opt"); + const km_core_option_item option = { + u"key", + u"value", + KM_CORE_OPT_KEYBOARD + }; + + /*TODO: const*/ km_core_option_item options[] = {{ + u"key", + u"value", + KM_CORE_OPT_KEYBOARD + }, + KM_CORE_OPTIONS_END + }; + + const km_core_action_item action_items[] = { + persist_opt_action_item(&option), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + options, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_UNCHANGED // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + +//------------------------------------------------------------------------------------- +// Launcher +//------------------------------------------------------------------------------------- + +constexpr const auto help_str = "\ +action_set_api [--color] \n\ +\n\ + --color Force color output\n\ + SOURCE_PATH Path where action_set_api.cpp is found; kmx files are\n\ + located relative to this path.\n"; + +int error_args() { + std::cerr << "action_set_api: Invalid arguments." << std::endl; + std::cout << help_str; + return 1; +} + +int main(int argc, char *argv []) { + + if(argc < 2) { + return error_args(); + } + + auto arg_color = std::string(argv[1]) == "--color"; + if(arg_color && argc < 3) { + return error_args(); + } + console_color::enabled = console_color::isaterminal() || arg_color; + +#ifdef __EMSCRIPTEN__ + arg_path = get_wasm_file_path(argv[arg_color ? 2 : 1]); +#else + arg_path = argv[arg_color ? 2 : 1]; +#endif + + // actions + test_two_backspaces(); + test_character(); + test_alert(); + test_emit_keystroke(); + test_invalidate_context(); + test_persist_opt(); +} + +//------------------------------------------------------------------------------------- +// Helper functions +//------------------------------------------------------------------------------------- + +const km_core_action_item alert_action_item() { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_ALERT; + return res; +} + +const km_core_action_item bksp_action_item(uint8_t type, uintptr_t value) { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_BACK; + res.backspace.expected_type = type; + res.backspace.expected_value = value; + return res; +} + +const km_core_action_item caps_action_item(uint8_t capsLock) { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_CAPSLOCK; + res.capsLock = capsLock; + return res; +} + +const km_core_action_item char_action_item(km_core_usv chr) { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_CHAR; + res.character = chr; + return res; +} + +const km_core_action_item emit_keystroke_action_item() { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_EMIT_KEYSTROKE; + return res; +} + +const km_core_action_item persist_opt_action_item(km_core_option_item const *option) { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_PERSIST_OPT; + res.option = option; + return res; +} + +const km_core_action_item end_action_item() { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_END; + return res; +} + +const km_core_action_item invalidate_context_action_item() { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_INVALIDATE_CONTEXT; + return res; +} + +const km_core_action_item marker_action_item(uint32_t marker) { + km_core_action_item res = {0}; + res.type = KM_CORE_IT_MARKER; + res.character = marker; + return res; +} diff --git a/core/tests/unit/kmnkbd/meson.build b/core/tests/unit/kmnkbd/meson.build index dc94227bef1..f59b79c3bef 100644 --- a/core/tests/unit/kmnkbd/meson.build +++ b/core/tests/unit/kmnkbd/meson.build @@ -17,6 +17,7 @@ endif local_defns = ['-DKM_CORE_LIBRARY_STATIC'] tests = [ ['action-api', 'action_api.cpp'], + ['action-set-api', 'action_set_api.cpp'], ['context-api', 'context_api.cpp'], ['keyboard-api', 'keyboard_api.cpp'], ['options-api', 'options_api.cpp'], From 802f22578a8b4a9b4934b24620b6e851451e6852 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 4 Dec 2023 13:37:59 +0700 Subject: [PATCH 2/2] chore(core): address review comments --- core/src/action.cpp | 2 +- core/src/state.hpp | 7 +++++++ core/tests/unit/kmnkbd/action_set_api.cpp | 24 +++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/core/src/action.cpp b/core/src/action.cpp index baa627ed3da..f4565844d76 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -151,7 +151,7 @@ bool km::core::state::set_actions( // number of codepoints (not codeunits!) to delete from app context. for(unsigned int i = 0; i < actions.code_points_to_delete; i++) { - _actions.push_backspace(KM_CORE_BT_CHAR, 0); //(*end).character); + _actions.push_backspace(KM_CORE_BT_CHAR, 0); // expected value is not known } for(auto output = actions.output; *output; output++) { diff --git a/core/src/state.hpp b/core/src/state.hpp index 3ff476bbfaa..b7f8baa486c 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -153,6 +153,13 @@ class state void imx_callback(uint32_t imx_id); + // This is intended to be used to take the actions given in the actions + // parameter, and load them into the _actions member of this class. Used by + // keyboard processors to set the output actions, and is a long-term + // replacement for the actions()::push_*() functions. Note that the + // km_core_actions struct does not include information about markers, which + // are maintained separately in the _ctxt member of this class, and the + // corresponding marker-backspace action items are never used here. bool set_actions( km_core_actions const &actions ); diff --git a/core/tests/unit/kmnkbd/action_set_api.cpp b/core/tests/unit/kmnkbd/action_set_api.cpp index 12e0ce3195b..378f3bcab15 100644 --- a/core/tests/unit/kmnkbd/action_set_api.cpp +++ b/core/tests/unit/kmnkbd/action_set_api.cpp @@ -247,6 +247,29 @@ void test_persist_opt() { run_test(action_items, actions); } +//------------------------------------------------------------------------------------- + +void test_caps_lock() { + puts("test_caps_lock"); + + const km_core_action_item action_items[] = { + caps_action_item(KM_CORE_CAPS_ON), + // invalidate_context_action_item(), + end_action_item() + }; + + const km_core_actions actions = { + 0, // unsigned int code_points_to_delete; + test_empty_output, // km_core_usv* output; + test_env_opts, // km_core_option_item* persist_options; + KM_CORE_FALSE, // km_core_bool do_alert; + KM_CORE_FALSE, // km_core_bool emit_keystroke; + KM_CORE_CAPS_ON // new_caps_lock_state; + }; + + run_test(action_items, actions); +} + //------------------------------------------------------------------------------------- // Launcher //------------------------------------------------------------------------------------- @@ -289,6 +312,7 @@ int main(int argc, char *argv []) { test_emit_keystroke(); test_invalidate_context(); test_persist_opt(); + test_caps_lock(); } //-------------------------------------------------------------------------------------