From 7118cf96106f0e9b097e216ceca380f6ea0d2257 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Wed, 15 Jan 2025 13:56:36 +0100 Subject: [PATCH] compat: Fix Interp & LED merge modes --- src/xkbcomp/compat.c | 62 ++++++++++++------------ test/data/keymaps/merge-modes/compat.xkb | 55 +++++++++++++++++++++ test/merge_modes.c | 46 ++++++++++++++++++ 3 files changed, 132 insertions(+), 31 deletions(-) create mode 100644 test/data/keymaps/merge-modes/compat.xkb diff --git a/src/xkbcomp/compat.c b/src/xkbcomp/compat.c index bf139aa20..cbea721f1 100644 --- a/src/xkbcomp/compat.c +++ b/src/xkbcomp/compat.c @@ -185,18 +185,17 @@ FindMatchingInterp(CompatInfo *info, SymInterpInfo *new) } static bool -UseNewInterpField(enum si_field field, SymInterpInfo *old, SymInterpInfo *new, - bool report, enum si_field *collide) +UseNewInterpField(enum si_field field, enum si_field old, enum si_field new, + bool clobber, bool report, enum si_field *collide) { - if (!(old->defined & field)) - return true; + if (!(old & field)) + return (new & field); - if (new->defined & field) { + if (new & field) { if (report) *collide |= field; - if (new->merge != MERGE_AUGMENT) - return true; + return clobber; } return false; @@ -207,6 +206,7 @@ AddInterp(CompatInfo *info, SymInterpInfo *new, bool same_file) { SymInterpInfo *old = FindMatchingInterp(info, new); if (old) { + const bool clobber = (new->merge != MERGE_AUGMENT); const int verbosity = xkb_context_get_log_verbosity(info->ctx); const bool report = (same_file && verbosity > 0) || verbosity > 9; enum si_field collide = 0; @@ -221,23 +221,23 @@ AddInterp(CompatInfo *info, SymInterpInfo *new, bool same_file) return true; } - if (UseNewInterpField(SI_FIELD_VIRTUAL_MOD, old, new, report, - &collide)) { + if (UseNewInterpField(SI_FIELD_VIRTUAL_MOD, old->defined, new->defined, + clobber, report, &collide)) { old->interp.virtual_mod = new->interp.virtual_mod; old->defined |= SI_FIELD_VIRTUAL_MOD; } - if (UseNewInterpField(SI_FIELD_ACTION, old, new, report, - &collide)) { + if (UseNewInterpField(SI_FIELD_ACTION, old->defined, new->defined, + clobber, report, &collide)) { old->interp.action = new->interp.action; old->defined |= SI_FIELD_ACTION; } - if (UseNewInterpField(SI_FIELD_AUTO_REPEAT, old, new, report, - &collide)) { + if (UseNewInterpField(SI_FIELD_AUTO_REPEAT, old->defined, new->defined, + clobber, report, &collide)) { old->interp.repeat = new->interp.repeat; old->defined |= SI_FIELD_AUTO_REPEAT; } - if (UseNewInterpField(SI_FIELD_LEVEL_ONE_ONLY, old, new, report, - &collide)) { + if (UseNewInterpField(SI_FIELD_LEVEL_ONE_ONLY, old->defined, new->defined, + clobber, report, &collide)) { old->interp.level_one_only = new->interp.level_one_only; old->defined |= SI_FIELD_LEVEL_ONE_ONLY; } @@ -247,7 +247,7 @@ AddInterp(CompatInfo *info, SymInterpInfo *new, bool same_file) "Multiple interpretations of \"%s\"; " "Using %s definition for duplicate fields\n", siText(new, info), - (new->merge != MERGE_AUGMENT ? "last" : "first")); + (clobber ? "last" : "first")); } return true; @@ -296,18 +296,17 @@ ResolveStateAndPredicate(ExprDef *expr, enum xkb_match_operation *pred_rtrn, /***====================================================================***/ static bool -UseNewLEDField(enum led_field field, LedInfo *old, LedInfo *new, - bool report, enum led_field *collide) +UseNewLEDField(enum led_field field, enum led_field old, enum led_field new, + bool clobber, bool report, enum led_field *collide) { - if (!(old->defined & field)) - return true; + if (!(old & field)) + return (new & field); - if (new->defined & field) { + if (new & field) { if (report) *collide |= field; - if (new->merge != MERGE_AUGMENT) - return true; + return clobber; } return false; @@ -317,6 +316,7 @@ static bool AddLedMap(CompatInfo *info, LedInfo *new, bool same_file) { enum led_field collide; + const bool clobber = (new->merge != MERGE_AUGMENT); const int verbosity = xkb_context_get_log_verbosity(info->ctx); const bool report = (same_file && verbosity > 0) || verbosity > 9; @@ -346,17 +346,20 @@ AddLedMap(CompatInfo *info, LedInfo *new, bool same_file) } collide = 0; - if (UseNewLEDField(LED_FIELD_MODS, old, new, report, &collide)) { + if (UseNewLEDField(LED_FIELD_MODS, old->defined, new->defined, + clobber, report, &collide)) { old->led.which_mods = new->led.which_mods; old->led.mods = new->led.mods; old->defined |= LED_FIELD_MODS; } - if (UseNewLEDField(LED_FIELD_GROUPS, old, new, report, &collide)) { + if (UseNewLEDField(LED_FIELD_GROUPS, old->defined, new->defined, + clobber, report, &collide)) { old->led.which_groups = new->led.which_groups; old->led.groups = new->led.groups; old->defined |= LED_FIELD_GROUPS; } - if (UseNewLEDField(LED_FIELD_CTRLS, old, new, report, &collide)) { + if (UseNewLEDField(LED_FIELD_CTRLS, old->defined, new->defined, + clobber, report, &collide)) { old->led.ctrls = new->led.ctrls; old->defined |= LED_FIELD_CTRLS; } @@ -366,7 +369,7 @@ AddLedMap(CompatInfo *info, LedInfo *new, bool same_file) "Map for indicator %s redefined; " "Using %s definition for duplicate fields\n", xkb_atom_text(info->ctx, old->led.name), - (new->merge == MERGE_AUGMENT ? "first" : "last")); + (clobber ? "last" : "first")); } return true; @@ -726,11 +729,8 @@ HandleLedMapDef(CompatInfo *info, LedMapDef *def, enum merge_mode merge) VarDef *var; bool ok; - if (def->merge != MERGE_DEFAULT) - merge = def->merge; - ledi = info->default_led; - ledi.merge = merge; + ledi.merge = (def->merge == MERGE_DEFAULT ? merge : def->merge); ledi.led.name = def->name; ok = true; diff --git a/test/data/keymaps/merge-modes/compat.xkb b/test/data/keymaps/merge-modes/compat.xkb new file mode 100644 index 000000000..9b3c542bf --- /dev/null +++ b/test/data/keymaps/merge-modes/compat.xkb @@ -0,0 +1,55 @@ +xkb_keymap { +xkb_keycodes "(unnamed)" { + minimum = 8; + maximum = 255; + indicator 1 = "A"; + indicator 2 = "B"; + indicator 3 = "C"; + indicator 4 = "D"; +}; + +xkb_types "(unnamed)" { + type "default" { + modifiers= none; + }; +}; + +xkb_compatibility "(unnamed)" { + interpret.useModMapMods= AnyLevel; + interpret.repeat= False; + interpret A+AnyOfOrNone(all) { + repeat= True; + action= SetMods(modifiers=Mod1); + }; + interpret B+AnyOfOrNone(all) { + repeat= True; + action= SetMods(modifiers=Mod1); + }; + interpret C+AnyOfOrNone(all) { + repeat= True; + action= SetMods(modifiers=Mod1); + }; + interpret D+AnyOfOrNone(all) { + action= SetMods(modifiers=Mod1); + }; + indicator "A" { + groups= 0x01; + modifiers= Lock; + }; + indicator "B" { + groups= 0x01; + modifiers= Lock; + }; + indicator "C" { + groups= 0x01; + modifiers= Lock; + }; + indicator "D" { + groups= 0x01; + }; +}; + +xkb_symbols "(unnamed)" { +}; + +}; diff --git a/test/merge_modes.c b/test/merge_modes.c index ca6eb8507..f09eb9548 100644 --- a/test/merge_modes.c +++ b/test/merge_modes.c @@ -38,6 +38,51 @@ compile_buffer(struct xkb_context *context, const char *buf, size_t len, return test_compile_buffer(context, buf, len); } +static void +test_compat(struct xkb_context *ctx, enum update_files update_output_files) +{ + // Github Issue #566 + const char keymap_str[] = + "xkb_keymap {\n" + " xkb_keycodes { };\n" + " xkb_types { };\n" + " xkb_compat {\n" + " interpret A { repeat = true; };\n" + " interpret A { repeat = true; };\n" + " interpret A { action = SetMods(mods=Mod1); };\n" + " interpret B { repeat = true; };\n" + " interpret B { repeat = true; };\n" + " augment interpret B { action = SetMods(mods=Mod1); };\n" + " interpret C { repeat = true; };\n" + " interpret C { repeat = true; };\n" + " override interpret C { action = SetMods(mods=Mod1); };\n" + " interpret D { repeat = true; };\n" + " interpret D { repeat = true; };\n" + " replace interpret D { action = SetMods(mods=Mod1); };\n" + "\n" + " indicator \"A\" { modifiers=Shift; };\n" + " indicator \"A\" { modifiers=Lock; };\n" + " indicator \"A\" { groups= Group1; };\n" + " indicator \"B\" { modifiers=Shift; };\n" + " indicator \"B\" { modifiers=Lock; };\n" + " augment indicator \"B\" { groups=Group1; };\n" + " indicator \"C\" { modifiers=Shift; };\n" + " indicator \"C\" { modifiers=Lock; };\n" + " override indicator \"C\" { groups=Group1; };\n" + " indicator \"D\" { modifiers=Shift; };\n" + " indicator \"D\" { modifiers=Lock; };\n" + " replace indicator \"D\" { groups=Group1; };\n" + " };\n" + " xkb_symbols { };\n" + "};\n"; + + assert(test_compile_output(ctx, compile_buffer, NULL, + "test_merge_mode: compat", + keymap_str, ARRAY_SIZE(keymap_str), + GOLDEN_TESTS_OUTPUTS "compat.xkb", + !!update_output_files)); +} + static void test_symbols(struct xkb_context *ctx, enum update_files update_output_files) { @@ -68,6 +113,7 @@ main(int argc, char *argv[]) struct xkb_context *ctx = test_get_context(CONTEXT_NO_FLAG); assert(ctx); + test_compat(ctx, update_output_files); test_symbols(ctx, update_output_files); xkb_context_unref(ctx);