Add support for modmap None (#291)

Unlike current xkbcommon, X11’s xkbcomp allows to remove entries in
the modifiers’ map using “modifier_map None { … }”.

“None” is translated to the special value “XkbNoModifier” defined in
“X11/extensions/XKB.h”. Then it relies on the fact that in "CopyModMapDef",
the following code:

    1U << entry->modifier

ends up being zero when “entry->modifier” is “XkbNoModifier” (i.e. 0xFF).
Indeed, it relies on the overflow behaviour of the left shift, which in
practice resolves to use only the 5 low bits of the shift amount, i.e.
0x1F here. Then the result of “1U << 0xFF” is cast to “char”, i.e. 0.

This is a good trick but too magical, so in libxkbcommon we will use
an explicit test against our new constant XKB_MOD_NONE.

8 files changed, 344 insertions, 9 deletions

diff --git a/meson.build b/meson.build
index 61c2f82..d55e8f3 100644
--- a/meson.build
+++ b/meson.build
@@ -675,6 +675,11 @@ test(
     env: test_env,
     suite: ['python-tests'],
 )
+test(
+    'modifiers',
+    executable('test-modifiers', 'test/modifiers.c', dependencies: test_dep),
+    env: test_env,
+)
 if get_option('enable-x11')
     test(
         'x11',
diff --git a/src/keymap.h b/src/keymap.h
index 7c5341d..f7ea5bd 100644
--- a/src/keymap.h
+++ b/src/keymap.h
@@ -109,6 +109,9 @@
 /* Don't allow more leds than we can hold in xkb_led_mask_t. */
 #define XKB_MAX_LEDS ((xkb_led_index_t) (sizeof(xkb_led_mask_t) * 8))
 
+/* Special value to handle modMap None {…} */
+#define XKB_MOD_NONE 0xffffffffU
+
 /* These should all go away. */
 enum mod_type {
     MOD_REAL = (1 << 0),
diff --git a/src/text.c b/src/text.c
index 60edb03..290a5ae 100644
--- a/src/text.c
+++ b/src/text.c
@@ -216,6 +216,9 @@ ModIndexText(struct xkb_context *ctx, const struct xkb_mod_set *mods,
     if (ndx == XKB_MOD_INVALID)
         return "none";
 
+    if (ndx == XKB_MOD_NONE)
+        return "None";
+
     if (ndx >= mods->num_mods)
         return NULL;
 
diff --git a/src/xkbcomp/ast.h b/src/xkbcomp/ast.h
index 6c51ce4..8b0901f 100644
--- a/src/xkbcomp/ast.h
+++ b/src/xkbcomp/ast.h
@@ -303,6 +303,7 @@ typedef struct {
 typedef struct {
     ParseCommon common;
     enum merge_mode merge;
+    // NOTE: Can also be “None”, rather than a modifier name.
     xkb_atom_t modifier;
     ExprDef *keys;
 } ModMapDef;
diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index eb78412..f990529 100644
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -162,6 +162,8 @@ ClearKeyInfo(KeyInfo *keyi)
 typedef struct {
     enum merge_mode merge;
     bool haveSymbol;
+    // NOTE: Can also be XKB_MOD_NONE, meaning
+    //       “don’t add a modifier to the modmap”.
     xkb_mod_index_t modifier;
     union {
         xkb_atom_t keyName;
@@ -1152,14 +1154,21 @@ HandleModMapDef(SymbolsInfo *info, ModMapDef *def)
     xkb_mod_index_t ndx;
     bool ok;
     struct xkb_context *ctx = info->ctx;
-
-    ndx = XkbModNameToIndex(&info->mods, def->modifier, MOD_REAL);
-    if (ndx == XKB_MOD_INVALID) {
-        log_err(info->ctx,
-                "Illegal modifier map definition; "
-                "Ignoring map for non-modifier \"%s\"\n",
-                xkb_atom_text(ctx, def->modifier));
-        return false;
+    const char *modifier_name = xkb_atom_text(ctx, def->modifier);
+
+    if (istreq(modifier_name, "none")) {
+        // Handle special "None" entry
+        ndx = XKB_MOD_NONE;
+    } else {
+        // Handle normal entry
+        ndx = XkbModNameToIndex(&info->mods, def->modifier, MOD_REAL);
+        if (ndx == XKB_MOD_INVALID) {
+            log_err(info->ctx,
+                    "Illegal modifier map definition; "
+                    "Ignoring map for non-modifier \"%s\"\n",
+                    xkb_atom_text(ctx, def->modifier));
+            return false;
+        }
     }
 
     ok = true;
@@ -1523,7 +1532,12 @@ CopyModMapDefToKeymap(struct xkb_keymap *keymap, SymbolsInfo *info,
         }
     }
 
-    key->modmap |= (1u << entry->modifier);
+    // Handle modMap None
+    if (entry->modifier != XKB_MOD_NONE) {
+        // Convert modifier index to modifier mask
+        key->modmap |= (1u << entry->modifier);
+    }
+
     return true;
 }
 
diff --git a/test/data/keymaps/modmap-none.xkb b/test/data/keymaps/modmap-none.xkb
new file mode 100644
index 0000000..a99b200
--- /dev/null
+++ b/test/data/keymaps/modmap-none.xkb
@@ -0,0 +1,148 @@
+xkb_keymap {
+xkb_keycodes "test" {
+    minimum = 8;
+    maximum = 255;
+	<LVL3>  = 92;
+    <LFSH>  = 50;
+	<RTSH>  = 62;
+	<LALT>  = 64;
+	<RALT>  = 108;
+	<LWIN>  = 133;
+	<RWIN>  = 134;
+	<LCTL>  = 37;
+	<RCTL>  = 105;
+	<CAPS>  = 66;
+
+	<AD01> = 24;
+	<AD02> = 25;
+	<AD03> = 26;
+	<AD04> = 27;
+	<AD05> = 28;
+	<AD06> = 29;
+	<AD07> = 30;
+	<AD08> = 31;
+	<AD09> = 32;
+};
+
+xkb_types "complete" {
+    type "ONE_LEVEL" {
+        modifiers= none;
+        level_name[Level1]= "Any";
+    };
+	type "TWO_LEVEL" {
+		modifiers= Shift;
+		map[Shift]= 2;
+		level_name[1]= "Base";
+		level_name[2]= "Shift";
+	};
+};
+xkb_compatibility "complete" {
+    interpret.useModMapMods= AnyLevel;
+    interpret.repeat= False;
+    interpret.locking= False;
+
+    interpret Any+AnyOf(all) {
+        action= SetMods(modifiers=modMapMods,clearLocks);
+    };
+};
+xkb_symbols {
+    name[group1]="Test";
+
+    // Reset modmap with a key with empty modmap
+    key <LVL3> { [VoidSymbol] };
+    modmap None { <LVL3> };
+
+    // Reset modmap with one key
+    key <LFSH> { [Shift_L] };
+    modmap Shift { <LFSH> };
+    modmap none  { <LFSH> };
+
+    // Reset modmap with one symbol
+    key <RTSH> { [Shift_R] };
+    modmap Shift { Shift_R };
+    modmap NONE  { Shift_R };
+
+    // Cycle
+    key <LWIN> { [Super_L] };
+    modmap Mod4 { <LWIN> };
+    modmap None { <LWIN> };
+    modmap Mod4 { <LWIN> };
+
+    // Cycle
+    key <RWIN> { [Super_R] };
+    modmap Mod4 { <RWIN> };
+    modmap None { <RWIN> };
+    modmap Mod4 { Super_R };
+
+    // Mix keycode and keysym
+    key <LCTL> { [Control_L] };
+    modmap Control { Control_L };
+    modmap None    { <LCTL> };
+
+    // Mix keycode and keysym
+    key <RCTL> { [Control_R] };
+    modmap Control { <RCTL> };
+    modmap None    { Control_R };
+
+    // Re-set with different modifier
+    key <LALT> { [Alt_L] };
+    modmap Mod2 { <LALT> };
+    modmap None { <LALT> };
+    modmap Mod1 { <LALT> };
+
+    // Re-set with different modifier with both key and keysym
+    key <RALT> { [ISO_Level3_Shift] };
+    modmap Mod1 { <RALT> };           // Mod1
+    modmap None { <RALT> };           // None
+    modmap Mod2 { <RALT> };           // Mod2
+    modmap Mod3 { ISO_Level3_Shift }; // Mod2 | Mod3
+    modmap None { ISO_Level3_Shift }; // Mod2
+    modmap Mod5 { ISO_Level3_Shift }; // Mod2 | Mod5
+
+    // Re-set with different modifier with both key and keysym
+    key <CAPS> { [Caps_Lock] };
+    modmap Mod1 { Caps_Lock };  // Mod1
+    modmap None { <CAPS> };     // Mod1
+    modmap None { Caps_Lock };  // None
+    modmap Lock { <CAPS> };     // Lock
+
+    // Merge modes
+    key <AD01> { [q] };
+    modMap Mod1 { <AD01> };
+    augment modMap None { <AD01> };
+
+    key <AD02> { [w] };
+    modMap Mod2 { <AD02> };
+    override modMap None { <AD02> };
+
+    key <AD03> { [e] };
+    modMap Mod3 { <AD03> };
+    replace modMap None { <AD03> };
+
+    key <AD04> { [r] };
+    modMap Mod1 { <AD04> };
+    augment modMap None { r };
+
+    key <AD05> { [t] };
+    modMap Mod2 { <AD05> };
+    replace modMap None { t };
+
+    key <AD06> { [y] };
+    modMap Mod3 { <AD06> };
+    override modMap None { y };
+
+    key <AD07> { [u] };
+    modMap Mod1 { <AD07> };
+    replace key <AD07> { [U] }; // Won’t affect the modMap
+
+    key <AD08> { [i] };
+    modMap Mod2 { i, I };
+    replace key <AD08> { [I] };
+    modMap None { i }; // Does not resolve
+
+    key <AD09> { [1] };
+    modMap Mod3 { 1, 2 };
+    override key <AD09> { [NoSymbol, 2] };
+    modMap None { 1 };
+};
+};
diff --git a/test/filecomp.c b/test/filecomp.c
index 827a19c..5fc7477 100644
--- a/test/filecomp.c
+++ b/test/filecomp.c
@@ -47,6 +47,7 @@ main(void)
     assert(test_file(ctx, "keymaps/no-types.xkb"));
     assert(test_file(ctx, "keymaps/quartz.xkb"));
     assert(test_file(ctx, "keymaps/no-aliases.xkb"));
+    assert(test_file(ctx, "keymaps/modmap-none.xkb"));
 
     assert(!test_file(ctx, "keymaps/divide-by-zero.xkb"));
     assert(!test_file(ctx, "keymaps/bad.xkb"));
diff --git a/test/modifiers.c b/test/modifiers.c
new file mode 100644
index 0000000..807c015
--- /dev/null
+++ b/test/modifiers.c
@@ -0,0 +1,160 @@
+/*
+ * Copyright © 2023 Pierre Le Marre <dev@wismill.eu>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "config.h"
+
+#include <assert.h>
+#include <stdlib.h>
+
+#include "test.h"
+#include "keymap.h"
+
+// Standard real modifier masks
+#define ShiftMask   (1 << 0)
+#define LockMask    (1 << 1)
+#define ControlMask (1 << 2)
+#define Mod1Mask    (1 << 3)
+#define Mod2Mask    (1 << 4)
+#define Mod3Mask    (1 << 5)
+#define Mod4Mask    (1 << 6)
+#define Mod5Mask    (1 << 7)
+#define NoModifier  0
+
+static void
+test_modmap_none(void)
+{
+    struct xkb_context *context = test_get_context(0);
+    struct xkb_keymap *keymap;
+    const struct xkb_key *key;
+    xkb_keycode_t keycode;
+
+    keymap = test_compile_file(context, "keymaps/modmap-none.xkb");
+    assert(keymap);
+
+    keycode = xkb_keymap_key_by_name(keymap, "LVL3");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == NoModifier);
+
+    keycode = xkb_keymap_key_by_name(keymap, "LFSH");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == NoModifier);
+
+    keycode = xkb_keymap_key_by_name(keymap, "RTSH");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == NoModifier);
+
+    keycode = xkb_keymap_key_by_name(keymap, "LWIN");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod4Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "RWIN");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod4Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "LCTL");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == ControlMask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "RCTL");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == ControlMask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "LALT");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod1Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "RALT");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == (Mod2Mask | Mod5Mask));
+
+    keycode = xkb_keymap_key_by_name(keymap, "CAPS");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == LockMask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD01");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod1Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD02");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == NoModifier);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD03");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == NoModifier);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD04");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod1Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD05");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod2Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD06");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod3Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD07");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod1Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD08");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod2Mask);
+
+    keycode = xkb_keymap_key_by_name(keymap, "AD09");
+    assert(keycode != XKB_KEYCODE_INVALID);
+    key = XkbKey(keymap, keycode);
+    assert(key->modmap == Mod3Mask);
+
+    xkb_keymap_unref(keymap);
+    xkb_context_unref(context);
+}
+
+int
+main(void)
+{
+    test_modmap_none();
+
+    return 0;
+}