From d5448e69028159c2867c04a991a032e6e78d8e1c Mon Sep 17 00:00:00 2001
From: Kovid Goyal <kovid@kovidgoyal.net>
Date: Sat, 3 Apr 2021 06:25:47 +0530
Subject: [PATCH] Add documentation and cleanup previous PR

---
 docs/keyboard-protocol.rst | 17 ++++++++-------
 glfw/xkb_glfw.c            | 43 ++++++++++++++++++++------------------
 2 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/docs/keyboard-protocol.rst b/docs/keyboard-protocol.rst
index 119189c48..d0a3ad970 100644
--- a/docs/keyboard-protocol.rst
+++ b/docs/keyboard-protocol.rst
@@ -140,14 +140,17 @@ sub-field for the shifted key, like this::
 Modifiers
 ~~~~~~~~~~~~~~
 
-This protocol supports four modifier keys, :kbd:`shift, alt, ctrl and super`.
-Here super is either the *Windows/Linux* key or the *Cmd* key on mac keyboards.
-Modifiers are encoded as a bit field with::
+This protocol supports six modifier keys, :kbd:`shift, alt, ctrl, super, hyper
+and meta`. Here :kbd:`super` is either the *Windows/Linux* key or the *Cmd* key on
+mac keyboards. :kbd:`hyper` and :kbd:`meta` are typically present only on X11
+based systems with special XKB rules. Modifiers are encoded as a bit field with::
 
-    shift 0b1     (1)
-    alt   0b10    (2)
-    ctrl  0b100   (4)
-    super 0b1000  (8)
+    shift 0b1      (1)
+    alt   0b10     (2)
+    ctrl  0b100    (4)
+    super 0b1000   (8)
+    hyper 0b10000  (16)
+    meta  0b100000 (32)
 
 In the escape code, the modifier value is encoded as a decimal number which is
 ``1 + actual modifiers``. So to represent :kbd:`shift` only, the value would be ``1 +
diff --git a/glfw/xkb_glfw.c b/glfw/xkb_glfw.c
index f774e724a..10bd554f6 100644
--- a/glfw/xkb_glfw.c
+++ b/glfw/xkb_glfw.c
@@ -317,6 +317,7 @@ glfw_xkb_update_x11_keyboard_id(_GLFWXKBData *xkb) {
 
 static void
 glfw_xkb_update_masks(_GLFWXKBData *xkb) {
+    // See https://github.com/kovidgoyal/kitty/pull/3430 for discussion
     bool succeeded = false;
     unsigned used_bits = 0; /* To avoid using the same bit twice */
     XkbDescPtr xkb_ptr = XkbGetMap( _glfw.x11.display, XkbVirtualModsMask | XkbVirtualModMapMask, XkbUseCoreKbd );
@@ -324,34 +325,34 @@ glfw_xkb_update_masks(_GLFWXKBData *xkb) {
 #define S( a ) xkb->a##Idx = XKB_MOD_INVALID; xkb->a##Mask = 0
     S(control); S(alt); S(shift); S(super); S(hyper); S(meta); S(capsLock); S(numLock);
 #undef S
-    if ( xkb_ptr ) {
-        Status status = XkbGetNames( _glfw.x11.display, XkbVirtualModNamesMask, xkb_ptr );
-        if ( status == Success ) {
-            for ( int indx = 0; indx < XkbNumVirtualMods; ++indx ) {
+    if (xkb_ptr) {
+        Status status = XkbGetNames(_glfw.x11.display, XkbVirtualModNamesMask, xkb_ptr);
+        if (status == Success) {
+            for (int indx = 0; indx < XkbNumVirtualMods; ++indx) {
                 Atom atom = xkb_ptr->names->vmods[indx];
-                if ( atom ) {
+                if (atom) {
                     unsigned mask_rtn = 0;
-                    if ( XkbVirtualModsToReal( xkb_ptr, 1<<indx, &mask_rtn ) ) {
-                        const char *name = XGetAtomName( _glfw.x11.display, atom );
-#define S( a, s ) if ( !(used_bits & mask_rtn) && strcmp( name, #s ) == 0 ) xkb->a##Mask = mask_rtn, used_bits |= mask_rtn
+                    if (XkbVirtualModsToReal( xkb_ptr, 1<<indx, &mask_rtn) ) {
+                        const char *name = XGetAtomName(_glfw.x11.display, atom);
+#define S( a, s ) if (!(used_bits & mask_rtn) && strcmp(name, #s) == 0) xkb->a##Mask = mask_rtn, used_bits |= mask_rtn
                         /* Note that the order matters here; earlier is higher priority. */
-                        S( alt, Alt );
-                        S( super, Super );
-                        S( numLock, NumLock );
-                        S( meta, Meta );
-                        S( hyper, Hyper );
+                        S(alt, Alt);
+                        S(super, Super);
+                        S(numLock, NumLock);
+                        S(meta, Meta);
+                        S(hyper, Hyper);
 #undef S
                     }
                 }
             }
             succeeded = true;
         }
-        XkbFreeNames( xkb_ptr, XkbVirtualModNamesMask, True );
-        XkbFreeKeyboard( xkb_ptr, 0, True );
+        XkbFreeNames(xkb_ptr, XkbVirtualModNamesMask, True);
+        XkbFreeKeyboard(xkb_ptr, 0, True);
     }
-    if ( succeeded ) {
+    if (succeeded) {
         unsigned indx, shifted;
-        for ( indx = 0, shifted = 1; used_bits; ++indx, shifted <<= 1, used_bits >>= 1 ) {
+        for (indx = 0, shifted = 1; used_bits; ++indx, shifted <<= 1, used_bits >>= 1) {
 #define S( a ) if ( xkb->a##Mask == shifted ) xkb->a##Idx = indx
             S(alt); S(super); S(hyper); S(meta); S(numLock);
 #undef S
@@ -361,13 +362,14 @@ glfw_xkb_update_masks(_GLFWXKBData *xkb) {
     S(control, XKB_MOD_NAME_CTRL);
     S(shift, XKB_MOD_NAME_SHIFT);
     S(capsLock, XKB_MOD_NAME_CAPS);
-    if ( !succeeded ) {
+    if (!succeeded) {
         S(numLock, XKB_MOD_NAME_NUM);
         S(alt, XKB_MOD_NAME_ALT);
         S(super, XKB_MOD_NAME_LOGO);
     }
 #undef S
-    debug( "Modifier indices alt:%d super:%d hyper:%d meta:%d num>ock:%d\n", xkb->altIdx, xkb->superIdx, xkb->hyperIdx, xkb->metaIdx, xkb->numLockIdx );
+    debug("Modifier indices alt:%d super:%d hyper:%d meta:%d numlock:%d\n",
+            xkb->altIdx, xkb->superIdx, xkb->hyperIdx, xkb->metaIdx, xkb->numLockIdx);
 }
 
 
@@ -378,7 +380,8 @@ glfw_xkb_update_masks(_GLFWXKBData *xkb) {
 
 static void
 glfw_xkb_update_masks(_GLFWXKBData *xkb) {
-    /* Should find better solution under evdev or wayland */
+    // Should find better solution under Wayland
+    // See https://github.com/kovidgoyal/kitty/pull/3430 for discussion
 
 #define S( a ) xkb->a##Idx = XKB_MOD_INVALID; xkb->a##Mask = 0
     S(hyper); S(meta);