Skip to content

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

Merged
mcdurdin merged 14 commits intoepic/core/9999-normalizationfrom
feat/core/9999-normalize-output
Jan 19, 2024
Merged

feat(core): infrastructure for normalization of output 🌱#10403
mcdurdin merged 14 commits intoepic/core/9999-normalizationfrom
feat/core/9999-normalize-output

Conversation

@mcdurdin
Copy link
Copy Markdown
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
Copy Markdown

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
@mcdurdin mcdurdin requested a review from rc-swag as a code owner 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
Copy Markdown
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.
@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
@mcdurdin mcdurdin merged commit 4f909dc into epic/core/9999-normalization Jan 19, 2024
@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

Labels

Projects

None yet

3 participants