From 61330ba06deafab2a81d7589cbd489e9dc4739a6 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 23 Oct 2023 15:01:55 +0700 Subject: [PATCH 01/11] feat(core): new actions APIs Fixes #9720. Note: this will deprecate a number of Keyman Core APIs -- pretty much all existing action and context APIs. Deprecation marks will come in a follow-up commit. --- core/include/keyman/keyman_core_api.h | 154 +++++++++ core/src/action.cpp | 135 ++++++++ core/src/action.hpp | 20 ++ core/src/km_core_action_api.cpp | 64 ++++ core/src/km_core_context_api.cpp | 7 + core/src/km_core_state_api.cpp | 99 ++++++ core/src/meson.build | 4 + core/tests/unit/kmnkbd/action_api.cpp | 432 ++++++++++++++++++++++++++ core/tests/unit/kmnkbd/meson.build | 1 + 9 files changed, 916 insertions(+) create mode 100644 core/src/action.cpp create mode 100644 core/src/action.hpp create mode 100644 core/src/km_core_action_api.cpp create mode 100644 core/tests/unit/kmnkbd/action_api.cpp diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 83182f53a33..bd779a55a16 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -310,6 +310,38 @@ km_core_context_items_to_utf8(km_core_context_item const *item, char *buf, size_t *buf_size); +/* +``` +### `km_core_context_items_to_utf32` +##### Description: +Convert a context item array into a UTF-32 encoded string placing it into +the supplied buffer of specified size, and return the number of codepoints +actually used in the conversion. If null is passed as the buffer the +number codepoints required is returned. This will strip markers from the +context during the conversion. +##### Return status: +- `KM_CORE_STATUS_OK`: On success. +- `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. +- `KM_CORE_STATUS_INSUFFICENT_BUFFER`: If the buffer is not large enough. + `buf_size` will contain the space required. The contents of the buffer are + undefined. +##### Parameters: +- __context_items__: A pointer to the start of an array `km_core_context_item`. + Must be terminated with a type of `KM_CORE_CT_END`. +- __buf__: A pointer to the buffer to place the UTF-32 string into. + May be null to request size calculation. +- __buf_size__: a pointer to the result variable: + The size of the supplied buffer in codepoints if `buf` is given. + On return will be the size required if `buf` is null. + +```c +*/ +KMN_API +km_core_status +km_core_context_items_to_utf32(km_core_context_item const *item, + km_core_usv *buf, + size_t *buf_size); + /* ``` ### `km_core_context_items_dispose` @@ -526,6 +558,128 @@ enum km_core_action_type { KM_CORE_IT_MAX_TYPE_ID }; +/* +``` +### Actions +This structure provides the results of processing a key event to the Platform layer and +should be processed by the Platform layer to issue commands to the os text +services framework to transform the text store in the Client Application, among +other actions. + +This API replaces the Action items APIs, which is now deprecated and will be +removed in the future. +```c +*/ + +typedef struct { + // number of codepoints (not codeunits!) to delete from app context, 0+ + int delete_back; + + // null-term string of characters to insert into document + km_core_usv* output; + + // list of options to persist, terminated with KM_CORE_OPTIONS_END + km_core_option_item* persist_options; + + // issue a beep + bool do_alert; + + // emit the input keystroke to the application, unmodified? + bool emit_keystroke; + + // -1=unchanged, 0=off, 1=on + int new_caps_lock_state; +} km_core_actions; + +/* +``` +### `km_core_state_get_actions` +##### Description: +Returns a pointer to an actions object which details all the actions +that the Platform layer must take after a keystroke. The `delete_back` +action must be performed before the `output` action, but the other +actions may be performed in any order. +##### Return: +A pointer to a `km_core_actions` object, which must be freed with +`km_core_actions_dispose`. +##### Parameters: +- __state__: An opaque pointer to a state object. + +```c +*/ +KMN_API +km_core_actions* +km_core_state_get_actions( + km_core_state const *state +); + +/* +``` +### `km_core_actions_dispose` +##### Description: +Free the allocated memory belonging to an actions object previously +returned by `km_core_state_get_actions`. +##### Parameters: +- __actions__: A pointer to the actions object to be disposed of. + +```c +*/ +KMN_API +km_core_status +km_core_actions_dispose( + km_core_actions* actions +); + +/* +``` +### `km_core_state_context_validate` +##### Description: +Determines if the input context has changed, and if so, resets +the internal cached context (including markers), to the new +context string. + +This and `km_core_state_context_invalidate` will replace existing Core context +APIs. + +##### Parameters: +- __state__: An opaque pointer to a state object. +- __application_context__: A pointer to an null-terminated `km_core_cp` string + representing the current context from the application. +##### Return status: +- `KM_CORE_STATUS_OK`: On success. +- `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. + +```c +*/ +KMN_API +km_core_status +km_core_state_context_validate( + km_core_state *state, + km_core_cp const *application_context +); + +/* +``` +### `km_core_state_context_invalidate` +##### Description: +Flushes the internal cached context for the state. + +This and `km_core_state_context_validate` will replace existing Core context +APIs. + +##### Parameters: +- __state__: An opaque pointer to a state object. +##### Return status: +- `KM_CORE_STATUS_OK`: On success. +- `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. + +```c +*/ +KMN_API +km_core_status +km_core_state_context_invalidate( + km_core_state *state +); /* ``` diff --git a/core/src/action.cpp b/core/src/action.cpp new file mode 100644 index 00000000000..d957b318c13 --- /dev/null +++ b/core/src/action.cpp @@ -0,0 +1,135 @@ +/* + Copyright: © 2023 SIL International. + Description: Implementation of the action API functions using internal + data structures and functions. + Create Date: 23 Oct 2023 + Authors: Marc Durdin (MCD) + History: 23 Oct 2023 - MCD - Initial implementation from #9720 +*/ +#include +#include +#include + +#include + +#include "action.hpp" +#include "state.hpp" +#include "option.hpp" + +km_core_actions * km::kbp::action_items_to_actions( + km_core_action_item const *action_items +) { + assert(action_items != nullptr); + if(action_items == nullptr) { + return nullptr; + } + + km_core_status status = KM_CORE_STATUS_OK; + + km_core_actions* actions = new km_core_actions; + + // Set actions defaults + std::vector output; + std::vector options; + actions->delete_back = 0; + actions->output = nullptr; + actions->persist_options = nullptr; + actions->do_alert = false; + actions->emit_keystroke = false; + actions->new_caps_lock_state = -1; + + for (; action_items->type != KM_CORE_IT_END; ++action_items) { + assert(action_items->type < KM_CORE_IT_MAX_TYPE_ID); + + switch(action_items->type) { + case KM_CORE_IT_ALERT: + actions->do_alert = true; + break; + case KM_CORE_IT_BACK: + switch(action_items->backspace.expected_type) { + case KM_CORE_BT_UNKNOWN: + // this is equivalent to emit_keystroke, because the only time we + // are allowed to do an unknown bksp is when a bksp is passed in + actions->emit_keystroke = true; + break; + case KM_CORE_BT_CHAR: + if(output.empty()) { + actions->delete_back++; + } else { + auto last_context_item = output.back(); + output.pop_back(); + assert(last_context_item.type == KM_CORE_CT_CHAR); + assert(last_context_item.character == action_items->backspace.expected_value); + } + break; + case KM_CORE_BT_MARKER: + if(output.empty()) { + // deleting a marker has no effect on the application + } else { + auto last_context_item = output.back(); + output.pop_back(); + assert(last_context_item.type == KM_CORE_CT_MARKER); + assert(last_context_item.marker == action_items->backspace.expected_value); + } + break; + default: + assert(false); + } + break; + case KM_CORE_IT_CAPSLOCK: + actions->new_caps_lock_state = action_items->capsLock; + break; + case KM_CORE_IT_CHAR: + output.push_back({KM_CORE_CT_CHAR,{0},action_items->character}); + break; + case KM_CORE_IT_EMIT_KEYSTROKE: + actions->emit_keystroke = true; + break; + case KM_CORE_IT_INVALIDATE_CONTEXT: + // no-op + break; + case KM_CORE_IT_MARKER: + output.push_back({KM_CORE_CT_MARKER,{0},action_items->marker}); + break; + case KM_CORE_IT_PERSIST_OPT: + // TODO: lowpri: replace existing item if already present in options vector? + options.push_back(km::kbp::option( + static_cast(action_items->option->scope), + action_items->option->key, + action_items->option->value + )); + break; + default: + assert(false); + } + } + + output.push_back({KM_CORE_CT_END}); + + // Strip the markers from the output to be passed to the app + + size_t buf_size; + + if((status = km_core_context_items_to_utf32(output.data(), nullptr, &buf_size)) != KM_CORE_STATUS_OK) { + delete actions; + return nullptr; + } + + actions->output = new km_core_usv[buf_size]; + + if((status = km_core_context_items_to_utf32(output.data(), actions->output, &buf_size)) != KM_CORE_STATUS_OK) { + delete[] actions->output; + delete actions; + return nullptr; + } + + // Create an array of the persisted options + + if(!options.empty()) { + options.push_back(KM_CORE_OPTIONS_END); + actions->persist_options = new km_core_option_item[options.size()]; + std::copy(options.begin(), options.end(), actions->persist_options); + } + + return actions; +} diff --git a/core/src/action.hpp b/core/src/action.hpp new file mode 100644 index 00000000000..115d328a8c0 --- /dev/null +++ b/core/src/action.hpp @@ -0,0 +1,20 @@ +/* + Copyright: © 2023 SIL International. + Description: Internal actions methods for Keyman Core + Create Date: 23 Oct 2023 + Authors: Marc Durdin (MCD) + History: 23 Oct 2023 - MCD - Initial implementation +*/ + +#pragma once + +#include + +namespace km { +namespace kbp +{ + km_core_actions* action_items_to_actions( + km_core_action_item const *action_items + ); +} // namespace kbp +} // namespace km diff --git a/core/src/km_core_action_api.cpp b/core/src/km_core_action_api.cpp new file mode 100644 index 00000000000..057d1338d12 --- /dev/null +++ b/core/src/km_core_action_api.cpp @@ -0,0 +1,64 @@ +/* + Copyright: © 2023 SIL International. + Description: Implementation of the action API functions using internal + data structures and functions. + Create Date: 23 Oct 2023 + Authors: Marc Durdin (MCD) + History: 23 Oct 2023 - MCD - Initial implementation. +*/ +#include +#include +#include + +#include +#include "jsonpp.hpp" + +#include "processor.hpp" +#include "state.hpp" +#include "action.hpp" + +using namespace km::kbp; + +km_core_actions* km_core_state_get_actions( + km_core_state const *state +) { + assert(state); + if(!state) { + return nullptr; + } + + km_core_actions* actions = nullptr; + auto action_items = km_core_state_action_items(state, nullptr); + if(!action_items) { + return nullptr; + } + + actions = action_items_to_actions(action_items); + return actions; +} + +km_core_status km_core_actions_dispose( + km_core_actions* actions +) { + if(actions == nullptr) { + return KM_CORE_STATUS_OK; + } + + if(actions->output) { + delete[] actions->output; + } + + if(actions->persist_options) { + for(auto option = actions->persist_options; option->scope; option++) { + delete[] option->key; + delete[] option->value; + } + delete[] actions->persist_options; + } + + delete actions; + + return KM_CORE_STATUS_OK; +} + + diff --git a/core/src/km_core_context_api.cpp b/core/src/km_core_context_api.cpp index 3655af4e964..77b3d76a74d 100644 --- a/core/src/km_core_context_api.cpp +++ b/core/src/km_core_context_api.cpp @@ -140,6 +140,13 @@ km_core_status km_core_context_items_to_utf16(km_core_context_item const *ci, sz_ptr); } +km_core_status km_core_context_items_to_utf32(km_core_context_item const *ci, + km_core_usv *buf, size_t * sz_ptr) +{ + return _context_items_to(ci, + reinterpret_cast(buf), + sz_ptr); +} void km_core_context_items_dispose(km_core_context_item *ci) { diff --git a/core/src/km_core_state_api.cpp b/core/src/km_core_state_api.cpp index 62080108aaf..49776d5125d 100644 --- a/core/src/km_core_state_api.cpp +++ b/core/src/km_core_state_api.cpp @@ -259,3 +259,102 @@ void km_core_state_imx_deregister_callback(km_core_state *state) } state->imx_deregister_callback(); } + + +bool is_context_valid(km_core_cp const * context, km_core_cp const * cached_context) { + km_core_cp const* context_p = context; + while(*context_p) { + context_p++; + } + + km_core_cp const* cached_context_p = cached_context; + while(*cached_context_p) { + cached_context_p++; + } + + // we need to compare from the end of the cached context + for(; context_p >= context && cached_context_p >= cached_context; context_p--, cached_context_p--) { + if(*context_p != *cached_context_p) { + // The cached context doesn't match the application context, so it is + // invalid + return false; + } + } + + if(cached_context_p > cached_context) { + // if the cached context is longer than the application context, then we also + // assume that it is invalid + return false; + } + + // It's acceptable for the application context to be longer than the cached + // context, so if we match the whole cached context, we can safely return true + return true; +} +km_core_status km_core_state_context_validate( + km_core_state *state, + km_core_cp const *application_context +) { + assert(state != nullptr); + assert(application_context != nullptr); + if(state == nullptr || application_context == nullptr) { + return KM_CORE_STATUS_INVALID_ARGUMENT; + } + + size_t buf_size; + km_core_status status; + km_core_context_item* context_items = nullptr; + + auto context = km_core_state_context(state); + if((status = km_core_context_get(context, &context_items)) != KM_CORE_STATUS_OK) { + return status; + } + + if((status = km_core_context_items_to_utf16(context_items, nullptr, &buf_size)) != KM_CORE_STATUS_OK) { + km_core_context_items_dispose(context_items); + return status; + } + + km_core_cp* cached_context = new km_core_cp[buf_size]; + + status = km_core_context_items_to_utf16(context_items, cached_context, &buf_size); + km_core_context_items_dispose(context_items); + + if(status != KM_CORE_STATUS_OK) { + delete[] cached_context; + return status; + } + + bool is_valid = is_context_valid(application_context, cached_context); + + delete[] cached_context; + + if(is_valid) { + // We keep the context as is + return KM_CORE_STATUS_OK; + } + + // We replace the cached context with the current application context + status = km_core_context_items_from_utf16(application_context, &context_items); + if (status == KM_CORE_STATUS_OK) { + km_core_context_set(context, context_items); + km_core_context_items_dispose(context_items); + } else { + // TODO: DebugLog as this is a fail case + km_core_context_clear(context); + } + + return KM_CORE_STATUS_OK; +} + + +km_core_status km_core_state_context_invalidate( + km_core_state *state +) { + assert(state != nullptr); + if(state == nullptr) { + return KM_CORE_STATUS_INVALID_ARGUMENT; + } + km_core_context_clear(km_core_state_context(state)); + return KM_CORE_STATUS_OK; +} \ No newline at end of file diff --git a/core/src/meson.build b/core/src/meson.build index 51df8c6b57f..6c17948ff21 100644 --- a/core/src/meson.build +++ b/core/src/meson.build @@ -42,10 +42,12 @@ endif kmx_files = files( + 'action.cpp', 'option.cpp', 'keyboard.cpp', 'state.cpp', 'debuglog.cpp', + 'km_core_action_api.cpp', 'km_core_context_api.cpp', 'km_core_keyboard_api.cpp', 'km_core_options_api.cpp', @@ -74,6 +76,7 @@ kmx_files = files( ) api_files = files( + 'km_core_action_api.cpp', 'km_core_context_api.cpp', 'km_core_keyboard_api.cpp', 'km_core_options_api.cpp', @@ -83,6 +86,7 @@ api_files = files( ) core_files = files( + 'action.cpp', 'option.cpp', 'keyboard.cpp', 'state.cpp', diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp new file mode 100644 index 00000000000..7e18ec2e8a6 --- /dev/null +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -0,0 +1,432 @@ +/* + Copyright: © 2018 SIL International. + Description: Tests for the context API family of functions. + Create Date: 19 Oct 2018 + Authors: Tim Eves (TSE) + History: 19 Oct 2018 - TSE - Initial implementation. + 22 Oct 2018 - TSE - Refactor to add and use try_status macro + for improved readability. + - Add more tests to cover corner cases and + mutation functions. +*/ +#include +#include + +#include "path.hpp" +#include "action.hpp" +#include "utfcodec.hpp" +#include + +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(uintptr_t marker); + +//------------------------------------------------------------------------------------- + +void test_two_backspaces() { + const km_core_action_item action_items[] = { + char_action_item('D'), + bksp_action_item(KM_CORE_BT_CHAR, 'D'), + bksp_action_item(KM_CORE_BT_CHAR, 'E'), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 1); + assert(std::u32string(actions->output) == U""); + assert(actions->persist_options == nullptr); + assert(actions->do_alert == false); + assert(actions->emit_keystroke == false); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- + +void test_marker_text_interleaved() { + const km_core_action_item action_items[] = { + char_action_item('A'), + marker_action_item(1), + char_action_item('B'), + marker_action_item(2), + char_action_item('C'), + bksp_action_item(KM_CORE_BT_CHAR, 'C'), + bksp_action_item(KM_CORE_BT_MARKER, 2), + char_action_item('D'), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 0); + assert(std::u32string(actions->output) == U"ABD"); + assert(actions->persist_options == nullptr); + assert(actions->do_alert == false); + assert(actions->emit_keystroke == false); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- + +void test_alert() { + const km_core_action_item action_items[] = { + alert_action_item(), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 0); + assert(std::u32string(actions->output) == U""); + assert(actions->persist_options == nullptr); + assert(actions->do_alert == true); + assert(actions->emit_keystroke == false); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- + +void test_emit_keystroke() { + const km_core_action_item action_items[] = { + emit_keystroke_action_item(), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 0); + assert(std::u32string(actions->output) == U""); + assert(actions->persist_options == nullptr); + assert(actions->do_alert == false); + assert(actions->emit_keystroke == true); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- + +void test_invalidate_context() { + // note, this generates a no-op + const km_core_action_item action_items[] = { + invalidate_context_action_item(), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 0); + assert(std::u32string(actions->output) == U""); + assert(actions->persist_options == nullptr); + assert(actions->do_alert == false); + assert(actions->emit_keystroke == false); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- + +void test_persist_opt() { + const km_core_option_item option = { + u"key", + u"value", + KM_CORE_OPT_KEYBOARD + }; + + const km_core_action_item action_items[] = { + persist_opt_action_item(&option), + end_action_item() + }; + + km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + + assert(actions->delete_back == 0); + assert(std::u32string(actions->output) == U""); + assert(actions->persist_options != nullptr); + assert(std::u16string(actions->persist_options[0].key) == u"key"); + assert(std::u16string(actions->persist_options[0].value) == u"value"); + assert(actions->persist_options[0].scope == KM_CORE_OPT_KEYBOARD); + + // verify that data is copied + assert(actions->persist_options[0].key != option.key); + assert(actions->persist_options[0].value != option.value); + + // verify that we have a KM_CORE_OPTIONS_END term + assert(actions->persist_options[1].key == nullptr); + assert(actions->persist_options[1].value == nullptr); + assert(actions->persist_options[1].scope == KM_CORE_OPT_UNKNOWN); + + assert(actions->do_alert == false); + assert(actions->emit_keystroke == false); + assert(actions->new_caps_lock_state == -1); + + try_status(km_core_actions_dispose(actions)); +} + +//------------------------------------------------------------------------------------- +// Context tests +//------------------------------------------------------------------------------------- + +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; +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::kbp::path path = km::kbp::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)); +} + +bool is_identical_context(km_core_cp const *cached_context) { + size_t buf_size; + try_status(km_core_context_get(km_core_state_context(test_state), &citems)); + try_status(km_core_context_items_to_utf16(citems, nullptr, &buf_size)); + km_core_cp* new_cached_context = new km_core_cp[buf_size]; + try_status(km_core_context_items_to_utf16(citems, new_cached_context, &buf_size)); + bool result = std::u16string(cached_context) == new_cached_context; + delete[] new_cached_context; + return result; +} + +void test_context_validate_identical_context() { + km_core_cp const *application_context = u"This is a test"; + km_core_cp const *cached_context = u"This is a test"; + setup("k_000___null_keyboard.kmx", cached_context); + try_status(km_core_state_context_validate(test_state, application_context)); + assert(is_identical_context(cached_context)); + teardown(); +} + +void test_context_validate_different_context() { + km_core_cp const *application_context = u"This is a test"; + km_core_cp const *cached_context = u"This isn't a test"; + setup("k_000___null_keyboard.kmx", cached_context); + try_status(km_core_state_context_validate(test_state, application_context)); + assert(!is_identical_context(cached_context)); + assert(is_identical_context(application_context)); + teardown(); +} + +void test_context_validate_app_context_is_longer() { + km_core_cp const *application_context = u"Longer This is a test"; + km_core_cp const *cached_context = u"This is a test"; + setup("k_000___null_keyboard.kmx", cached_context); + try_status(km_core_state_context_validate(test_state, application_context)); + // Should be true -- longer, but what exists is identical to cached + assert(is_identical_context(cached_context)); + teardown(); +} + +void test_context_validate_app_context_is_shorter() { + km_core_cp const *application_context = u"is a test"; + km_core_cp const *cached_context = u"This is a test"; + setup("k_000___null_keyboard.kmx", cached_context); + try_status(km_core_state_context_validate(test_state, application_context)); + // Should be false -- app ctxt is shorter, so doesn't matter that what we have + // matches + assert(!is_identical_context(cached_context)); + assert(is_identical_context(application_context)); + teardown(); +} + +void test_context_validate_cached_context_has_markers() { + km_core_cp const *application_context = u"123"; + km_core_cp const *cached_context = u"123"; + setup("k_000___null_keyboard.kmx", cached_context); + + km_core_context_item const citems[] = { + { KM_CORE_CT_MARKER, {0}, { 5 } }, + { KM_CORE_CT_CHAR, {0}, { '1' } }, + { KM_CORE_CT_MARKER, {0}, { 1 } }, + { KM_CORE_CT_CHAR, {0}, { '2' } }, + { KM_CORE_CT_MARKER, {0}, { 2 } }, + { KM_CORE_CT_CHAR, {0}, { '3' } }, + { KM_CORE_CT_MARKER, {0}, { 3 } }, + { KM_CORE_CT_MARKER, {0}, { 4 } }, + KM_CORE_CONTEXT_ITEM_END + }; + + try_status(km_core_context_set(km_core_state_context(test_state), citems)); + try_status(km_core_state_context_validate(test_state, application_context)); + + km_core_context_item* citems_new; + + try_status(km_core_context_get(km_core_state_context(test_state), &citems_new)); + + for(int i = 0; citems[i].type || citems_new[i].type; i++) { + assert(citems_new[i].type == citems[i].type); + if(citems[i].type == KM_CORE_CT_CHAR) { + assert(citems_new[i].character == citems[i].character); + } else { + assert(citems_new[i].marker == citems[i].marker); + } + } + + teardown(); +} + +void test_context_validate() { + test_context_validate_identical_context(); + test_context_validate_different_context(); + test_context_validate_app_context_is_longer(); + test_context_validate_app_context_is_shorter(); + test_context_validate_cached_context_has_markers(); +} + +void test_context_invalidate() { + km_core_cp const *cached_context = u"This is a test"; + setup("k_000___null_keyboard.kmx", cached_context); + try_status(km_core_state_context_invalidate(test_state)); + assert(!is_identical_context(cached_context)); + assert(is_identical_context(u"")); + teardown(); +} + +//------------------------------------------------------------------------------------- +// Launcher +//------------------------------------------------------------------------------------- + +constexpr const auto help_str = "\ +action_api [--color] \n\ +\n\ + --color Force color output\n\ + SOURCE_PATH Path where debug_api.cpp is found; kmx files are\n\ + located relative to this path.\n"; + +int error_args() { + std::cerr << "debug_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_marker_text_interleaved(); + test_alert(); + test_emit_keystroke(); + test_invalidate_context(); + + // context -- todo move to another file + test_context_validate(); + test_context_invalidate(); +} + +//------------------------------------------------------------------------------------- +// 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(uintptr_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 3643a7a11f4..a3ec15a7aad 100644 --- a/core/tests/unit/kmnkbd/meson.build +++ b/core/tests/unit/kmnkbd/meson.build @@ -16,6 +16,7 @@ endif local_defns = ['-DKM_CORE_LIBRARY_STATIC'] tests = [ + ['action-api', 'action_api.cpp'], ['context-api', 'context_api.cpp'], ['keyboard-api', 'keyboard_api.cpp'], ['options-api', 'options_api.cpp'], From 051e9f2b4d89c8d31ab65051d586207a13945cd1 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Mon, 23 Oct 2023 15:16:59 +0700 Subject: [PATCH 02/11] chore(core): c++ ftw --- core/src/action.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/action.cpp b/core/src/action.cpp index d957b318c13..1531652ba17 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -89,7 +89,12 @@ km_core_actions * km::kbp::action_items_to_actions( // no-op break; case KM_CORE_IT_MARKER: - output.push_back({KM_CORE_CT_MARKER,{0},action_items->marker}); + { + km_core_context_item ci = {0}; + ci.type = KM_CORE_CT_MARKER; + ci.marker = action_items->marker; + output.push_back(ci); + } break; case KM_CORE_IT_PERSIST_OPT: // TODO: lowpri: replace existing item if already present in options vector? From 0aa86501c699181f03615461afefaaa652a436fc Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 09:25:08 +0700 Subject: [PATCH 03/11] chore(core): fix types and compile errors Fixes #9832. Addresses a couple of other cross-platform compile issues. --- core/include/keyman/keyman_core_api.h | 4 ++-- core/src/action.cpp | 11 +++-------- core/src/state.hpp | 4 ++-- core/tests/unit/kmnkbd/action_api.cpp | 7 ++++--- core/tests/unit/kmnkbd/debug_api.cpp | 5 ++++- core/tests/unit/kmnkbd/state_api.cpp | 6 ++++-- developer/src/tike/main/Keyman.System.KeymanCore.pas | 2 +- .../keyman-engine-tests/kmprocessactionstests.cpp | 4 ++-- 8 files changed, 22 insertions(+), 21 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index bd779a55a16..6375ef52051 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -533,10 +533,10 @@ typedef struct { uint8_t type; uint8_t _reserved[sizeof(void*)-sizeof(uint8_t)]; union { - uintptr_t marker; // MARKER type + uint32_t marker; // MARKER type km_core_option_item const * option; // OPT types km_core_usv character; // CHAR type - uint8_t capsLock; // CAPSLOCK type, 1 to turn on, 0 to turn off + uint8_t capsLock; // CAPSLOCK type, 1 to turn on, 0 to turn off; re name see #9833 km_core_backspace_item backspace; // BACKSPACE type }; } km_core_action_item; diff --git a/core/src/action.cpp b/core/src/action.cpp index 1531652ba17..9d24e082eb9 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -80,7 +80,7 @@ km_core_actions * km::kbp::action_items_to_actions( actions->new_caps_lock_state = action_items->capsLock; break; case KM_CORE_IT_CHAR: - output.push_back({KM_CORE_CT_CHAR,{0},action_items->character}); + output.push_back({KM_CORE_CT_CHAR,{0},{action_items->character}}); break; case KM_CORE_IT_EMIT_KEYSTROKE: actions->emit_keystroke = true; @@ -89,12 +89,7 @@ km_core_actions * km::kbp::action_items_to_actions( // no-op break; case KM_CORE_IT_MARKER: - { - km_core_context_item ci = {0}; - ci.type = KM_CORE_CT_MARKER; - ci.marker = action_items->marker; - output.push_back(ci); - } + output.push_back({KM_CORE_CT_MARKER,{0},{action_items->marker}}); break; case KM_CORE_IT_PERSIST_OPT: // TODO: lowpri: replace existing item if already present in options vector? @@ -109,7 +104,7 @@ km_core_actions * km::kbp::action_items_to_actions( } } - output.push_back({KM_CORE_CT_END}); + output.push_back(KM_CORE_CONTEXT_ITEM_END); // Strip the markers from the output to be passed to the app diff --git a/core/src/state.hpp b/core/src/state.hpp index 04bb294931f..961d5010655 100644 --- a/core/src/state.hpp +++ b/core/src/state.hpp @@ -37,7 +37,7 @@ class actions : public std::vector actions(Args&&... args); void push_character(km_core_usv usv); - void push_marker(uintptr_t marker); + void push_marker(uint32_t marker); void push_alert(); void push_backspace(km_core_backspace_type expected_type, uintptr_t expected_value = 0); void push_persist(option const &); @@ -68,7 +68,7 @@ void actions::push_character(km_core_usv usv) { inline -void actions::push_marker(uintptr_t marker) { +void actions::push_marker(uint32_t marker) { assert(empty() || (!empty() && back().type != KM_CORE_IT_END)); emplace_back(km_core_action_item {KM_CORE_IT_MARKER, {0,}, {marker}}); } diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index 7e18ec2e8a6..22c8edca4ec 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -14,8 +14,9 @@ #include "path.hpp" #include "action.hpp" -#include "utfcodec.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); @@ -25,7 +26,7 @@ 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(uintptr_t marker); +const km_core_action_item marker_action_item(uint32_t marker); //------------------------------------------------------------------------------------- @@ -424,7 +425,7 @@ const km_core_action_item invalidate_context_action_item() { return res; } -const km_core_action_item marker_action_item(uintptr_t marker) { +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; diff --git a/core/tests/unit/kmnkbd/debug_api.cpp b/core/tests/unit/kmnkbd/debug_api.cpp index b23aaf5176a..f1da37e86ea 100644 --- a/core/tests/unit/kmnkbd/debug_api.cpp +++ b/core/tests/unit/kmnkbd/debug_api.cpp @@ -402,8 +402,11 @@ void test_save_option() { km_core_state_debug_item{KM_CORE_DEBUG_END, 0, {}, {u"", nullptr, nullptr, {}, 1}}, })); + km_core_action_item action = {KM_CORE_IT_PERSIST_OPT, {0,}, }; + action.option = &opt; + assert(action_items(test_state, { - {KM_CORE_IT_PERSIST_OPT, {0,}, {uintptr_t(&opt)}}, + action, {KM_CORE_IT_END} })); } diff --git a/core/tests/unit/kmnkbd/state_api.cpp b/core/tests/unit/kmnkbd/state_api.cpp index a228996127e..0432bfa1fdb 100644 --- a/core/tests/unit/kmnkbd/state_api.cpp +++ b/core/tests/unit/kmnkbd/state_api.cpp @@ -170,8 +170,10 @@ int main(int argc, char * argv[]) KM_CORE_MODIFIER_SHIFT, 1, KM_CORE_EVENT_FLAG_DEFAULT)); assert(action_items(test_state, {{KM_CORE_IT_CHAR, {0,}, {km_core_usv('L')}}, {KM_CORE_IT_END}})); try_status(km_core_process_event(test_state, KM_CORE_VKEY_F2, 0, 1, KM_CORE_EVENT_FLAG_DEFAULT)); - assert(action_items(test_state, {{KM_CORE_IT_PERSIST_OPT, {0,}, - {uintptr_t(&expected_persist_opt)}}, {KM_CORE_IT_END}})); + + km_core_action_item action = {KM_CORE_IT_PERSIST_OPT, {0,}, }; + action.option = &expected_persist_opt; + assert(action_items(test_state, {action, {KM_CORE_IT_END}})); // Test debug dump auto doc1 = get_json_doc(*test_state), diff --git a/developer/src/tike/main/Keyman.System.KeymanCore.pas b/developer/src/tike/main/Keyman.System.KeymanCore.pas index 8abe0733175..85edf0db84e 100644 --- a/developer/src/tike/main/Keyman.System.KeymanCore.pas +++ b/developer/src/tike/main/Keyman.System.KeymanCore.pas @@ -192,7 +192,7 @@ km_core_action_item = record _reserved: array[0..2] of uint8_t; {$ENDIF} case Integer of - 0: (marker: uintptr_t); + 0: (marker: uint32_t); 1: (option: pkm_core_option_item); 2: (character: km_core_usv); 3: (backspace: km_core_backspace_item); diff --git a/windows/src/engine/keyman32/tests/keyman-engine-tests/kmprocessactionstests.cpp b/windows/src/engine/keyman32/tests/keyman-engine-tests/kmprocessactionstests.cpp index 7af218cd77b..362bc0497f3 100644 --- a/windows/src/engine/keyman32/tests/keyman-engine-tests/kmprocessactionstests.cpp +++ b/windows/src/engine/keyman32/tests/keyman-engine-tests/kmprocessactionstests.cpp @@ -49,7 +49,7 @@ TEST_F(KMPROCESSACTIONS, processMarkertest) { WCHAR callbuf[MAXCONTEXT]; AITIP testApp; WCHAR expectedContext[] = {UC_SENTINEL, CODE_DEADKEY, 2, 0}; - uintptr_t marker = 2; + uint32_t marker = 2; km_core_action_item itemAddMarker = {KM_CORE_IT_MARKER, {0,}, {marker}}; processMarker(&testApp, &itemAddMarker); @@ -67,7 +67,7 @@ TEST_F(KMPROCESSACTIONS, processBackDeadkeytest) { km_core_action_item itemAddChar = {KM_CORE_IT_CHAR, {0,}, {'A'}}; processUnicodeChar(&testApp, &itemAddChar); - uintptr_t marker = 2; + uint32_t marker = 2; km_core_action_item itemAddMarker = {KM_CORE_IT_MARKER, {0,}, {marker}}; processMarker(&testApp, &itemAddMarker); km_core_action_item itemBackSpace = {KM_CORE_IT_BACK}; From cbb708e74d4d813be6250ff051a7736cc3b2863e Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 09:48:08 +0700 Subject: [PATCH 04/11] chore(core): bool to int for cross-platform --- core/include/keyman/keyman_core_api.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 6375ef52051..53ccbf5d2c8 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -581,11 +581,11 @@ typedef struct { // list of options to persist, terminated with KM_CORE_OPTIONS_END km_core_option_item* persist_options; - // issue a beep - bool do_alert; + // issue a beep, 0 = no, 1 = yes + int do_alert; - // emit the input keystroke to the application, unmodified? - bool emit_keystroke; + // emit the input keystroke to the application, unmodified? 0 = no, 1 = yes + int emit_keystroke; // -1=unchanged, 0=off, 1=on int new_caps_lock_state; From cd428ae412168ac212edf3dd2fba8cf7f2e51eb3 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 12:22:34 +0700 Subject: [PATCH 05/11] feat(core): address review comments * Renamed various functions and types * Clarified return values * Updated doc comments --- core/include/keyman/keyman_core_api.h | 84 ++++++++++++++++--------- core/src/action.cpp | 20 +++--- core/src/action.hpp | 2 +- core/src/km_core_action_api.cpp | 2 +- core/src/km_core_state_api.cpp | 45 +++++++------- core/tests/unit/kmnkbd/action_api.cpp | 88 +++++++++++++-------------- 6 files changed, 131 insertions(+), 110 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 53ccbf5d2c8..6d0778f8dd7 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -253,8 +253,8 @@ km_core_context_items_from_utf8(char const *text, Convert a context item array into a UTF-16 encoded string placing it into the supplied buffer of specified size, and return the number of code units actually used in the conversion. If null is passed as the buffer the -number codeunits required is returned. This will strip markers from the -context during the conversion. +number of codeunits required is returned. Any markers in the context will +not be included in the output buffer. ##### Return status: - `KM_CORE_STATUS_OK`: On success. - `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. @@ -285,8 +285,8 @@ km_core_context_items_to_utf16(km_core_context_item const *item, Convert a context item array into a UTF-8 encoded string placing it into the supplied buffer of specified size, and return the number of code units actually used in the conversion. If null is passed as the buffer the -number codeunits required is returned. This will strip markers from the -context during the conversion. +number of codeunits required is returned. Any markers in the context will +not be included in the output buffer. ##### Return status: - `KM_CORE_STATUS_OK`: On success. - `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. @@ -317,8 +317,8 @@ km_core_context_items_to_utf8(km_core_context_item const *item, Convert a context item array into a UTF-32 encoded string placing it into the supplied buffer of specified size, and return the number of codepoints actually used in the conversion. If null is passed as the buffer the -number codepoints required is returned. This will strip markers from the -context during the conversion. +number of codepoints required is returned. Any markers in the context will +not be included in the output buffer. ##### Return status: - `KM_CORE_STATUS_OK`: On success. - `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. @@ -571,9 +571,12 @@ removed in the future. ```c */ +typedef enum { KM_CORE_FALSE = 0, KM_CORE_TRUE = 1 } km_core_bool; +typedef enum { KM_CORE_CAPS_UNCHANGED = -1, KM_CORE_CAPS_OFF = 0, KM_CORE_CAPS_ON = 1 } km_core_caps_state; + typedef struct { - // number of codepoints (not codeunits!) to delete from app context, 0+ - int delete_back; + // number of codepoints (not codeunits!) to delete from app context. + unsigned int delete_back_codepoints; // null-term string of characters to insert into document km_core_usv* output; @@ -582,13 +585,13 @@ typedef struct { km_core_option_item* persist_options; // issue a beep, 0 = no, 1 = yes - int do_alert; + km_core_bool do_alert; // emit the input keystroke to the application, unmodified? 0 = no, 1 = yes - int emit_keystroke; + km_core_bool emit_keystroke; // -1=unchanged, 0=off, 1=on - int new_caps_lock_state; + km_core_caps_state new_caps_lock_state; } km_core_actions; /* @@ -596,7 +599,7 @@ typedef struct { ### `km_core_state_get_actions` ##### Description: Returns a pointer to an actions object which details all the actions -that the Platform layer must take after a keystroke. The `delete_back` +that the Platform layer must take after a keystroke. The `delete_back_codepoints` action must be performed before the `output` action, but the other actions may be performed in any order. ##### Return: @@ -632,52 +635,73 @@ km_core_actions_dispose( /* ``` -### `km_core_state_context_validate` +### `km_core_state_context_set_if_needed` ##### Description: -Determines if the input context has changed, and if so, resets -the internal cached context (including markers), to the new -context string. +Sets the internal cached context for the state object, to the passed-in +application context string, if it differs from the codepoints in the +cached context. For the purposes of comparison, (1) cached markers are +ignored, (2) if the cached context is shorter than the application +context, it is considered identical, but (3) if the cached context is +longer, then it is considered different. + +If a difference is found, then the cached context will be set to the +application context, and thus any cached markers will be cleared. -This and `km_core_state_context_invalidate` will replace existing Core context -APIs. +`km_core_state_context_set_if_needed` and `km_core_state_context_clear` +will replace most uses of the existing Core context APIs. ##### Parameters: - __state__: An opaque pointer to a state object. -- __application_context__: A pointer to an null-terminated `km_core_cp` string - representing the current context from the application. +- __application_context__: A pointer to an null-terminated `km_core_cp` + string representing the current context from the application. ##### Return status: -- `KM_CORE_STATUS_OK`: On success. -- `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. +- `KM_CORE_CONTEXT_STATUS_UNCHANGED`: Cached context change was not needed +- `KM_CORE_CONTEXT_STATUS_UPDATED`: Cached context was set to application + context +- `KM_CORE_CONTEXT_STATUS_CLEARED`: Application context was invalid, perhaps + had unpaired surrogates, and so cached context was cleared instead +- `KM_CORE_CONTEXT_STATUS_ERROR`: Internal error +- `KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT`: One or more parameters was null ```c */ + +typedef enum { + KM_CORE_CONTEXT_STATUS_UNCHANGED = 0, // Cached context change was not needed + KM_CORE_CONTEXT_STATUS_UPDATED = 1, // Cached context was set to application context + KM_CORE_CONTEXT_STATUS_CLEARED = 2, // Application context was invalid, context was cleared + KM_CORE_CONTEXT_STATUS_ERROR = 3, // Internal error + KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT = 4, // Invalid arguments +} km_core_context_status; + KMN_API -km_core_status -km_core_state_context_validate( +km_core_context_status +km_core_state_context_set_if_needed( km_core_state *state, km_core_cp const *application_context ); /* ``` -### `km_core_state_context_invalidate` +### `km_core_state_context_clear` ##### Description: -Flushes the internal cached context for the state. +Clears the internal cached context for the state. This is the same as +`km_core_context_clear(km_core_state_context(&state))`. -This and `km_core_state_context_validate` will replace existing Core context -APIs. +`km_core_state_context_set_if_needed` and `km_core_state_context_clear` +will replace most uses of the existing Core context APIs. ##### Parameters: - __state__: An opaque pointer to a state object. ##### Return status: - `KM_CORE_STATUS_OK`: On success. -- `KM_CORE_STATUS_INVALID_ARGUMENT`: If non-optional parameters are null. +- `KM_CORE_STATUS_INVALID_ARGUMENT`: If any parameters are null. ```c */ KMN_API km_core_status -km_core_state_context_invalidate( +km_core_state_context_clear( km_core_state *state ); diff --git a/core/src/action.cpp b/core/src/action.cpp index 9d24e082eb9..0ef134c4fbb 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -16,7 +16,7 @@ #include "state.hpp" #include "option.hpp" -km_core_actions * km::kbp::action_items_to_actions( +km_core_actions * km::kbp::action_item_list_to_actions_object( km_core_action_item const *action_items ) { assert(action_items != nullptr); @@ -31,30 +31,30 @@ km_core_actions * km::kbp::action_items_to_actions( // Set actions defaults std::vector output; std::vector options; - actions->delete_back = 0; + actions->delete_back_codepoints = 0; actions->output = nullptr; actions->persist_options = nullptr; - actions->do_alert = false; - actions->emit_keystroke = false; - actions->new_caps_lock_state = -1; + actions->do_alert = KM_CORE_FALSE; + actions->emit_keystroke = KM_CORE_FALSE; + actions->new_caps_lock_state = KM_CORE_CAPS_UNCHANGED; for (; action_items->type != KM_CORE_IT_END; ++action_items) { assert(action_items->type < KM_CORE_IT_MAX_TYPE_ID); switch(action_items->type) { case KM_CORE_IT_ALERT: - actions->do_alert = true; + actions->do_alert = KM_CORE_TRUE; break; case KM_CORE_IT_BACK: switch(action_items->backspace.expected_type) { case KM_CORE_BT_UNKNOWN: // this is equivalent to emit_keystroke, because the only time we // are allowed to do an unknown bksp is when a bksp is passed in - actions->emit_keystroke = true; + actions->emit_keystroke = KM_CORE_TRUE; break; case KM_CORE_BT_CHAR: if(output.empty()) { - actions->delete_back++; + actions->delete_back_codepoints++; } else { auto last_context_item = output.back(); output.pop_back(); @@ -77,13 +77,13 @@ km_core_actions * km::kbp::action_items_to_actions( } break; case KM_CORE_IT_CAPSLOCK: - actions->new_caps_lock_state = action_items->capsLock; + actions->new_caps_lock_state = action_items->capsLock ? KM_CORE_CAPS_ON : KM_CORE_CAPS_OFF; break; case KM_CORE_IT_CHAR: output.push_back({KM_CORE_CT_CHAR,{0},{action_items->character}}); break; case KM_CORE_IT_EMIT_KEYSTROKE: - actions->emit_keystroke = true; + actions->emit_keystroke = KM_CORE_TRUE; break; case KM_CORE_IT_INVALIDATE_CONTEXT: // no-op diff --git a/core/src/action.hpp b/core/src/action.hpp index 115d328a8c0..c84ba1ae094 100644 --- a/core/src/action.hpp +++ b/core/src/action.hpp @@ -13,7 +13,7 @@ namespace km { namespace kbp { - km_core_actions* action_items_to_actions( + km_core_actions* action_item_list_to_actions_object( km_core_action_item const *action_items ); } // namespace kbp diff --git a/core/src/km_core_action_api.cpp b/core/src/km_core_action_api.cpp index 057d1338d12..b8fc351fa8b 100644 --- a/core/src/km_core_action_api.cpp +++ b/core/src/km_core_action_api.cpp @@ -33,7 +33,7 @@ km_core_actions* km_core_state_get_actions( return nullptr; } - actions = action_items_to_actions(action_items); + actions = action_item_list_to_actions_object(action_items); return actions; } diff --git a/core/src/km_core_state_api.cpp b/core/src/km_core_state_api.cpp index 49776d5125d..f3a9d6656a1 100644 --- a/core/src/km_core_state_api.cpp +++ b/core/src/km_core_state_api.cpp @@ -260,7 +260,6 @@ void km_core_state_imx_deregister_callback(km_core_state *state) state->imx_deregister_callback(); } - bool is_context_valid(km_core_cp const * context, km_core_cp const * cached_context) { km_core_cp const* context_p = context; while(*context_p) { @@ -291,64 +290,62 @@ bool is_context_valid(km_core_cp const * context, km_core_cp const * cached_cont // context, so if we match the whole cached context, we can safely return true return true; } -km_core_status km_core_state_context_validate( + +km_core_context_status km_core_state_context_set_if_needed( km_core_state *state, km_core_cp const *application_context ) { assert(state != nullptr); assert(application_context != nullptr); if(state == nullptr || application_context == nullptr) { - return KM_CORE_STATUS_INVALID_ARGUMENT; + return KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT; } size_t buf_size; - km_core_status status; km_core_context_item* context_items = nullptr; auto context = km_core_state_context(state); - if((status = km_core_context_get(context, &context_items)) != KM_CORE_STATUS_OK) { - return status; + if(km_core_context_get(context, &context_items) != KM_CORE_STATUS_OK) { + return KM_CORE_CONTEXT_STATUS_ERROR; } - if((status = km_core_context_items_to_utf16(context_items, nullptr, &buf_size)) != KM_CORE_STATUS_OK) { + if(km_core_context_items_to_utf16(context_items, nullptr, &buf_size)) { km_core_context_items_dispose(context_items); - return status; + return KM_CORE_CONTEXT_STATUS_ERROR; } - km_core_cp* cached_context = new km_core_cp[buf_size]; + std::unique_ptr cached_context(new km_core_cp[buf_size]); - status = km_core_context_items_to_utf16(context_items, cached_context, &buf_size); + km_core_status status = km_core_context_items_to_utf16(context_items, cached_context.get(), &buf_size); km_core_context_items_dispose(context_items); if(status != KM_CORE_STATUS_OK) { - delete[] cached_context; - return status; + return KM_CORE_CONTEXT_STATUS_ERROR; } - bool is_valid = is_context_valid(application_context, cached_context); - - delete[] cached_context; + bool is_valid = is_context_valid(application_context, cached_context.get()); if(is_valid) { // We keep the context as is - return KM_CORE_STATUS_OK; + return KM_CORE_CONTEXT_STATUS_UNCHANGED; } + km_core_context_item* new_context_items = nullptr; + // We replace the cached context with the current application context - status = km_core_context_items_from_utf16(application_context, &context_items); - if (status == KM_CORE_STATUS_OK) { - km_core_context_set(context, context_items); - km_core_context_items_dispose(context_items); - } else { - // TODO: DebugLog as this is a fail case + status = km_core_context_items_from_utf16(application_context, &new_context_items); + if (status != KM_CORE_STATUS_OK) { km_core_context_clear(context); + return KM_CORE_CONTEXT_STATUS_CLEARED; } - return KM_CORE_STATUS_OK; + km_core_context_set(context, new_context_items); + km_core_context_items_dispose(new_context_items); + return KM_CORE_CONTEXT_STATUS_UPDATED; } -km_core_status km_core_state_context_invalidate( +km_core_status km_core_state_context_clear( km_core_state *state ) { assert(state != nullptr); diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index 22c8edca4ec..82519312153 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -38,9 +38,9 @@ void test_two_backspaces() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 1); + assert(actions->delete_back_codepoints == 1); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); assert(actions->do_alert == false); @@ -65,9 +65,9 @@ void test_marker_text_interleaved() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 0); + assert(actions->delete_back_codepoints == 0); assert(std::u32string(actions->output) == U"ABD"); assert(actions->persist_options == nullptr); assert(actions->do_alert == false); @@ -85,14 +85,14 @@ void test_alert() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 0); + assert(actions->delete_back_codepoints == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); - assert(actions->do_alert == true); - assert(actions->emit_keystroke == false); - assert(actions->new_caps_lock_state == -1); + assert(actions->do_alert == KM_CORE_TRUE); + assert(actions->emit_keystroke == KM_CORE_FALSE); + assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); try_status(km_core_actions_dispose(actions)); } @@ -105,14 +105,14 @@ void test_emit_keystroke() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 0); + assert(actions->delete_back_codepoints == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); - assert(actions->do_alert == false); - assert(actions->emit_keystroke == true); - assert(actions->new_caps_lock_state == -1); + assert(actions->do_alert == KM_CORE_FALSE); + assert(actions->emit_keystroke == KM_CORE_TRUE); + assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); try_status(km_core_actions_dispose(actions)); } @@ -126,14 +126,14 @@ void test_invalidate_context() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 0); + assert(actions->delete_back_codepoints == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); - assert(actions->do_alert == false); - assert(actions->emit_keystroke == false); - assert(actions->new_caps_lock_state == -1); + assert(actions->do_alert == KM_CORE_FALSE); + assert(actions->emit_keystroke == KM_CORE_FALSE); + assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); try_status(km_core_actions_dispose(actions)); } @@ -152,9 +152,9 @@ void test_persist_opt() { end_action_item() }; - km_core_actions *actions = km::kbp::action_items_to_actions(action_items); + km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back == 0); + assert(actions->delete_back_codepoints == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options != nullptr); assert(std::u16string(actions->persist_options[0].key) == u"key"); @@ -170,9 +170,9 @@ void test_persist_opt() { assert(actions->persist_options[1].value == nullptr); assert(actions->persist_options[1].scope == KM_CORE_OPT_UNKNOWN); - assert(actions->do_alert == false); - assert(actions->emit_keystroke == false); - assert(actions->new_caps_lock_state == -1); + assert(actions->do_alert == KM_CORE_FALSE); + assert(actions->emit_keystroke == KM_CORE_FALSE); + assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); try_status(km_core_actions_dispose(actions)); } @@ -227,40 +227,40 @@ bool is_identical_context(km_core_cp const *cached_context) { return result; } -void test_context_validate_identical_context() { +void test_context_set_if_needed_identical_context() { km_core_cp const *application_context = u"This is a test"; km_core_cp const *cached_context = u"This is a test"; setup("k_000___null_keyboard.kmx", cached_context); - try_status(km_core_state_context_validate(test_state, application_context)); + assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UNCHANGED); assert(is_identical_context(cached_context)); teardown(); } -void test_context_validate_different_context() { +void test_context_set_if_needed_different_context() { km_core_cp const *application_context = u"This is a test"; km_core_cp const *cached_context = u"This isn't a test"; setup("k_000___null_keyboard.kmx", cached_context); - try_status(km_core_state_context_validate(test_state, application_context)); + assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UPDATED); assert(!is_identical_context(cached_context)); assert(is_identical_context(application_context)); teardown(); } -void test_context_validate_app_context_is_longer() { +void test_context_set_if_needed_app_context_is_longer() { km_core_cp const *application_context = u"Longer This is a test"; km_core_cp const *cached_context = u"This is a test"; setup("k_000___null_keyboard.kmx", cached_context); - try_status(km_core_state_context_validate(test_state, application_context)); + assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UNCHANGED); // Should be true -- longer, but what exists is identical to cached assert(is_identical_context(cached_context)); teardown(); } -void test_context_validate_app_context_is_shorter() { +void test_context_set_if_needed_app_context_is_shorter() { km_core_cp const *application_context = u"is a test"; km_core_cp const *cached_context = u"This is a test"; setup("k_000___null_keyboard.kmx", cached_context); - try_status(km_core_state_context_validate(test_state, application_context)); + assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UPDATED); // Should be false -- app ctxt is shorter, so doesn't matter that what we have // matches assert(!is_identical_context(cached_context)); @@ -268,7 +268,7 @@ void test_context_validate_app_context_is_shorter() { teardown(); } -void test_context_validate_cached_context_has_markers() { +void test_context_set_if_needed_cached_context_has_markers() { km_core_cp const *application_context = u"123"; km_core_cp const *cached_context = u"123"; setup("k_000___null_keyboard.kmx", cached_context); @@ -286,7 +286,7 @@ void test_context_validate_cached_context_has_markers() { }; try_status(km_core_context_set(km_core_state_context(test_state), citems)); - try_status(km_core_state_context_validate(test_state, application_context)); + assert(km_core_state_context_set_if_needed(test_state, application_context) == KM_CORE_CONTEXT_STATUS_UNCHANGED); km_core_context_item* citems_new; @@ -304,18 +304,18 @@ void test_context_validate_cached_context_has_markers() { teardown(); } -void test_context_validate() { - test_context_validate_identical_context(); - test_context_validate_different_context(); - test_context_validate_app_context_is_longer(); - test_context_validate_app_context_is_shorter(); - test_context_validate_cached_context_has_markers(); +void test_context_set_if_needed() { + test_context_set_if_needed_identical_context(); + test_context_set_if_needed_different_context(); + test_context_set_if_needed_app_context_is_longer(); + test_context_set_if_needed_app_context_is_shorter(); + test_context_set_if_needed_cached_context_has_markers(); } -void test_context_invalidate() { +void test_context_clear() { km_core_cp const *cached_context = u"This is a test"; setup("k_000___null_keyboard.kmx", cached_context); - try_status(km_core_state_context_invalidate(test_state)); + try_status(km_core_state_context_clear(test_state)); assert(!is_identical_context(cached_context)); assert(is_identical_context(u"")); teardown(); @@ -364,8 +364,8 @@ int main(int argc, char *argv []) { test_invalidate_context(); // context -- todo move to another file - test_context_validate(); - test_context_invalidate(); + test_context_set_if_needed(); + test_context_clear(); } //------------------------------------------------------------------------------------- From 243d1a8417d88b3a3b1af6c9bebb00049f38cdde Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 12:33:52 +0700 Subject: [PATCH 06/11] chore(core): cross-platform differences again --- core/src/km_core_state_api.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/km_core_state_api.cpp b/core/src/km_core_state_api.cpp index f3a9d6656a1..1a61727cde9 100644 --- a/core/src/km_core_state_api.cpp +++ b/core/src/km_core_state_api.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include "jsonpp.hpp" From c3bdce21280f22f7651c995064c4065e02d16405 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 12:58:46 +0700 Subject: [PATCH 07/11] chore(core): update debian symbols --- linux/debian/libkmnkbp0-0.symbols | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/linux/debian/libkmnkbp0-0.symbols b/linux/debian/libkmnkbp0-0.symbols index e507e05a2ca..982da90c64a 100644 --- a/linux/debian/libkmnkbp0-0.symbols +++ b/linux/debian/libkmnkbp0-0.symbols @@ -2,6 +2,7 @@ libkmnkbp0.so.0 libkmnkbp0-0 #MINVER# * Build-Depends-Package: libkmnkbp-dev km_core_state_get_intermediate_context@Base 17.0.193 + km_core_actions_dispose@Base 17.0.197 km_core_context_append@Base 15.0 km_core_context_clear@Base 15.0 km_core_context_get@Base 15.0 @@ -10,6 +11,7 @@ libkmnkbp0.so.0 libkmnkbp0-0 #MINVER# km_core_context_items_from_utf16@Base 15.0 km_core_context_items_from_utf8@Base 15.0 km_core_context_items_to_utf16@Base 15.0 + km_core_context_items_to_utf32@Base 17.0.197 km_core_context_items_to_utf8@Base 15.0 km_core_context_length@Base 15.0 km_core_context_set@Base 15.0 @@ -29,11 +31,14 @@ libkmnkbp0.so.0 libkmnkbp0-0 #MINVER# km_core_state_action_items@Base 15.0 km_core_state_clone@Base 15.0 km_core_state_context@Base 15.0 + km_core_state_context_clear@Base 17.0.197 + km_core_state_context_set_if_needed@Base 17.0.197 km_core_state_create@Base 15.0 km_core_state_debug_get@Base 15.0 km_core_state_debug_items@Base 15.0 km_core_state_debug_set@Base 15.0 km_core_state_dispose@Base 15.0 + km_core_state_get_actions@Base 17.0.197 km_core_state_imx_deregister_callback@Base 15.0 km_core_state_imx_register_callback@Base 15.0 km_core_state_option_lookup@Base 15.0 From 69e92f312a64e5e0ff339a8bbd87720417cb4241 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 13:46:52 +0700 Subject: [PATCH 08/11] chore(core): rename to code_points_to_delete --- core/include/keyman/keyman_core_api.h | 4 ++-- core/src/action.cpp | 4 ++-- core/tests/unit/kmnkbd/action_api.cpp | 12 ++++++------ 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 6d0778f8dd7..fb41142d1e2 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -576,7 +576,7 @@ typedef enum { KM_CORE_CAPS_UNCHANGED = -1, KM_CORE_CAPS_OFF = 0, KM_CORE_CAPS_O typedef struct { // number of codepoints (not codeunits!) to delete from app context. - unsigned int delete_back_codepoints; + unsigned int code_points_to_delete; // null-term string of characters to insert into document km_core_usv* output; @@ -599,7 +599,7 @@ typedef struct { ### `km_core_state_get_actions` ##### Description: Returns a pointer to an actions object which details all the actions -that the Platform layer must take after a keystroke. The `delete_back_codepoints` +that the Platform layer must take after a keystroke. The `code_points_to_delete` action must be performed before the `output` action, but the other actions may be performed in any order. ##### Return: diff --git a/core/src/action.cpp b/core/src/action.cpp index 0ef134c4fbb..db104f9f604 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -31,7 +31,7 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( // Set actions defaults std::vector output; std::vector options; - actions->delete_back_codepoints = 0; + actions->code_points_to_delete = 0; actions->output = nullptr; actions->persist_options = nullptr; actions->do_alert = KM_CORE_FALSE; @@ -54,7 +54,7 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( break; case KM_CORE_BT_CHAR: if(output.empty()) { - actions->delete_back_codepoints++; + actions->code_points_to_delete++; } else { auto last_context_item = output.back(); output.pop_back(); diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index 82519312153..e2d308651b8 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -40,7 +40,7 @@ void test_two_backspaces() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 1); + assert(actions->code_points_to_delete == 1); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); assert(actions->do_alert == false); @@ -67,7 +67,7 @@ void test_marker_text_interleaved() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 0); + assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U"ABD"); assert(actions->persist_options == nullptr); assert(actions->do_alert == false); @@ -87,7 +87,7 @@ void test_alert() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 0); + assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); assert(actions->do_alert == KM_CORE_TRUE); @@ -107,7 +107,7 @@ void test_emit_keystroke() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 0); + assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); assert(actions->do_alert == KM_CORE_FALSE); @@ -128,7 +128,7 @@ void test_invalidate_context() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 0); + assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options == nullptr); assert(actions->do_alert == KM_CORE_FALSE); @@ -154,7 +154,7 @@ void test_persist_opt() { km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); - assert(actions->delete_back_codepoints == 0); + assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); assert(actions->persist_options != nullptr); assert(std::u16string(actions->persist_options[0].key) == u"key"); From 40ca1d1ed819282787620796d5f9257d852c3837 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Tue, 24 Oct 2023 20:53:44 +0700 Subject: [PATCH 09/11] chore(core): address review comments --- core/include/keyman/keyman_core_api.h | 25 +++++++++++++++++-------- core/src/action.cpp | 13 ++++++------- core/src/km_core_state_api.cpp | 2 +- core/tests/unit/kmnkbd/action_api.cpp | 10 +++------- 4 files changed, 27 insertions(+), 23 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index fb41142d1e2..93b35786dc4 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -633,6 +633,23 @@ km_core_actions_dispose( km_core_actions* actions ); +/* +``` +### `km_core_context_status` +##### Description: +Return values for `km_core_state_context_set_if_needed`. + +```c +*/ + +typedef enum { + KM_CORE_CONTEXT_STATUS_UNCHANGED = 0, // Cached context change was not needed + KM_CORE_CONTEXT_STATUS_UPDATED = 1, // Cached context was set to application context + KM_CORE_CONTEXT_STATUS_CLEARED = 2, // Application context was invalid, context was cleared + KM_CORE_CONTEXT_STATUS_ERROR = 3, // Internal error + KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT = 4, // Invalid arguments +} km_core_context_status; + /* ``` ### `km_core_state_context_set_if_needed` @@ -666,14 +683,6 @@ will replace most uses of the existing Core context APIs. ```c */ -typedef enum { - KM_CORE_CONTEXT_STATUS_UNCHANGED = 0, // Cached context change was not needed - KM_CORE_CONTEXT_STATUS_UPDATED = 1, // Cached context was set to application context - KM_CORE_CONTEXT_STATUS_CLEARED = 2, // Application context was invalid, context was cleared - KM_CORE_CONTEXT_STATUS_ERROR = 3, // Internal error - KM_CORE_CONTEXT_STATUS_INVALID_ARGUMENT = 4, // Invalid arguments -} km_core_context_status; - KMN_API km_core_context_status km_core_state_context_set_if_needed( diff --git a/core/src/action.cpp b/core/src/action.cpp index db104f9f604..4dc8cdc14a7 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include @@ -26,7 +27,7 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( km_core_status status = KM_CORE_STATUS_OK; - km_core_actions* actions = new km_core_actions; + std::unique_ptr actions(new km_core_actions); // Set actions defaults std::vector output; @@ -111,15 +112,12 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( size_t buf_size; if((status = km_core_context_items_to_utf32(output.data(), nullptr, &buf_size)) != KM_CORE_STATUS_OK) { - delete actions; return nullptr; } - actions->output = new km_core_usv[buf_size]; + std::unique_ptr output_usv(new km_core_usv[buf_size]); - if((status = km_core_context_items_to_utf32(output.data(), actions->output, &buf_size)) != KM_CORE_STATUS_OK) { - delete[] actions->output; - delete actions; + if((status = km_core_context_items_to_utf32(output.data(), output_usv.get(), &buf_size)) != KM_CORE_STATUS_OK) { return nullptr; } @@ -131,5 +129,6 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( std::copy(options.begin(), options.end(), actions->persist_options); } - return actions; + actions->output = output_usv.release(); + return actions.release(); } diff --git a/core/src/km_core_state_api.cpp b/core/src/km_core_state_api.cpp index 1a61727cde9..2e7d6e3d602 100644 --- a/core/src/km_core_state_api.cpp +++ b/core/src/km_core_state_api.cpp @@ -310,7 +310,7 @@ km_core_context_status km_core_state_context_set_if_needed( return KM_CORE_CONTEXT_STATUS_ERROR; } - if(km_core_context_items_to_utf16(context_items, nullptr, &buf_size)) { + if(km_core_context_items_to_utf16(context_items, nullptr, &buf_size) != KM_CORE_STATUS_OK) { km_core_context_items_dispose(context_items); return KM_CORE_CONTEXT_STATUS_ERROR; } diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index e2d308651b8..b6a03726e18 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -1,13 +1,9 @@ /* Copyright: © 2018 SIL International. Description: Tests for the context API family of functions. - Create Date: 19 Oct 2018 - Authors: Tim Eves (TSE) - History: 19 Oct 2018 - TSE - Initial implementation. - 22 Oct 2018 - TSE - Refactor to add and use try_status macro - for improved readability. - - Add more tests to cover corner cases and - mutation functions. + Create Date: 23 Oct 2023 + Authors: Marc Durdin + History: 23 Oct 2023 - MCD - Initial implementation. */ #include #include From 82237924040186efa0854af0b784ad13a58bb369 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 25 Oct 2023 05:30:22 +0700 Subject: [PATCH 10/11] chore(core): rename kbp to core --- core/src/action.cpp | 4 ++-- core/src/action.hpp | 4 ++-- core/src/km_core_action_api.cpp | 2 +- core/tests/unit/kmnkbd/action_api.cpp | 14 +++++++------- linux/debian/libkeymancore.symbols | 5 +++++ 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/core/src/action.cpp b/core/src/action.cpp index 4dc8cdc14a7..d5d72decf4f 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::kbp::action_item_list_to_actions_object( +km_core_actions * km::core::action_item_list_to_actions_object( km_core_action_item const *action_items ) { assert(action_items != nullptr); @@ -94,7 +94,7 @@ km_core_actions * km::kbp::action_item_list_to_actions_object( break; case KM_CORE_IT_PERSIST_OPT: // TODO: lowpri: replace existing item if already present in options vector? - options.push_back(km::kbp::option( + options.push_back(km::core::option( static_cast(action_items->option->scope), action_items->option->key, action_items->option->value diff --git a/core/src/action.hpp b/core/src/action.hpp index c84ba1ae094..4cc0aabdf27 100644 --- a/core/src/action.hpp +++ b/core/src/action.hpp @@ -11,10 +11,10 @@ #include namespace km { -namespace kbp +namespace core { km_core_actions* action_item_list_to_actions_object( km_core_action_item const *action_items ); -} // namespace kbp +} // namespace core } // namespace km diff --git a/core/src/km_core_action_api.cpp b/core/src/km_core_action_api.cpp index b8fc351fa8b..d552d410592 100644 --- a/core/src/km_core_action_api.cpp +++ b/core/src/km_core_action_api.cpp @@ -17,7 +17,7 @@ #include "state.hpp" #include "action.hpp" -using namespace km::kbp; +using namespace km::core; km_core_actions* km_core_state_get_actions( km_core_state const *state diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index b6a03726e18..aee356fe4c7 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::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 1); assert(std::u32string(actions->output) == U""); @@ -61,7 +61,7 @@ void test_marker_text_interleaved() { end_action_item() }; - km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *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"); @@ -81,7 +81,7 @@ void test_alert() { end_action_item() }; - km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -101,7 +101,7 @@ void test_emit_keystroke() { end_action_item() }; - km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -122,7 +122,7 @@ void test_invalidate_context() { end_action_item() }; - km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -148,7 +148,7 @@ void test_persist_opt() { end_action_item() }; - km_core_actions *actions = km::kbp::action_item_list_to_actions_object(action_items); + km_core_actions *actions = km::core::action_item_list_to_actions_object(action_items); assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); @@ -205,7 +205,7 @@ void teardown() { void setup(const char *keyboard, const km_core_cp* context) { teardown(); - km::kbp::path path = km::kbp::path::join(arg_path, keyboard); + 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)); diff --git a/linux/debian/libkeymancore.symbols b/linux/debian/libkeymancore.symbols index 210008e9104..9482ded8ae5 100644 --- a/linux/debian/libkeymancore.symbols +++ b/linux/debian/libkeymancore.symbols @@ -1,6 +1,7 @@ libkeymancore.so.1 libkeymancore #MINVER# * Build-Depends-Package: libkeymancore-dev + km_core_actions_dispose@Base 17.0.197 km_core_context_append@Base 17.0.195 km_core_context_clear@Base 17.0.195 km_core_context_get@Base 17.0.195 @@ -9,6 +10,7 @@ libkeymancore.so.1 libkeymancore #MINVER# km_core_context_items_from_utf16@Base 17.0.195 km_core_context_items_from_utf8@Base 17.0.195 km_core_context_items_to_utf16@Base 17.0.195 + km_core_context_items_to_utf32@Base 17.0.197 km_core_context_items_to_utf8@Base 17.0.195 km_core_context_length@Base 17.0.195 km_core_context_set@Base 17.0.195 @@ -28,11 +30,14 @@ libkeymancore.so.1 libkeymancore #MINVER# km_core_state_action_items@Base 17.0.195 km_core_state_clone@Base 17.0.195 km_core_state_context@Base 17.0.195 + km_core_state_context_clear@Base 17.0.197 + km_core_state_context_set_if_needed@Base 17.0.197 km_core_state_create@Base 17.0.195 km_core_state_debug_get@Base 17.0.195 km_core_state_debug_items@Base 17.0.195 km_core_state_debug_set@Base 17.0.195 km_core_state_dispose@Base 17.0.195 + km_core_state_get_actions@Base 17.0.197 km_core_state_get_intermediate_context@Base 17.0.195 km_core_state_imx_deregister_callback@Base 17.0.195 km_core_state_imx_register_callback@Base 17.0.195 From 8475b6824839c08051d6cc127b09364c087f6981 Mon Sep 17 00:00:00 2001 From: Marc Durdin Date: Wed, 25 Oct 2023 05:46:43 +0700 Subject: [PATCH 11/11] feat(core): make persist_options always point to a valid array --- core/include/keyman/keyman_core_api.h | 2 +- core/src/action.cpp | 26 +++++++++++++++----------- core/tests/unit/kmnkbd/action_api.cpp | 26 +++++++++++++++++++++----- 3 files changed, 37 insertions(+), 17 deletions(-) diff --git a/core/include/keyman/keyman_core_api.h b/core/include/keyman/keyman_core_api.h index 042a6b71197..309fdb20e1b 100644 --- a/core/include/keyman/keyman_core_api.h +++ b/core/include/keyman/keyman_core_api.h @@ -587,7 +587,7 @@ typedef struct { // issue a beep, 0 = no, 1 = yes km_core_bool do_alert; - // emit the input keystroke to the application, unmodified? 0 = no, 1 = yes + // emit the (unmodified) input keystroke to the application, 0 = no, 1 = yes km_core_bool emit_keystroke; // -1=unchanged, 0=off, 1=on diff --git a/core/src/action.cpp b/core/src/action.cpp index d5d72decf4f..3387d8f85d7 100644 --- a/core/src/action.cpp +++ b/core/src/action.cpp @@ -29,16 +29,18 @@ km_core_actions * km::core::action_item_list_to_actions_object( std::unique_ptr actions(new km_core_actions); - // Set actions defaults + // Set actions default values std::vector output; std::vector options; actions->code_points_to_delete = 0; - actions->output = nullptr; - actions->persist_options = nullptr; actions->do_alert = KM_CORE_FALSE; actions->emit_keystroke = KM_CORE_FALSE; actions->new_caps_lock_state = KM_CORE_CAPS_UNCHANGED; + // Clear output pointers, will be set later once we have sizes + actions->output = nullptr; + actions->persist_options = nullptr; + for (; action_items->type != KM_CORE_IT_END; ++action_items) { assert(action_items->type < KM_CORE_IT_MAX_TYPE_ID); @@ -105,9 +107,10 @@ km_core_actions * km::core::action_item_list_to_actions_object( } } - output.push_back(KM_CORE_CONTEXT_ITEM_END); - // Strip the markers from the output to be passed to the app + // Strip the markers from the output, and convert to an string of UTF-32 + + output.push_back(KM_CORE_CONTEXT_ITEM_END); size_t buf_size; @@ -121,14 +124,15 @@ km_core_actions * km::core::action_item_list_to_actions_object( return nullptr; } + actions->output = output_usv.release(); + // Create an array of the persisted options - if(!options.empty()) { - options.push_back(KM_CORE_OPTIONS_END); - actions->persist_options = new km_core_option_item[options.size()]; - std::copy(options.begin(), options.end(), actions->persist_options); - } + options.push_back(KM_CORE_OPTIONS_END); + actions->persist_options = new km_core_option_item[options.size()]; + std::copy(options.begin(), options.end(), actions->persist_options); + + // We now have a complete set of actions - actions->output = output_usv.release(); return actions.release(); } diff --git a/core/tests/unit/kmnkbd/action_api.cpp b/core/tests/unit/kmnkbd/action_api.cpp index aee356fe4c7..79512352a3a 100644 --- a/core/tests/unit/kmnkbd/action_api.cpp +++ b/core/tests/unit/kmnkbd/action_api.cpp @@ -38,7 +38,11 @@ void test_two_backspaces() { assert(actions->code_points_to_delete == 1); assert(std::u32string(actions->output) == U""); - assert(actions->persist_options == nullptr); + assert(actions->persist_options != nullptr); + assert(actions->persist_options[0].key == nullptr); + assert(actions->persist_options[0].value == nullptr); + assert(actions->persist_options[0].scope == KM_CORE_OPT_UNKNOWN); + assert(actions->do_alert == false); assert(actions->emit_keystroke == false); assert(actions->new_caps_lock_state == -1); @@ -65,7 +69,10 @@ void test_marker_text_interleaved() { assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U"ABD"); - assert(actions->persist_options == nullptr); + assert(actions->persist_options != nullptr); + assert(actions->persist_options[0].key == nullptr); + assert(actions->persist_options[0].value == nullptr); + assert(actions->persist_options[0].scope == KM_CORE_OPT_UNKNOWN); assert(actions->do_alert == false); assert(actions->emit_keystroke == false); assert(actions->new_caps_lock_state == -1); @@ -85,7 +92,10 @@ void test_alert() { assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); - assert(actions->persist_options == nullptr); + assert(actions->persist_options != nullptr); + assert(actions->persist_options[0].key == nullptr); + assert(actions->persist_options[0].value == nullptr); + assert(actions->persist_options[0].scope == KM_CORE_OPT_UNKNOWN); assert(actions->do_alert == KM_CORE_TRUE); assert(actions->emit_keystroke == KM_CORE_FALSE); assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); @@ -105,7 +115,10 @@ void test_emit_keystroke() { assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); - assert(actions->persist_options == nullptr); + assert(actions->persist_options != nullptr); + assert(actions->persist_options[0].key == nullptr); + assert(actions->persist_options[0].value == nullptr); + assert(actions->persist_options[0].scope == KM_CORE_OPT_UNKNOWN); assert(actions->do_alert == KM_CORE_FALSE); assert(actions->emit_keystroke == KM_CORE_TRUE); assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED); @@ -126,7 +139,10 @@ void test_invalidate_context() { assert(actions->code_points_to_delete == 0); assert(std::u32string(actions->output) == U""); - assert(actions->persist_options == nullptr); + assert(actions->persist_options != nullptr); + assert(actions->persist_options[0].key == nullptr); + assert(actions->persist_options[0].value == nullptr); + assert(actions->persist_options[0].scope == KM_CORE_OPT_UNKNOWN); assert(actions->do_alert == KM_CORE_FALSE); assert(actions->emit_keystroke == KM_CORE_FALSE); assert(actions->new_caps_lock_state == KM_CORE_CAPS_UNCHANGED);