Fix explicit vmod dump

changes/api/+dump-explicit-vmod-mappings.bugfix.md

@@ -0,0 +1 @@
+Fixed missing explicit virtual modifier mappings when export the keymap as a string.

src/keymap.h

@@ -382,6 +382,7 @@ struct xkb_mod {
 struct xkb_mod_set {
     struct xkb_mod mods[XKB_MAX_MODS];
     unsigned int num_mods;
+    xkb_mod_mask_t explicit_vmods;
 };
 
 /* Common keyboard description structure */
@@ -437,6 +438,12 @@ struct xkb_keymap {
          (iter) < (mods_)->mods + (mods_)->num_mods; \
          (iter)++)
 
+#define xkb_mods_mask_foreach(mask, iter, mods_) \
+    for ((iter) = (mods_)->mods; \
+         mask && (iter) < (mods_)->mods + (mods_)->num_mods; \
+         (iter)++, mask >>= 1) \
+        if (mask & 0x1)
+
 #define xkb_mods_enumerate(idx, iter, mods_) \
     for ((idx) = 0, (iter) = (mods_)->mods; \
          (idx) < (mods_)->num_mods; \

src/text.c

@@ -263,7 +263,6 @@ ModMaskText(struct xkb_context *ctx, const struct xkb_mod_set *mods,
 {
     char buf[1024] = {0};
     size_t pos = 0;
-    xkb_mod_index_t i;
     const struct xkb_mod *mod;
 
     if (mask == 0)
@@ -272,19 +271,21 @@ ModMaskText(struct xkb_context *ctx, const struct xkb_mod_set *mods,
     if (mask == MOD_REAL_MASK_ALL)
         return "all";
 
-    xkb_mods_enumerate(i, mod, mods) {
-        int ret;
-
-        if (!(mask & (UINT32_C(1) << i)))
-            continue;
-
-        ret = snprintf(buf + pos, sizeof(buf) - pos, "%s%s",
-                       pos == 0 ? "" : "+",
-                       xkb_atom_text(ctx, mod->name));
-        if (ret <= 0 || pos + ret >= sizeof(buf))
-            break;
-        else
-            pos += ret;
+    if (unlikely(mask & ~((UINT64_C(1) << mods->num_mods) - 1))) {
+        /* If we get a mask that cannot be expressed with the known modifiers,
+         * print it as hexadecimal */
+        snprintf(buf + pos, sizeof(buf) - pos, "0x%"PRIx32, mask);
+    } else {
+        /* Print known mods */
+        xkb_mods_mask_foreach(mask, mod, mods) {
+            int ret = snprintf(buf + pos, sizeof(buf) - pos, "%s%s",
+                               pos == 0 ? "" : "+",
+                               xkb_atom_text(ctx, mod->name));
+            if (ret <= 0 || pos + ret >= sizeof(buf))
+                break;
+            else
+                pos += ret;
+        }
     }
 
     return strcpy(xkb_context_get_buffer(ctx, pos + 1), buf);

src/xkbcomp/keymap-dump.c

@@ -126,21 +126,32 @@ static bool
 write_vmods(struct xkb_keymap *keymap, struct buf *buf)
 {
     const struct xkb_mod *mod;
-    xkb_mod_index_t num_vmods = 0;
+    xkb_mod_index_t vmod = 0;
+    bool has_some = false;
 
-    xkb_mods_foreach(mod, &keymap->mods) {
+    xkb_mods_enumerate(vmod, mod, &keymap->mods) {
         if (mod->type != MOD_VIRT)
             continue;
 
-        if (num_vmods == 0)
+        if (!has_some) {
             write_buf(buf, "\tvirtual_modifiers ");
-        else
+            has_some = true;
+        } else {
             write_buf(buf, ",");
+        }
         write_buf(buf, "%s", xkb_atom_text(keymap->ctx, mod->name));
-        num_vmods++;
+        if (keymap->mods.explicit_vmods & (UINT32_C(1) << vmod)) {
+            /* Can only pretty-print real modifiers in this context */
+            if (mod->mapping & ~MOD_REAL_MASK_ALL) {
+                write_buf(buf, "=0x%"PRIx32, mod->mapping);
+            } else {
+                write_buf(buf, "=%s",
+                          ModMaskText(keymap->ctx, &keymap->mods, mod->mapping));
+            }
+        }
     }
 
-    if (num_vmods > 0)
+    if (has_some)
         write_buf(buf, ";\n\n");
 
     return true;

src/xkbcomp/vmod.c

@@ -39,8 +39,6 @@ HandleVModDef(struct xkb_context *ctx, struct xkb_mod_set *mods,
     struct xkb_mod *mod;
     xkb_mod_mask_t mapping;
 
-    merge = (merge == MERGE_DEFAULT ? stmt->merge : merge);
-
     if (stmt->value) {
         /*
          * This is a statement such as 'virtualModifiers NumLock = Mod1';
@@ -58,6 +56,8 @@ HandleVModDef(struct xkb_context *ctx, struct xkb_mod_set *mods,
         mapping = 0;
     }
 
+    merge = (merge == MERGE_DEFAULT ? stmt->merge : merge);
+
     xkb_mods_enumerate(i, mod, mods) {
         if (mod->name == stmt->name) {
             if (mod->type != MOD_VIRT) {
@@ -68,14 +68,22 @@ HandleVModDef(struct xkb_context *ctx, struct xkb_mod_set *mods,
                 return false;
             }
 
-            if (mod->mapping == mapping)
+            if (mod->mapping == mapping || !stmt->value) {
+                /*
+                 * Same definition or no new explicit mapping: do nothing.
+                 * Note that we must test the statement value and not the mapping
+                 * in order to allow resetting it: e.g. `VMod=none`.
+                 */
                 return true;
+            }
 
-            if (mod->mapping != 0) {
-                xkb_mod_mask_t use, ignore;
-
-                use = (merge == MERGE_OVERRIDE ? mapping : mod->mapping);
-                ignore = (merge == MERGE_OVERRIDE ? mod->mapping : mapping);
+            const xkb_mod_mask_t vmod = UINT32_C(1) << i;
+            if (mods->explicit_vmods & vmod) {
+                /* Handle conflicting mappings */
+                assert(mod->mapping != 0);
+                const bool clobber = (merge != MERGE_AUGMENT);
+                const xkb_mod_mask_t use = (clobber ? mapping : mod->mapping);
+                const xkb_mod_mask_t ignore = (clobber ? mod->mapping : mapping);
 
                 log_warn(ctx, XKB_LOG_MESSAGE_NO_ID,
                          "Virtual modifier %s defined multiple times; "
@@ -88,6 +96,12 @@ HandleVModDef(struct xkb_context *ctx, struct xkb_mod_set *mods,
             }
 
             mod->mapping = mapping;
+            if (mapping) {
+                mods->explicit_vmods |= vmod;
+            } else {
+                mods->explicit_vmods &= ~vmod;
+            }
+
             return true;
         }
     }
@@ -102,6 +116,8 @@ HandleVModDef(struct xkb_context *ctx, struct xkb_mod_set *mods,
     mods->mods[mods->num_mods].name = stmt->name;
     mods->mods[mods->num_mods].type = MOD_VIRT;
     mods->mods[mods->num_mods].mapping = mapping;
+    if (mapping)
+        mods->explicit_vmods |= UINT32_C(1) << mods->num_mods;
     mods->num_mods++;
     return true;
 }

test/data/keymaps/stringcomp.data

@@ -296,7 +296,7 @@ xkb_keycodes "evdev_aliases(qwerty)" {
 };
 
 xkb_types "complete" {
-	virtual_modifiers NumLock,Alt,LevelThree,LAlt,RAlt,RControl,LControl,ScrollLock,LevelFive,AltGr,Meta,Super,Hyper;
+	virtual_modifiers NumLock,Alt,LevelThree,LAlt=Mod1,RAlt=0x100,RControl=0x210001,LControl,ScrollLock,LevelFive,AltGr,Meta,Super,Hyper;
 
 	type "ONE_LEVEL" {
 		modifiers= none;
@@ -590,7 +590,7 @@ xkb_types "complete" {
 };
 
 xkb_compatibility "complete_caps(caps_lock)_4_misc(assign_shift_left_action)_4_level5(level5_lock)_4" {
-	virtual_modifiers NumLock,Alt,LevelThree,LAlt,RAlt,RControl,LControl,ScrollLock,LevelFive,AltGr,Meta,Super,Hyper;
+	virtual_modifiers NumLock,Alt,LevelThree,LAlt=Mod1,RAlt=0x100,RControl=0x210001,LControl,ScrollLock,LevelFive,AltGr,Meta,Super,Hyper;
 
 	interpret.useModMapMods= AnyLevel;
 	interpret.repeat= False;