Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): infrastructure for normalization of output 🌱 #10403

Merged
merged 14 commits into from
Jan 19, 2024

Conversation

mcdurdin
Copy link
Member

@mcdurdin mcdurdin commented Jan 16, 2024

Relates to #9999.
Fixes #10384.

The context API endpoints should no longer be considered as part of the
standard Core API. The only consumers that have a need to access these
APIs are the IMX integration in Engine for Windows, and the Keyman
Developer Debugger.

These symbols are currently used by Developer:
* `km_core_context` struct
* `km_core_context_type` enum
* `km_core_context_item` struct
* `KM_CORE_CONTEXT_ITEM_END` macro
* `km_core_state_context()`
* `km_core_context_set()`
* `km_core_context_clear()`

These symbols are currently used by Windows IMX:
* `km_core_context` struct
* `km_core_context_type` enum
* `km_core_context_item` struct
* `KM_CORE_CONTEXT_ITEM_END` macro
* `km_core_context_items_dispose()`
* `km_core_context_item_list_size()`
* `km_core_state_get_intermediate_context()`

The following functions and symbols are moving to
keyman_core_api_context.h:
* `km_core_context` struct
* `km_core_context_type` enum
* `km_core_context_item` struct
* `KM_CORE_CONTEXT_ITEM_END` macro
* `km_core_state_context()` function
* `km_core_state_get_intermediate_context()` function
* `km_core_context_set()` function
* `km_core_context_clear()` function
* `km_core_context_get()` function
* `km_core_context_items_from_utf16()` function
* `km_core_context_items_from_utf8()` function
* `km_core_context_items_to_utf8()` function
* `km_core_context_items_to_utf16()` function
* `km_core_context_items_to_utf32()` function
* `km_core_context_items_dispose()` function
* `km_core_context_length()` function
* `km_core_context_append()` function
* `km_core_context_shrink()` function
* `km_core_context_item_list_size()` function
Relates to #9999.

Establishes functions, unit test sources, normalization entry point and
an effectively no-op unit test for normalization support.
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Jan 16, 2024
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Jan 16, 2024

User Test Results

Test specification and instructions

User tests are not required

@keymanapp-test-bot keymanapp-test-bot bot changed the title feat(core): infrastructure for normalization of output feat(core): infrastructure for normalization of output 🌱 Jan 16, 2024
@keymanapp-test-bot keymanapp-test-bot bot added this to the A17S30 milestone Jan 16, 2024
@github-actions github-actions bot added core/ Keyman Core feat labels Jan 16, 2024
Fixes #9999.

Note TODO items:
- [ ] Renormalize cached_context across action boundary. Blocked by #10369.
- [ ] Add extra tests for surrogate pairs
- [ ] Move set_context_from_string into helper module
- [ ] if we don't apply normalization, we still need to fixup the
      app_context, to keep it coherent with cached_context (or at least
      we need to verify that app_context is never used in this
      situation)
@mcdurdin mcdurdin force-pushed the chore/mac/9999-remove-legacy-action-items-references branch from ba1f592 to cf2d6c4 Compare January 17, 2024 03:44
@mcdurdin mcdurdin marked this pull request as ready for review January 17, 2024 06:21
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Jan 17, 2024
@mcdurdin
Copy link
Member Author

Returning to draft to rework part of this per #10422 (comment)

Per discussion in #10422, we can assume that input cached_context is
always NFD. However input actions->output may not start at a
normalization boundary, so we still need to backtrack to a normalization
boundary in order to get our NFC output. But cached_context never need
change.

This makes no change to the algorithm, but tweaks some of the unit tests
to adhere to this input assumption.

Note: we could consider adding a debug assertion that cached_context is
NFD.
core/src/actions_normalize.cpp Outdated Show resolved Hide resolved
core/tests/unit/kmnkbd/test_actions_normalize.cpp Outdated Show resolved Hide resolved
core/src/km_core_action_api.cpp Outdated Show resolved Hide resolved
@mcdurdin mcdurdin marked this pull request as ready for review January 18, 2024 01:57
Base automatically changed from chore/mac/9999-remove-legacy-action-items-references to epic/core/9999-normalization January 18, 2024 03:46
core/src/km_core_action_api.cpp Outdated Show resolved Hide resolved
core/src/actions_normalize.cpp Outdated Show resolved Hide resolved
@mcdurdin mcdurdin merged commit 4f909dc into epic/core/9999-normalization Jan 19, 2024
3 checks passed
@mcdurdin mcdurdin deleted the feat/core/9999-normalize-output branch January 19, 2024 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants