feat(override): add defcfg option to eagerly release override output (#…

…1120)

cfg_samples/kanata.kbd

@@ -254,6 +254,14 @@ If you need help, please feel welcome to ask in the GitHub discussions.
  ;; changes will be logged.
  ;;
  ;; log-layer-changes no
+
+ ;; This configuration will press and then immediately release the non-modifier key
+ ;; as soon as the override activates, meaning you are unlikely as a human to ever
+ ;; release modifiers first, which can result in unintended behaviour.
+ ;;
+ ;; The downside of this configuration is that the non-modifier key
+ ;; does not remain held which is important to consider for your use cases.
+ override-release-on-activation yes
 )
 
 ;; deflocalkeys-* enables you to define and use key names that match your locale

docs/config.adoc

@@ -2188,6 +2188,42 @@ The default (and minimum) value is `5` and the unit is milliseconds.
 )
 ----
 
+[[override-release-on-activation]]
+=== override-release-on-activation
+<<table-of-contents,Back to ToC>>
+
+This configuration item changes activation behaviour from `defoverrides`.
+
+Take this example override:
+
+[source]
+----
+(defoverrides (lsft a) (lsft 9))
+----
+
+The default behaviour is that if `lsft` is released **before** releasing `a`,
+kanata's behaviour would be to send `a`.
+
+A future improvement could be to make the `9` continue to be the key held,
+but that is not implemented today.
+
+The workaround in case the above behaviour negatively impacts your workflow
+is to enable this configuration.
+This configuration will press and then immediately release the `9` output
+as soon as the override activates, meaning you are unlikely as a human to ever
+release `lsft` first.
+
+The effect of this configuration is that the `9` key cannot remain held
+when activated by the override which is important to consider for your use cases.
+
+.Example:
+[source]
+----
+(defcfg
+ override-release-on-activation yes
+)
+----
+
 [[linux-only-linux-dev]]
 === Linux only: linux-dev
 <<table-of-contents,Back to ToC>>

keyberon/src/layout.rs

@@ -362,7 +362,10 @@ impl<'a, T: 'a> State<'a, T> {
  }
  pub fn release_state(&self, s: ReleasableState) -> Option<Self> {
  match (*self, s) {
- (NormalKey { keycode: k1, .. }, ReleasableState::KeyCode(k2)) => {
+ (
+ NormalKey { keycode: k1, .. } | FakeKey { keycode: k1 },
+ ReleasableState::KeyCode(k2),
+ ) => {
  if k1 == k2 {
  None
  } else {

parser/src/cfg/defcfg.rs

@@ -61,6 +61,7 @@ pub struct CfgOptions {
  pub delegate_to_first_layer: bool,
  pub movemouse_inherit_accel_state: bool,
  pub movemouse_smooth_diagonals: bool,
+ pub override_release_on_activation: bool,
  pub dynamic_macro_max_presses: u16,
  pub dynamic_macro_replay_delay_behaviour: ReplayDelayBehaviour,
  pub concurrent_tap_hold: bool,
@@ -115,6 +116,7 @@ impl Default for CfgOptions {
  delegate_to_first_layer: false,
  movemouse_inherit_accel_state: false,
  movemouse_smooth_diagonals: false,
+ override_release_on_activation: false,
  dynamic_macro_max_presses: 128,
  dynamic_macro_replay_delay_behaviour: ReplayDelayBehaviour::Recorded,
  concurrent_tap_hold: false,
@@ -587,6 +589,9 @@ pub fn parse_defcfg(expr: &[SExpr]) -> Result<CfgOptions> {
  "movemouse-inherit-accel-state" => {
  cfg.movemouse_inherit_accel_state = parse_defcfg_val_bool(val, label)?
  }
+ "override-release-on-activation" => {
+ cfg.override_release_on_activation = parse_defcfg_val_bool(val, label)?
+ }
  "concurrent-tap-hold" => {
  cfg.concurrent_tap_hold = parse_defcfg_val_bool(val, label)?
  }

parser/src/cfg/key_override.rs

@@ -56,6 +56,10 @@ impl OverrideStates {
  fn add_overrides(&self, oscs: &mut Vec<KeyCode>) {
  oscs.extend(self.oscs_to_add.iter().copied().map(KeyCode::from));
  }
+
+ pub fn removed_oscs(&self) -> impl Iterator<Item = OsCode> + '_ {
+ self.oscs_to_remove.iter().copied()
+ }
 }
 
 /// A collection of global key overrides.
@@ -84,12 +88,12 @@ impl Overrides {
  if self.is_empty() {
  return;
  }
+ states.cleanup();
  for kc in kcs.iter().copied() {
  states.update(kc.into(), self);
  }
  kcs.retain(|kc| !states.is_key_overridden((*kc).into()));
  states.add_overrides(kcs);
- states.cleanup();
  }
 
  pub fn output_non_mods_for_input_non_mod(&self, in_osc: OsCode) -> Vec<OsCode> {

parser/src/cfg/tests.rs

@@ -1355,6 +1355,7 @@ fn parse_all_defcfg() {
  delegate-to-first-layer yes
  movemouse-inherit-accel-state yes
  movemouse-smooth-diagonals yes
+ override-release-on-activation yes
  dynamic-macro-max-presses 1000
  concurrent-tap-hold yes
  rapid-event-delay 5

parser/src/keys/mod.rs

@@ -66,6 +66,20 @@ impl OsCode {
  #[cfg(target_os = "macos")]
  return OsCode::from_u16_macos(code);
  }
+
+ pub fn is_modifier(self) -> bool {
+ matches!(
+ self,
+ OsCode::KEY_LEFTSHIFT
+ | OsCode::KEY_RIGHTSHIFT
+ | OsCode::KEY_LEFTMETA
+ | OsCode::KEY_RIGHTMETA
+ | OsCode::KEY_LEFTCTRL
+ | OsCode::KEY_RIGHTCTRL
+ | OsCode::KEY_LEFTALT
+ | OsCode::KEY_RIGHTALT
+ )
+ }
 }
 
 static CUSTOM_STRS_TO_OSCODES: Lazy<Mutex<HashMap<String, OsCode>>> = Lazy::new(|| {

src/kanata/mod.rs

@@ -11,6 +11,7 @@ use std::sync::mpsc::{Receiver, SyncSender as Sender, TryRecvError};
 #[cfg(feature = "passthru_ahk")]
 use std::sync::mpsc::Sender as ASender;
 
+use kanata_keyberon::action::ReleasableState;
 use kanata_keyberon::key_code::*;
 use kanata_keyberon::layout::{CustomEvent, Event, Layout, State};
 
@@ -186,6 +187,7 @@ pub struct Kanata {
  /// gets stored in this buffer and if the next movemouse action is opposite axis
  /// than the one stored in the buffer, both events are outputted at the same time.
  movemouse_buffer: Option<(Axis, CalculatedMouseMove)>,
+ override_release_on_activation: bool,
  /// Configured maximum for dynamic macro recording, to protect users from themselves if they
  /// have accidentally left it on.
  dynamic_macro_max_presses: u16,
@@ -355,6 +357,7 @@ impl Kanata {
  .unwrap_or(cfg.options.log_layer_changes),
  caps_word: None,
  movemouse_smooth_diagonals: cfg.options.movemouse_smooth_diagonals,
+ override_release_on_activation: cfg.options.override_release_on_activation,
  movemouse_inherit_accel_state: cfg.options.movemouse_inherit_accel_state,
  dynamic_macro_max_presses: cfg.options.dynamic_macro_max_presses,
  dynamic_macro_replay_behaviour: ReplayBehaviour {
@@ -454,6 +457,7 @@ impl Kanata {
  .unwrap_or(cfg.options.log_layer_changes),
  caps_word: None,
  movemouse_smooth_diagonals: cfg.options.movemouse_smooth_diagonals,
+ override_release_on_activation: cfg.options.override_release_on_activation,
  movemouse_inherit_accel_state: cfg.options.movemouse_inherit_accel_state,
  dynamic_macro_max_presses: cfg.options.dynamic_macro_max_presses,
  dynamic_macro_replay_behaviour: ReplayBehaviour {
@@ -511,6 +515,7 @@ impl Kanata {
  self.log_layer_changes =
  get_forced_log_layer_changes().unwrap_or(cfg.options.log_layer_changes);
  self.movemouse_smooth_diagonals = cfg.options.movemouse_smooth_diagonals;
+ self.override_release_on_activation = cfg.options.override_release_on_activation;
  self.movemouse_inherit_accel_state = cfg.options.movemouse_inherit_accel_state;
  self.dynamic_macro_max_presses = cfg.options.dynamic_macro_max_presses;
  self.dynamic_macro_replay_behaviour = ReplayBehaviour {
@@ -935,6 +940,17 @@ impl Kanata {
 
  self.overrides
  .override_keys(cur_keys, &mut self.override_states);
+ if self.override_release_on_activation {
+ for removed in self.override_states.removed_oscs() {
+ if !removed.is_modifier() {
+ layout.states.retain(|s| {
+ s.release_state(ReleasableState::KeyCode(removed.into()))
+ .is_some()
+ });
+ }
+ }
+ }
+
  if let Some(caps_word) = &mut self.caps_word {
  if caps_word.maybe_add_lsft(cur_keys) == CapsWordNextState::End {
  self.caps_word = None;

src/kanata/sequences.rs

@@ -317,14 +317,7 @@ pub(super) fn do_successful_sequence_termination(
  // desired to fix this, a shorter list of keys would
  // probably be the list of keys that **do** output
  // characters than those that don't.
- OsCode::KEY_LEFTSHIFT
- | OsCode::KEY_RIGHTSHIFT
- | OsCode::KEY_LEFTMETA
- | OsCode::KEY_RIGHTMETA
- | OsCode::KEY_LEFTCTRL
- | OsCode::KEY_RIGHTCTRL
- | OsCode::KEY_LEFTALT
- | OsCode::KEY_RIGHTALT => continue,
+ osc if osc.is_modifier() => continue,
  osc if matches!(u16::from(osc), KEY_IGNORE_MIN..=KEY_IGNORE_MAX) => continue,
  _ => {
  kbd_out.press_key(OsCode::KEY_BACKSPACE)?;

src/tests/sim_tests/override_tests.rs

@@ -25,3 +25,36 @@ fn override_with_unmod() {
  result
  );
 }
+
+#[test]
+fn override_release_mod_change_key() {
+ let result = simulate(
+ "
+(defsrc)
+(deflayer base)
+(defoverrides (lsft a) (lsft 9))
+ ",
+ "d:lsft t:10 d:a t:10 u:lsft t:10 u:a t:10",
+ )
+ .to_ascii()
+ .no_time();
+ assert_eq!("dn:LShift dn:Kb9 up:LShift up:Kb9 dn:A up:A", result);
+}
+
+#[test]
+fn override_eagerly_releases() {
+ let result = simulate(
+ "
+(defcfg override-release-on-activation yes)
+(defsrc)
+(deflayer base)
+(defoverrides (lsft a) (lsft 9))
+ ",
+ "d:lsft t:10 d:a t:10 u:lsft t:10 u:a t:10",
+ )
+ .to_ascii();
+ assert_eq!(
+ "dn:LShift t:10ms dn:Kb9 t:1ms up:Kb9 t:9ms up:LShift",
+ result
+ );
+}