erabo/spiegel-keyman
1
0
Fork 0
mirror of https://github.com/keymanapp/keyman.git synced 2026-09-19 06:07:40 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

fix(core): ldml fix for multiple marker deletion

Browse source 
- current code only deletes a single marker and falls through
- update the ldml test code, get rid of 'expected character' backspace logic (now that we have context object)
- update test cases
This commit is contained in:
Steven R. Loomis 2024-04-25 16:59:20 -05:00
parent dc965f4e1a
commit fa1fd75157
4 changed files with 109 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
core/src/ldml/ldml_processor.cpp
View file

@ -246,17 +246,15 @@ void ldml_event_state::emit_backspace() {
// TODO-LDML: emoji backspace
auto end = state->context().rbegin();
while (end != state->context().rend()) {
if ((*end).type == KM_CORE_CT_CHAR) {
if (end->type == KM_CORE_CT_CHAR) {
actions.code_points_to_delete++;
state->context().pop_back();
return;
} else if ((*end).type == KM_CORE_BT_MARKER) {
// remove any markers before char
state->context().pop_back();
end++;
// loop again
}
}
// else loop again
assert(end->type != KM_CORE_CT_END); // inappropriate here.
state->context().pop_back();
}
/*
We couldn't find a character at end of context (context is empty),
so we'll pass the backspace keystroke on to the app to process; the

61
core/tests/unit/ldml/keyboards/k_210_marker-test.xml
View file

@ -62,4 +62,65 @@
<check result="´"/>
</test>
</tests>
<tests name="double-marker-10955">
<!-- unmatched delete should delete ALL markers, not just one -->
<test name="double-marker-10955-null">
<keystroke key="v" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<keystroke key="b" />
<check result="v2-squiggles" />
</test>
<test name="double-marker-10955-del-1">
<keystroke key="v" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="no-squiggles" />
</test>
<test name="double-marker-10955-del-2">
<keystroke key="v" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="no-squiggles" />
</test>
<test name="double-marker-10955-del-3">
<keystroke key="v" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="no-squiggles" />
</test>
<test name="double-marker-10955-del-2-1">
<keystroke key="v" />
<keystroke key="v" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="vno-squiggles" />
</test>
<test name="double-marker-10955-del-2-2">
<keystroke key="v" />
<keystroke key="v" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="vno-squiggles" />
</test>
<test name="double-marker-10955-del-2-3">
<keystroke key="v" />
<keystroke key="v" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<keystroke key="squiggle" />
<backspace />
<keystroke key="b" />
<check result="vno-squiggles" />
</test>
</tests>
</keyboardTest3>

12
core/tests/unit/ldml/keyboards/k_210_marker.xml
View file

@ -17,14 +17,15 @@
<key id="acute" output="\m{acute}" />
<key id="caret" output="C" /> <!-- see transform -->
<key id="hacek" output="H" /> <!-- see transform -->
<key id="squiggle" output="\m{squiggle}" />
</keys>
<layers formId="us">
<layer modifiers="none" id="base">
<row keys="grave acute caret hacek" />
<row keys="grave acute caret hacek squiggle" />
<row keys="q w e" /> <!-- etc -->
<row keys="a s d" /> <!-- etc -->
<row keys="z x c v" /> <!-- etc -->
<row keys="z x c v b" /> <!-- etc -->
</layer>
</layers>
@ -56,5 +57,12 @@
<transform from="\m{grave}" to="_" /> <!-- trailing grave becomes _ -->
<!-- no cleanup for trailing acute -->
</transformGroup>
<!-- for the do uble-marker-10955delete tests -->
<transformGroup>
<transform from="\m{squiggle}\m{squiggle}\m{squiggle}b" to="3-squiggles"/>
<transform from="\m{squiggle}\m{squiggle}b" to="2-squiggles"/>
<transform from="\m{squiggle}b" to="1-squiggles"/>
<transform from="b" to="no-squiggles"/>
</transformGroup>
</transforms>
</keyboard3>

63
core/tests/unit/ldml/ldml.cpp
View file

@ -39,8 +39,6 @@ namespace {
bool g_beep_found = false;
bool g_already_complained = false;
km_core_option_item test_env_opts[] =
{
KM_CORE_OPTIONS_END
@ -115,49 +113,54 @@ apply_action(
{(uint32_t)act.marker}});
break;
case KM_CORE_IT_BACK:
{
// single char removed in context
km_core_usv ch = 0;
bool matched_text = false;
// assume the backspace came from set_action() and there's no further info.
assert(act.backspace.expected_type == KM_CORE_BT_CHAR);
assert(act.backspace.expected_value == 0);
// It is valid for a backspace to be received with an empty text store
// as the user can press backspace with no text in the store and Keyman
// will pass that back to the client, as the client may do additional
// processing at start of a text store, e.g. delete from a previous cell
// in a table. Or, if Keyman has a cached context, then there may be
// additional text in the text store that Keyman can't see.
if (act.backspace.expected_type == KM_CORE_BT_MARKER) {
assert(!context.empty());
assert(context.back().type == KM_CORE_CT_MARKER);
context.pop_back();
// no change to text store.
} else if (text_store.length() > 0) {
// If there's anything in the text store, pop it off. Two if a pair.
if (text_store.length() > 0) {
assert(!context.empty() && !text_store.empty());
km_core_usv ch = text_store.back();
const auto ch1 = text_store.back();
text_store.pop_back();
if (text_store.length() > 0 && Uni_IsSurrogate2(ch)) {
auto ch1 = text_store.back();
if (Uni_IsSurrogate1(ch1)) {
if (text_store.length() > 0 && Uni_IsSurrogate2(ch1)) {
const auto ch2 = text_store.back();
if (Uni_IsSurrogate1(ch2)) {
// We'll only pop the next character off it is actually a
// surrogate pair
ch = Uni_SurrogateToUTF32(ch1, ch);
ch = Uni_SurrogateToUTF32(ch2, ch1); // reverse order
text_store.pop_back();
}
}
if (act.backspace.expected_type == KM_CORE_BT_CHAR) {
if (act.backspace.expected_value == 0) {
// using set_action() doesn't provide for expected backspaces, so can't validate here
// only complain once.
if (!g_already_complained) {
std::cerr << "Note: TODO-LDML: not validating backspace.expected_value nor ch - no information available." << std::endl;
g_already_complained = true;
}
} else {
assert(ch == act.backspace.expected_value);
assert(context.back().character == ch);
ch = 0xFFFF; // unpaired
}
assert(context.back().type == KM_CORE_CT_CHAR);
context.pop_back();
} else {
// assume it's otherwise KM_CORE_BT_UNKNOWN
assert(act.backspace.expected_type == KM_CORE_BT_UNKNOWN);
assert(context.empty()); // if KM_CORE_BT_UNKNOWN, context should be empty.
ch = ch1; // single char
}
} else {
matched_text = true; // no text to match as context is empty.
}
// now, we need to simulate what ldml_processor::emit_backspace() is going to do.
auto end = context.rbegin();
while (end != context.rend()) {
if (end->type == KM_CORE_CT_CHAR) {
assert(!matched_text);
assert_equal(end->character, ch); // expect popped char to be same as what's in context
matched_text = true;
context.pop_back();
break; // exit on first real char
}
assert(end->type != KM_CORE_CT_END); // inappropriate here.
context.pop_back();
}
assert(matched_text);
}
break;
case KM_CORE_IT_PERSIST_OPT: