From 076a1868e594fdcd1002623cbb68747c1ac4e62f Mon Sep 17 00:00:00 2001
From: Raheman Vaiya <r.vaiya@gmail.com>
Date: Thu, 17 Feb 2022 02:05:56 -0500
Subject: [PATCH] Prevent spurious modifier key presses (#128)

Some applications make use of the alt (e.g firefox) and meta (e.g gnome)
keys in isolation. Thus it is necessary to interpose additional events
to prevent the likes of '<alt down> <alt up>' from being interpreted as
key presses.

E.G

	[alt]

	a = x

will currently cause 'alt+a' to produce:

	<alt down>
	<alt up>

	<x down>
	<x up>

In most cases this is identical to '<x>', however in some contexts the
additional alt keypress is meaningful.

To prevent this, we intelligently emit '<alt>+<control>' instead by sandwiching the
'<alt up>' event like so: '<control down> <alt up> <control up>'

The full sequence thus becomes:

	<alt down>
	<control down>
	<alt up>
	<control up>

	<x down>
	<x up>
---
 src/keyboard.c | 125 ++++++++++++++++++++++++++++++++++---------------
 src/keyboard.h |   1 +
 t/disarm.t     |   9 ++++
 t/disarm2.t    |   5 ++
 t/disarm3.t    |  15 ++++++
 t/meta.t       |   8 ++++
 t/swap.t       |   2 +
 t/swap2.t      |   2 +
 t/swap4.t      |   4 ++
 t/swap5.t      |   4 ++
 t/swap7.t      |   2 +
 t/test.conf    |   1 +
 12 files changed, 140 insertions(+), 38 deletions(-)
 create mode 100644 t/disarm.t
 create mode 100644 t/disarm2.t
 create mode 100644 t/disarm3.t

diff --git a/src/keyboard.c b/src/keyboard.c
index 302388c..350e2b0 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -32,6 +32,13 @@
 #define MACRO_REPEAT_TIMEOUT 400 /* In ms */
 #define MACRO_REPEAT_INTERVAL 20 /* In ms */
 
+static long get_time_ms()
+{
+	struct timespec tv;
+	clock_gettime(CLOCK_MONOTONIC, &tv);
+	return (tv.tv_sec*1E3)+tv.tv_nsec/1E6;
+}
+
 static void send_mods(uint16_t mods, int press)
 {
 	size_t i;
@@ -45,6 +52,30 @@ static void send_mods(uint16_t mods, int press)
 	}
 }
 
+/* Intelligently disarm active mods to avoid spurious key press events. */
+static void disarm_mods(uint16_t mods)
+{
+	uint16_t dmods = (MOD_ALT|MOD_SUPER) & mods;
+	/*
+	 * We interpose a control sequence to prevent '<alt down> <alt up>'
+	 * from being interpreted as an alt keypress.
+	 */
+	if (dmods) {
+		if (keystate[KEY_LEFTCTRL])
+			send_mods(dmods, 0);
+		else {
+			send_key(KEY_LEFTCTRL, 1);
+			send_mods(dmods, 0);
+			send_key(KEY_LEFTCTRL, 0);
+		}
+
+		mods &= ~dmods;
+	}
+
+	send_mods(mods, 0);
+}
+
+
 
 static void kbd_lookup_descriptor(struct keyboard *kbd,
 				  uint16_t code,
@@ -114,7 +145,7 @@ static void kbd_lookup_descriptor(struct keyboard *kbd,
 	*descriptor_layer = dl;
 }
 
-/* Compute the current modifier state based on the activated layers. */
+/* Compute the current modifier set based on the activated layers. */
 static uint16_t kbd_compute_mods(struct keyboard *kbd)
 {
 	size_t i;
@@ -128,12 +159,6 @@ static uint16_t kbd_compute_mods(struct keyboard *kbd)
 	return mods;
 }
 
-/* Compute and apply the current mod state to the virtual keyboard.  */
-static void kbd_reify_mods(struct keyboard *kbd)
-{
-	set_mods(kbd_compute_mods(kbd));
-}
-
 /* Returns the active_layer struct associated with the given layer index. */
 static const struct active_layer *kbd_lookup_active_layer(struct keyboard *kbd, int layer)
 {
@@ -149,7 +174,7 @@ static const struct active_layer *kbd_lookup_active_layer(struct keyboard *kbd,
 	return NULL;
 }
 
-static void kbd_deactivate_layer(struct keyboard *kbd, int layer_idx)
+static void kbd_deactivate_layer(struct keyboard *kbd, int layer_idx, int disarm_mods_p)
 {
 	int i;
 	int n = kbd->nr_active_layers;
@@ -168,7 +193,10 @@ static void kbd_deactivate_layer(struct keyboard *kbd, int layer_idx)
 		}
 	}
 
-	send_mods(layer->mods & ~active_mods, 0);
+	if (disarm_mods_p)
+		disarm_mods(layer->mods & ~active_mods);
+	else
+		send_mods(layer->mods & ~active_mods, 0);
 }
 
 static void kbd_clear_oneshots(struct keyboard *kbd)
@@ -234,7 +262,7 @@ static void kbd_swap_layer(struct keyboard *kbd,
 		struct active_key *ak = &kbd->active_keys[i];
 
 		if((ak->d.op == OP_LAYER || ak->d.op == OP_OVERLOAD || ak->d.op == OP_SWAP) && ak->d.args[0].idx == dl) {
-			kbd_deactivate_layer(kbd, dl);
+			kbd_deactivate_layer(kbd, dl, 1);
 			kbd_activate_layer(kbd, replacment_layer, 0);
 
 			ak->d = new_descriptor;
@@ -304,33 +332,36 @@ void kbd_reset(struct keyboard *kbd)
 	kbd->config = kbd->original_config;
 }
 
-static long get_time_ms()
-{
-	struct timespec tv;
-	clock_gettime(CLOCK_MONOTONIC, &tv);
-	return (tv.tv_sec*1E3)+tv.tv_nsec/1E6;
-}
-
-void kbd_execute_sequence(struct keyboard *kbd, const struct key_sequence *sequence, int literally)
+static void kbd_start_sequence(struct keyboard *kbd, const struct key_sequence *sequence, int literally)
 {
 	uint16_t active_mods = kbd_compute_mods(kbd);
 
-	/* Avoid spurious key up actions */
 	if (!sequence->code && !sequence->mods)
 		return;
 
 	if (literally)
-		send_mods(active_mods, 0);
+		disarm_mods(active_mods);
 
 	send_mods(sequence->mods, 1);
 	send_key(sequence->code, 1);
+}
+
+static void kbd_end_sequence(struct keyboard *kbd, const struct key_sequence *sequence)
+{
+	uint16_t active_mods = kbd_compute_mods(kbd);
+
 	send_key(sequence->code, 0);
 	send_mods(sequence->mods, 0);
+	send_mods(active_mods, 1);
+}
 
-	if (literally)
-		send_mods(active_mods, 1);
+static void kbd_execute_sequence(struct keyboard *kbd, const struct key_sequence *sequence, int literally)
+{
+	kbd_start_sequence(kbd, sequence, literally);
+	kbd_end_sequence(kbd, sequence);
 }
 
+
 /*
  * Here be tiny dragons.
  *
@@ -347,9 +378,10 @@ long kbd_process_key_event(struct keyboard *kbd,
 	int dl;
 	struct descriptor d;
 	int timeout = 0;
+	int disarm = 1;
+	static struct key_sequence active_sequence = {0};
 
 	static int oneshot_latch = 0;
-	static int last_pressed_keycode = 0;
 	static const struct macro *active_macro = NULL;
 
 	static struct {
@@ -361,6 +393,11 @@ long kbd_process_key_event(struct keyboard *kbd,
 		uint8_t is_active;
 	} pending_overload = {0};
 
+	if (active_sequence.code) {
+		kbd_end_sequence(kbd, &active_sequence);
+		active_sequence.code = 0;
+	}
+
 	if (active_macro) {
 		if (!code) {
 			kbd_execute_macro(kbd, active_macro);
@@ -397,13 +434,17 @@ long kbd_process_key_event(struct keyboard *kbd,
 	const struct macro *macro;
 
 	case OP_KEYSEQ:
+		/* TODO: distinguish between key codes and key sequences at the config level. */
 		sequence = &d.args[0].sequence;
 		is_sequence = sequence->mods || dl != kbd->layout;
 
 		if (is_sequence) {
-			if (pressed)
-				kbd_execute_sequence(kbd, sequence, 1);
+			if (pressed) {
+				active_sequence = *sequence;
+				kbd_start_sequence(kbd, sequence, 1);
+			}
 		} else {
+			disarm = 0;
 			send_key(sequence->code, pressed);
 		}
 
@@ -424,7 +465,7 @@ long kbd_process_key_event(struct keyboard *kbd,
 				if (al->oneshot) /* Allow oneshot layers to toggle themselves. */
 					kbd_activate_layer(kbd, layer, 0);
 				else
-					kbd_deactivate_layer(kbd, layer);
+					kbd_deactivate_layer(kbd, layer, 1);
 			} else
 				kbd_activate_layer(kbd, layer, 0);
 		}
@@ -440,8 +481,13 @@ long kbd_process_key_event(struct keyboard *kbd,
 
 		if (pressed)
 			kbd_activate_layer(kbd, layer, 0);
-		else
-			kbd_deactivate_layer(kbd, layer);
+		else {
+			int disarm = kbd->disarm_flag;
+			if (kbd->last_pressed_keycode == code)
+				disarm = 0;
+
+			kbd_deactivate_layer(kbd, layer, disarm);
+		}
 
 		break;
 	case OP_ONESHOT:
@@ -453,7 +499,7 @@ long kbd_process_key_event(struct keyboard *kbd,
 		} else if (oneshot_latch) /* No interposed KEYSEQ since key down (other modifiers don't interfere with this).  */
 			kbd_activate_layer(kbd, layer, 1);
 		else
-			kbd_deactivate_layer(kbd, layer);
+			kbd_deactivate_layer(kbd, layer, 1);
 
 		break;
 	case OP_SWAP:
@@ -463,8 +509,8 @@ long kbd_process_key_event(struct keyboard *kbd,
 		if (pressed) {
 			kbd_execute_sequence(kbd, sequence, 1);
 			kbd_swap_layer(kbd, dl, layer, d);
-		} else if (last_pressed_keycode != code) {  /* We only reach this from the remapped dl activate key. */
-			kbd_deactivate_layer(kbd, layer);
+		} else if (kbd->last_pressed_keycode != code) {  /* We only reach this from the remapped dl activate key. */
+			kbd_deactivate_layer(kbd, layer, 1);
 		}
 
 		break;
@@ -474,12 +520,13 @@ long kbd_process_key_event(struct keyboard *kbd,
 
 		if (pressed) {
 			kbd_activate_layer(kbd, layer, 0);
-		} else if (last_pressed_keycode == code) {
-			kbd_deactivate_layer(kbd, layer);
+		} else if (kbd->last_pressed_keycode == code) {
+			kbd_deactivate_layer(kbd, layer, 1);
 
+			/* TODO: enforce this as a code in the config. */
 			kbd_execute_sequence(kbd, sequence, dl != kbd->layout);
 		} else {
-			kbd_deactivate_layer(kbd, layer);
+			kbd_deactivate_layer(kbd, layer, 1);
 		}
 
 		break;
@@ -495,7 +542,7 @@ long kbd_process_key_event(struct keyboard *kbd,
 
 			pending_overload.is_active = 1;
 		} else {
-			kbd_deactivate_layer(kbd, layer);
+			kbd_deactivate_layer(kbd, layer, 0);
 		}
 
 		break;
@@ -514,9 +561,6 @@ long kbd_process_key_event(struct keyboard *kbd,
 		break;
 	}
 
-	if (pressed)
-		last_pressed_keycode = code;
-
 	if (!d.op ||
 		(pressed &&
 		 (d.op == OP_KEYSEQ ||
@@ -526,6 +570,11 @@ long kbd_process_key_event(struct keyboard *kbd,
 		kbd_clear_oneshots(kbd);
 	}
 
+	if (pressed) {
+		kbd->last_pressed_keycode = code;
+		kbd->disarm_flag = disarm;
+	}
+
 
 	return timeout;
 }
diff --git a/src/keyboard.h b/src/keyboard.h
index 8c14ee3..f1fecee 100644
--- a/src/keyboard.h
+++ b/src/keyboard.h
@@ -38,6 +38,7 @@ struct keyboard {
 	size_t nr_active_keys;
 
 	uint16_t last_pressed_keycode;
+	int disarm_flag;
 
 	struct keyboard *next;
 };
diff --git a/t/disarm.t b/t/disarm.t
new file mode 100644
index 0000000..7a30626
--- /dev/null
+++ b/t/disarm.t
@@ -0,0 +1,9 @@
+alt down
+a down
+a up
+alt up
+
+alt down
+a down
+a up
+alt up
diff --git a/t/disarm2.t b/t/disarm2.t
new file mode 100644
index 0000000..a3a022c
--- /dev/null
+++ b/t/disarm2.t
@@ -0,0 +1,5 @@
+alt down
+alt up
+
+alt down
+alt up
diff --git a/t/disarm3.t b/t/disarm3.t
new file mode 100644
index 0000000..a534fca
--- /dev/null
+++ b/t/disarm3.t
@@ -0,0 +1,15 @@
+alt down
+m down
+m up
+alt up
+
+alt down
+control down
+alt up
+control up
+x down
+x up
+alt down
+control down
+alt up
+control up
diff --git a/t/meta.t b/t/meta.t
index 638ec8e..d2b33e6 100644
--- a/t/meta.t
+++ b/t/meta.t
@@ -9,14 +9,22 @@ rightmeta up
 
 
 meta down
+control down
 meta up
+control up
 b down
 b up
 meta down
+control down
 meta up
+control up
 meta down
+control down
 meta up
+control up
 b down
 b up
 meta down
+control down
 meta up
+control up
diff --git a/t/swap.t b/t/swap.t
index ff9dcd5..8443256 100644
--- a/t/swap.t
+++ b/t/swap.t
@@ -10,7 +10,9 @@ a up
 alt up
 
 alt down
+control down
 alt up
+control up
 shift down
 x down
 x up
diff --git a/t/swap2.t b/t/swap2.t
index 10d5754..8f58c71 100644
--- a/t/swap2.t
+++ b/t/swap2.t
@@ -12,7 +12,9 @@ a up
 alt up
 
 alt down
+control down
 alt up
+control up
 shift down
 x down
 x up
diff --git a/t/swap4.t b/t/swap4.t
index 3ed2a79..bc5ad38 100644
--- a/t/swap4.t
+++ b/t/swap4.t
@@ -12,11 +12,15 @@ x up
 alt up
 
 alt down
+control down
 alt up
+control up
 tab down
 tab up
 alt down
+control down
 alt up
+control up
 shift down
 x down
 x up
diff --git a/t/swap5.t b/t/swap5.t
index fe00260..9c0ac87 100644
--- a/t/swap5.t
+++ b/t/swap5.t
@@ -9,13 +9,17 @@ meta up
 
 meta down
 alt down
+control down
 meta up
 alt up
+control up
 tab down
 tab up
 meta down
 alt down
+control down
 alt up
+control up
 control down
 a down
 a up
diff --git a/t/swap7.t b/t/swap7.t
index 0ee5afb..d3b26d9 100644
--- a/t/swap7.t
+++ b/t/swap7.t
@@ -10,7 +10,9 @@ a up
 alt up
 
 alt down
+control down
 alt up
+control up
 b down
 b up
 c down
diff --git a/t/test.conf b/t/test.conf
index 93ed81a..e35148f 100644
--- a/t/test.conf
+++ b/t/test.conf
@@ -66,6 +66,7 @@ a = b
 
 [myalt:A]
 
+m = x
 s = swap(swapped1)
 ` = swap(tablayer)
 1 = swap(tablayer, tab)