fix(repeat): check event code itself for repeat output (#986)

This commit fixes key repeat in Linux TTY and Windows when a key on the
base layer is transparent, OR process-unmapped-keys is enabled and the
key is unmapped.

This functionality was broken in commit 089853a which changed how layers
worked by removing the duplicated base-version and while-held-version of
layers, for the benefit of reduced memory consumption.

src/kanata/key_repeat.rs

@@ -0,0 +1,95 @@
+use super::*;
+
+impl Kanata {
+    /// This compares the active keys in the keyberon layout against the potential key outputs for
+    /// corresponding physical key in the configuration. If any of keyberon active keys match any
+    /// potential physical key output, write the repeat event to the OS.
+    pub(super) fn handle_repeat(&mut self, event: &KeyEvent) -> Result<()> {
+        let ret = self.handle_repeat_actual(event);
+        // The cur_keys Vec is re-used for processing, for efficiency reasons to avoid allocation.
+        // Unlike prev_keys which has useful info for the next call to handle_time_ticks, cur_keys
+        // can be reused and cleared — it just needs to be empty for the next handle_time_ticks
+        // call.
+        self.cur_keys.clear();
+        ret
+    }
+
+    pub(super) fn handle_repeat_actual(&mut self, event: &KeyEvent) -> Result<()> {
+        if self.sequence_state.is_some() {
+            // While in sequence mode, don't send key repeats. I can't imagine it's a helpful use
+            // case for someone trying to type in a sequence that they want to rely on key repeats
+            // to finish a sequence. I suppose one might want to do repeat in order to try and
+            // cancel an input sequence... I'll wait for a user created issue to deal with this.
+            return Ok(());
+        }
+        self.cur_keys.extend(self.layout.bm().keycodes());
+        self.overrides
+            .override_keys(&mut self.cur_keys, &mut self.override_states);
+
+        // Prioritize checking the active layer in case a layer-while-held is active.
+        let active_held_layers = self.layout.bm().trans_resolution_layer_order();
+        let mut held_layer_active = false;
+        for layer in active_held_layers {
+            held_layer_active = true;
+            if let Some(outputs_for_key) = self.key_outputs[usize::from(layer)].get(&event.code) {
+                log::debug!("key outs for active layer-while-held: {outputs_for_key:?};");
+                for osc in outputs_for_key.iter().rev().copied() {
+                    let kc = osc.into();
+                    if self.cur_keys.contains(&kc)
+                        || self.unshifted_keys.contains(&kc)
+                        || self.unmodded_keys.contains(&kc)
+                    {
+                        log::debug!("repeat    {:?}", KeyCode::from(osc));
+                        if let Err(e) = write_key(&mut self.kbd_out, osc, KeyValue::Repeat) {
+                            bail!("could not write key {e:?}")
+                        }
+                        return Ok(());
+                    }
+                }
+            }
+        }
+        if held_layer_active {
+            log::debug!("empty layer-while-held outputs, probably transparent");
+        }
+
+        if let Some(outputs_for_key) =
+            self.key_outputs[self.layout.bm().default_layer].get(&event.code)
+        {
+            // Try matching a key on the default layer.
+            //
+            // This code executes in two cases:
+            // 1. current layer is the default layer
+            // 2. current layer is layer-while-held but did not find a match in the code above, e.g. a
+            //    transparent key was pressed.
+            log::debug!("key outs for default layer: {outputs_for_key:?};");
+            for osc in outputs_for_key.iter().rev().copied() {
+                let kc = osc.into();
+                if self.cur_keys.contains(&kc)
+                    || self.unshifted_keys.contains(&kc)
+                    || self.unmodded_keys.contains(&kc)
+                {
+                    log::debug!("repeat    {:?}", KeyCode::from(osc));
+                    if let Err(e) = write_key(&mut self.kbd_out, osc, KeyValue::Repeat) {
+                        bail!("could not write key {e:?}")
+                    }
+                    return Ok(());
+                }
+            }
+        }
+
+        // Reached here and have not exited yet.
+        // Check the standard key output itself because default layer might also be transparent
+        // and have delegated to defsrc handling.
+        log::debug!("checking defsrc output");
+        let kc = event.code.into();
+        if self.cur_keys.contains(&kc)
+            || self.unshifted_keys.contains(&kc)
+            || self.unmodded_keys.contains(&kc)
+        {
+            if let Err(e) = write_key(&mut self.kbd_out, event.code, KeyValue::Repeat) {
+                bail!("could not write key {e:?}");
+            }
+        }
+        Ok(())
+    }
+}

src/kanata/mod.rs

@@ -20,6 +20,7 @@ use crate::tcp_server::simple_sexpr_to_json_array;
 use crate::SocketAddrWrapper;
 use crate::ValidatedArgs;
 use kanata_parser::cfg;
+use kanata_parser::cfg::list_actions::*;
 use kanata_parser::cfg::*;
 use kanata_parser::custom_action::*;
 pub use kanata_parser::keys::*;
@@ -28,7 +29,7 @@ use kanata_tcp_protocol::ServerMessage;
 mod dynamic_macro;
 use dynamic_macro::*;
 
-use kanata_parser::cfg::list_actions::*;
+mod key_repeat;
 
 #[cfg(feature = "cmd")]
 mod cmd;
@@ -1580,85 +1581,6 @@ impl Kanata {
         Ok(live_reload_requested)
     }
 
-    /// This compares the active keys in the keyberon layout against the potential key outputs for
-    /// corresponding physical key in the configuration. If any of keyberon active keys match any
-    /// potential physical key output, write the repeat event to the OS.
-    fn handle_repeat(&mut self, event: &KeyEvent) -> Result<()> {
-        let ret = self.handle_repeat_actual(event);
-        // The cur_keys Vec is re-used for processing, for efficiency reasons to avoid allocation.
-        // Unlike prev_keys which has useful info for the next call to handle_time_ticks, cur_keys
-        // can be reused and cleared — it just needs to be empty for the next handle_time_ticks
-        // call.
-        self.cur_keys.clear();
-        ret
-    }
-
-    fn handle_repeat_actual(&mut self, event: &KeyEvent) -> Result<()> {
-        if self.sequence_state.is_some() {
-            // While in sequence mode, don't send key repeats. I can't imagine it's a helpful use
-            // case for someone trying to type in a sequence that they want to rely on key repeats
-            // to finish a sequence. I suppose one might want to do repeat in order to try and
-            // cancel an input sequence... I'll wait for a user created issue to deal with this.
-            return Ok(());
-        }
-        self.cur_keys.extend(self.layout.bm().keycodes());
-        self.overrides
-            .override_keys(&mut self.cur_keys, &mut self.override_states);
-
-        // Prioritize checking the active layer in case a layer-while-held is active.
-        let active_held_layers = self.layout.bm().active_held_layers();
-        let mut held_layer_active = false;
-        for layer in active_held_layers {
-            held_layer_active = true;
-            if let Some(outputs_for_key) = self.key_outputs[usize::from(layer)].get(&event.code) {
-                log::debug!("key outs for active layer-while-held: {outputs_for_key:?};");
-                for osc in outputs_for_key.iter().rev().copied() {
-                    let kc = osc.into();
-                    if self.cur_keys.contains(&kc)
-                        || self.unshifted_keys.contains(&kc)
-                        || self.unmodded_keys.contains(&kc)
-                    {
-                        log::debug!("repeat    {:?}", KeyCode::from(osc));
-                        if let Err(e) = write_key(&mut self.kbd_out, osc, KeyValue::Repeat) {
-                            bail!("could not write key {:?}", e)
-                        }
-                        return Ok(());
-                    }
-                }
-            }
-        }
-        if held_layer_active {
-            log::debug!("empty layer-while-held outputs, probably transparent");
-        }
-
-        // Try matching a key on the default layer.
-        //
-        // This code executes in two cases:
-        // 1. current layer is the default layer
-        // 2. current layer is layer-while-held but did not find a match in the code above, e.g. a
-        //    transparent key was pressed.
-        let outputs_for_key =
-            match self.key_outputs[self.layout.bm().default_layer].get(&event.code) {
-                None => return Ok(()),
-                Some(v) => v,
-            };
-        log::debug!("key outs for default layer: {outputs_for_key:?};");
-        for osc in outputs_for_key.iter().rev().copied() {
-            let kc = osc.into();
-            if self.cur_keys.contains(&kc)
-                || self.unshifted_keys.contains(&kc)
-                || self.unmodded_keys.contains(&kc)
-            {
-                log::debug!("repeat    {:?}", KeyCode::from(osc));
-                if let Err(e) = write_key(&mut self.kbd_out, osc, KeyValue::Repeat) {
-                    bail!("could not write key {:?}", e)
-                }
-                return Ok(());
-            }
-        }
-        Ok(())
-    }
-
     #[cfg(feature = "tcp_server")]
     pub fn change_layer(&mut self, layer_name: String) {
         for (i, l) in self.layer_info.iter().enumerate() {

src/tests/sim_tests/mod.rs

@@ -13,6 +13,7 @@ use kanata_state_machine::{
 mod block_keys_tests;
 mod chord_sim_tests;
 mod layer_sim_tests;
+mod repeat_sim_tests;
 mod seq_sim_tests;
 
 fn simulate(cfg: &str, sim: &str) -> String {

src/tests/sim_tests/repeat_sim_tests.rs

@@ -0,0 +1,136 @@
+use super::*;
+
+#[test]
+fn repeat_standard() {
+    let result = simulate(
+        "
+         (defsrc a)
+         (deflayer base b)
+        ",
+        "
+         d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "out:↓B\nt:10ms\nout:↓B\nt:10ms\nout:↓B\nt:10ms\nout:↑B",
+        result
+    );
+}
+
+#[test]
+fn repeat_layer_while_held() {
+    let result = simulate(
+        "
+         (defsrc a b)
+         (deflayer base a (layer-while-held held))
+         (deflayer held c b)
+        ",
+        "
+         d:b t:10 r:b t:10 d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "t:20ms\nout:↓C\nt:10ms\nout:↓C\nt:10ms\nout:↓C\nt:10ms\nout:↑C",
+        result
+    );
+}
+
+#[test]
+fn repeat_layer_switch() {
+    let result = simulate(
+        "
+         (defsrc a b)
+         (deflayer base a (layer-switch swtc))
+         (deflayer swtc d b)
+        ",
+        "
+         d:b t:10 r:b t:10 d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "t:20ms\nout:↓D\nt:10ms\nout:↓D\nt:10ms\nout:↓D\nt:10ms\nout:↑D",
+        result
+    );
+}
+
+#[test]
+fn repeat_layer_held_trans() {
+    let result = simulate(
+        "
+         (defsrc a b)
+         (deflayer base e (layer-while-held held))
+         (deflayer held _ b)
+        ",
+        "
+         d:b t:10 r:b t:10 d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "t:20ms\nout:↓E\nt:10ms\nout:↓E\nt:10ms\nout:↓E\nt:10ms\nout:↑E",
+        result
+    );
+}
+
+#[test]
+fn repeat_many_layer_held_trans() {
+    let result = simulate(
+        "
+         (defsrc a b c d e)
+         (deflayer base e (layer-while-held held1) _ _ _)
+         (deflayer held1 f b (layer-while-held held2) _ _)
+         (deflayer held2 _ _ _ (layer-while-held held3) _)
+         (deflayer held3 _ _ _ _ (layer-while-held held4))
+         (deflayer held4 _ _ _ _ _)
+        ",
+        "
+         d:b t:10 r:b t:10
+         d:c t:10 r:c t:10
+         d:d t:10 r:d t:10
+         d:e t:10 r:e t:10
+         d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "t:80ms\nout:↓F\nt:10ms\nout:↓F\nt:10ms\nout:↓F\nt:10ms\nout:↑F",
+        result
+    );
+}
+
+#[test]
+fn repeat_base_layer_trans() {
+    let result = simulate(
+        "
+         (defsrc a)
+         (deflayer base _)
+        ",
+        "
+         d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+        ",
+    );
+    assert_eq!(
+        "out:↓A\nt:10ms\nout:↓A\nt:10ms\nout:↓A\nt:10ms\nout:↑A",
+        result
+    );
+}
+
+#[test]
+fn repeat_delegate_to_base_layer_trans() {
+    let result = simulate(
+        "
+         (defcfg delegate-to-first-layer yes)
+         (defsrc a c b)
+         (deflayer base e _ (layer-switch swtc))
+         (deflayer swtc _ _ _)
+        ",
+        "
+         d:b t:10 r:b t:10
+         d:a t:10 r:a t:10 r:a t:10 u:a t:10 r:a
+         d:c t:10 r:c t:10 r:c t:10 u:c t:10 r:c
+        ",
+    );
+    assert_eq!(
+        "t:20ms\nout:↓E\nt:10ms\nout:↓E\nt:10ms\nout:↓E\nt:10ms\nout:↑E\n\
+         t:10ms\nout:↓C\nt:10ms\nout:↓C\nt:10ms\nout:↓C\nt:10ms\nout:↑C",
+        result
+    );
+}