Skip to content

Commit

Permalink
feat(core): ldml actions: updates per code review 🌱
Browse files Browse the repository at this point in the history
- fix backspace processing
- fix k_102 - we can no longer test for context invalidation here
- fix a test that assumed non-const km_core_actions.output

For: #10410
  • Loading branch information
srl295 committed Jan 19, 2024
1 parent 6ea8339 commit 04a6e16
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 20 deletions.
17 changes: 4 additions & 13 deletions core/src/ldml/ldml_processor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,7 @@ void ldml_event_state::emit_backspace() {
} else if ((*end).type == KM_CORE_BT_MARKER) {
// nothing in actions
state->context().pop_back();
// TODO-LDML: fall through here?
// no action?
// falls through - end wasn't a character
}
}
/*
Expand All @@ -260,8 +259,7 @@ void ldml_event_state::emit_backspace() {
dumped in somewhere unknown, so we will have to depend on the app to
be sensible about backspacing because we know nothing.
*/
// TODO-LDML: What here?
// state->actions().push_backspace(KM_CORE_BT_UNKNOWN);
actions.emit_keystroke = KM_CORE_TRUE;
}

void
Expand All @@ -272,7 +270,7 @@ ldml_processor::process_key_down(ldml_event_state &ldml_state) const {

if (!found) {
// no key was found, so pass the keystroke on to the Engine
ldml_state.emit_invalidate_passthrough_keystroke();
ldml_state.emit_passthrough_keystroke();
} else if (!key_str.empty()) {
process_key_string(ldml_state, key_str);
} // else no action: It's a gap or gap-like key.
Expand Down Expand Up @@ -408,7 +406,6 @@ ldml_event_state::remove_text(std::u32string &str, size_t length) {
actions.code_points_to_delete++;
} else if (type == KM_CORE_BT_MARKER) {
assert(length >= 3);
state->actions().push_backspace(KM_CORE_BT_MARKER, c->marker);
// #3 - the marker.
assert(lastCtx == c->marker);
str.pop_back();
Expand Down Expand Up @@ -496,13 +493,7 @@ ldml_event_state::emit_marker( KMX_DWORD marker_no) {
state->context().push_marker(marker_no);
}

void ldml_event_state::emit_invalidate_passthrough_keystroke() {
if ((vk < 0x100) && km::core::kmx::vkey_to_contextreset[vk]) {
// TODO-LDML: how to invalidate context?
context_clear();
} else {
assert(vk < 0x100); // don't expect synthetic vkeys here
}
void ldml_event_state::emit_passthrough_keystroke() {
// assert we haven't already requested a keystroke
assert(actions.emit_keystroke != KM_CORE_TRUE);
actions.emit_keystroke = KM_CORE_TRUE;
Expand Down
4 changes: 2 additions & 2 deletions core/src/ldml/ldml_processor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,8 @@ class ldml_event_state {
void emit_text(km_core_usv ch);
/** emit a marker */
void emit_marker(KMX_DWORD marker);
/** emit a pass-through and invalidate */
void emit_invalidate_passthrough_keystroke();
/** emit a pass-through */
void emit_passthrough_keystroke();
/** emit a backspace (for a user-initiated deletion) */
void emit_backspace();

Expand Down
8 changes: 5 additions & 3 deletions core/tests/unit/kmnkbd/test_actions_normalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,11 @@ void setup(const km_core_cp *app_context, const km_core_cp *cached_context, int

test_actions = new km_core_actions;
test_actions->code_points_to_delete = actions_code_points_to_delete;
test_actions->output = new km_core_usv[actions_output.length() + 1];
actions_output.copy(test_actions->output, actions_output.length());
test_actions->output[actions_output.length()] = 0;
std::unique_ptr<km_core_usv[]> buf(new km_core_usv[actions_output.length() + 1]);
actions_output.copy(buf.get(), actions_output.length());
// terimate the buffer
buf.get()[actions_output.length()] = 0;
test_actions->output = buf.release();
}

//-------------------------------------------------------------------------------------
Expand Down
4 changes: 2 additions & 2 deletions core/tests/unit/ldml/keyboards/k_102_keytest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<!--
@@keys: [K_BKQUOTE][K_1][K_ENTER][K_Z][K_Q][K_BKQUOTE][K_Z][K_Q][K_2][K_BKQUOTE][K_Z][K_Q][K_1][K_BKQUOTE][K_Z][K_Q][K_ENTER][K_BKQUOTE][K_Z]
@@expected: \u0061\u007a\u0041\u007a\u0041\u007a\u0041\u007a\u0071\u0061\u007a
@@expected: \u0061\u007a\u0041\u007a\u0041\u007a\u0041\u007a\u0041\u007a
-->

<!--
Expand All @@ -13,7 +13,7 @@
qaz Az q\m{q}a => A due to transform
q2az Az 2=not mapped, but NO reset.
q1az Az 1 is a gap (no effect) so no ctx reset
q<enter>az qaz enter=not mappable, causes ctx reset.
q<enter>az Az enter=not mappable, should cause context reset of qaz. TODO-LDML no invalidate here?
-->

<!DOCTYPE keyboard3 SYSTEM "../../../../../resources/standards-data/ldml-keyboards/techpreview/dtd/ldmlKeyboard3.dtd">
Expand Down

0 comments on commit 04a6e16

Please sign in to comment.