feat: release key/layer actions in chord when all keys are released (#…

…406)

docs/config.adoc

@@ -1728,6 +1728,41 @@ and it maps to a tap-hold with a chord as the hold action within, that chord
 key will immediately activate instead of the key needing to be held for the
 timeout period.
 
+**Release behaviour**
+
+For single key actions and output chords — like `lctl` or `S-tab` —
+and for `layer-while-held`,
+an input chord will release the action only when all keys that are part of
+the input chord have been released.
+In other words, if even one key is held for the input chord
+then the output action will be continued to be held,
+but only for the mentioned action categories.
+
+An exception to the behaviour described above
+for the action categories that would normally apply
+is if a chord decomposition occurs.
+A chord decomposition occurs when you input a chord
+that does not correspond to any action.
+When this happens, kanata splits up the key presses to activate
+other actions from the components of the input chord.
+In this scenario, the behaviour described in the next paragraph will occur.
+
+For chord decompositions and all other action categories,
+the release behaviour is more confusing:
+the output action will end when any key is released during the timeout,
+or if the timeout expires, the output action ends when the *first* key
+that was pressed in the chord gets released.
+This inconsistency is a limitation of the current implementation.
+In these scenarios it is recommended
+to hold down all keys if you want to keep holding
+and to release all keys if you want to do a release.
+This is because it will probably be difficult
+to know which key was pressed first.
+
+If you want to bypass the behaviour of keys being held for chord outputs,
+you could change the chord output actions to be <<macro,macros>> instead.
+Using a macro will guarantee a rapid press+release for the output keys.
+
 [[defaliasenvcond]]
 === defaliasenvcond
 <<table-of-contents,Back to ToC>>

keyberon/src/layout.rs

@@ -29,6 +29,9 @@ use heapless::Vec;
 
 use State::*;
 
+/// The coordinate type.
+pub type KCoord = (u8, u16);
+
 /// The Layers type.
 ///
 /// `Layers` type is an array of layers which contain the description
@@ -39,17 +42,23 @@ use State::*;
 pub type Layers<'a, const C: usize, const R: usize, const L: usize, T = core::convert::Infallible> =
  [[[Action<'a, T>; C]; R]; L];
 
+const QUEUE_SIZE: usize = 32;
+
 /// The current event queue.
 ///
 /// Events can be retrieved by iterating over this struct and calling [Queued::event].
-type Queue = ArrayDeque<[Queued; 32], arraydeque::behavior::Wrapping>;
+type Queue = ArrayDeque<[Queued; QUEUE_SIZE], arraydeque::behavior::Wrapping>;
+
+/// A list of queued press events. Used for special handling of potentially multiple press events
+/// that occur during a Waiting event.
+type PressedQueue = ArrayDeque<[KCoord; QUEUE_SIZE]>;
 
 /// The queue is currently only used for chord decomposition when a longer chord does not result in
 /// an action, but splitting it into smaller chords would. The buffer size of 8 should be more than
 /// enough for real world usage, but if one wanted to be extra safe, this should be ChordKeys::BITS
 /// since that should guarantee that all potentially queueable actions can fit.
 type ActionQueue<'a, T> = ArrayDeque<[QueuedAction<'a, T>; 8], arraydeque::behavior::Wrapping>;
-type QueuedAction<'a, T> = Option<((u8, u16), &'a Action<'a, T>)>;
+type QueuedAction<'a, T> = Option<(KCoord, &'a Action<'a, T>)>;
 
 /// The layout manager. It takes `Event`s and `tick`s as input, and
 /// generate keyboard reports.
@@ -81,7 +90,7 @@ pub enum Event {
 }
 impl Event {
  /// Returns the coordinates (i, j) of the event.
- pub fn coord(self) -> (u8, u16) {
+ pub fn coord(self) -> KCoord {
  match self {
  Event::Press(i, j) => (i, j),
  Event::Release(i, j) => (i, j),
@@ -99,7 +108,7 @@ impl Event {
  /// Event::Press(3, 1).transform(|i, j| (i, 11 - j)),
  /// );
  /// ```
- pub fn transform(self, f: impl FnOnce(u8, u16) -> (u8, u16)) -> Self {
+ pub fn transform(self, f: impl FnOnce(u8, u16) -> KCoord) -> Self {
  match self {
  Event::Press(i, j) => {
  let (i, j) = f(i, j);
@@ -159,22 +168,22 @@ impl<'a, T> CustomEvent<'a, T> {
 pub enum State<'a, T: 'a> {
  NormalKey {
  keycode: KeyCode,
- coord: (u8, u16),
+ coord: KCoord,
  },
  LayerModifier {
  value: usize,
- coord: (u8, u16),
+ coord: KCoord,
  },
  Custom {
  value: &'a T,
- coord: (u8, u16),
+ coord: KCoord,
  },
  FakeKey {
  keycode: KeyCode,
  }, // Fake key event for sequences
  RepeatingSequence {
  sequence: &'a &'a [SequenceEvent<'a, T>],
- coord: (u8, u16),
+ coord: KCoord,
  },
  SeqCustomPending(&'a T),
  SeqCustomActive(&'a T),
@@ -198,7 +207,7 @@ impl<'a, T: 'a> State<'a, T> {
  Some(*self)
  }
  /// Returns None if the key has been released and Some otherwise.
- pub fn release(&self, c: (u8, u16), custom: &mut CustomEvent<'a, T>) -> Option<Self> {
+ pub fn release(&self, c: KCoord, custom: &mut CustomEvent<'a, T>) -> Option<Self> {
  match *self {
  NormalKey { coord, .. }
  | LayerModifier { coord, .. }
@@ -256,7 +265,7 @@ struct TapDanceState<'a, T: 'a> {
 
 #[derive(Copy, Clone, Debug)]
 pub struct TapDanceEagerState<'a, T: 'a> {
- coord: (u8, u16),
+ coord: KCoord,
  actions: &'a [&'a Action<'a, T>],
  timeout: u16,
  orig_timeout: u16,
@@ -291,7 +300,7 @@ enum WaitingConfig<'a, T: 'a + std::fmt::Debug> {
 
 #[derive(Debug)]
 pub struct WaitingState<'a, T: 'a + std::fmt::Debug> {
- coord: (u8, u16),
+ coord: KCoord,
  timeout: u16,
  delay: u16,
  ticks: u16,
@@ -319,9 +328,10 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  &mut self,
  queued: &mut Queue,
  action_queue: &mut ActionQueue<'a, T>,
- ) -> Option<WaitingAction> {
+ ) -> Option<(WaitingAction, Option<PressedQueue>)> {
  self.timeout = self.timeout.saturating_sub(1);
  self.ticks = self.ticks.saturating_add(1);
+ let mut pq = None;
  let (ret, cfg_change) = match self.config {
  WaitingConfig::HoldTap(htc) => (self.handle_hold_tap(htc, queued), None),
  WaitingConfig::TapDance(ref tds) => {
@@ -342,8 +352,9 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  )
  }
  WaitingConfig::Chord(config) => {
- if let Some((ret, action)) = self.handle_chord(config, queued, action_queue) {
+ if let Some((ret, action, cpq)) = self.handle_chord(config, queued, action_queue) {
  self.tap = action;
+ pq = Some(cpq);
  (Some(ret), None)
  } else {
  (None, None)
@@ -353,7 +364,7 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  if let Some(cfg) = cfg_change {
  self.config = cfg;
  }
- ret
+ ret.map(|v| (v, pq))
  }
 
  fn handle_hold_tap(&mut self, cfg: HoldTapConfig, queued: &Queue) -> Option<WaitingAction> {
@@ -451,9 +462,10 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  config: &'a ChordsGroup<'a, T>,
  queued: &mut Queue,
  action_queue: &mut ActionQueue<'a, T>,
- ) -> Option<(WaitingAction, &'a Action<'a, T>)> {
+ ) -> Option<(WaitingAction, &'a Action<'a, T>, PressedQueue)> {
  // need to keep track of how many Press events we handled so we can filter them out later
  let mut handled_press_events = 0;
+ let mut released_coord = None;
 
  // Compute the set of chord keys that are currently pressed
  // `Ok` when chording mode may continue
@@ -469,7 +481,12 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  handled_press_events += 1;
  Ok(active | chord_keys)
  }
- Event::Release(_, _) => Err(active), // released a chord key, abort
+ Event::Release(i, j) => {
+ // release chord quickly by changing the coordinate to the released
+ // key, to be consistent with chord decomposition behaviour.
+ released_coord = Some((i, j));
+ Err(active)
+ }
  }
  } else if matches!(s.event, Event::Press(..)) {
  Err(active) // pressed a non-chord key, abort
@@ -489,6 +506,9 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  Ok(active) => {
  // Chording mode still active, only trigger action if it's unambiguous
  if let Some(action) = config.get_chord_if_unambiguous(active) {
+ if let Some(coord) = released_coord {
+ self.coord = coord;
+ }
  (WaitingAction::Tap, action)
  } else {
  return None; // nothing to do yet, we'll check back later
@@ -497,6 +517,9 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  Err(active) => {
  // Abort chording mode. Trigger a chord action if there is one.
  if let Some(action) = config.get_chord(active) {
+ if let Some(coord) = released_coord {
+ self.coord = coord;
+ }
  (WaitingAction::Tap, action)
  } else {
  self.decompose_chord_into_action_queue(config, queued, action_queue);
@@ -505,21 +528,24 @@ impl<'a, T: std::fmt::Debug> WaitingState<'a, T> {
  }
  };
 
- // Consume all press events that were logically handled by this chording event
+ let mut pq = PressedQueue::new();
+
+ // Return all press events that were logically handled by this chording event
  queued.retain(|s| {
  if self.delay.saturating_sub(s.since) > self.timeout {
  true
  } else if matches!(s.event, Event::Press(i, j) if config.get_keys((i, j)).is_some())
  && handled_press_events > 0
  {
  handled_press_events -= 1;
+ let _ = pq.push_back(s.event().coord());
  false
  } else {
  true
  }
  });
 
- Some(res)
+ Some((res.0, res.1, pq))
  }
 
  fn decompose_chord_into_action_queue(
@@ -643,14 +669,14 @@ pub struct SequenceState<'a, T: 'a> {
  remaining_events: &'a [SequenceEvent<'a, T>],
 }
 
-type OneShotKeys = [(u8, u16); ONE_SHOT_MAX_ACTIVE];
-type ReleasedOneShotKeys = Vec<(u8, u16), ONE_SHOT_MAX_ACTIVE>;
+type OneShotKeys = [KCoord; ONE_SHOT_MAX_ACTIVE];
+type ReleasedOneShotKeys = Vec<KCoord, ONE_SHOT_MAX_ACTIVE>;
 
 /// Contains the state of one shot keys that are currently active.
 pub struct OneShotState {
- /// Coordinates of one shot keys that are active
+ /// KCoordinates of one shot keys that are active
  pub keys: ArrayDeque<OneShotKeys, arraydeque::behavior::Wrapping>,
- /// Coordinates of one shot keys that have been released
+ /// KCoordinates of one shot keys that have been released
  pub released_keys: ArrayDeque<OneShotKeys, arraydeque::behavior::Wrapping>,
  /// Used to keep track of already-pressed keys for the release variants.
  pub other_pressed_keys: ArrayDeque<OneShotKeys, arraydeque::behavior::Wrapping>,
@@ -664,8 +690,8 @@ pub struct OneShotState {
 
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 enum OneShotHandlePressKey {
- OneShotKey((u8, u16)),
- Other((u8, u16)),
+ OneShotKey(KCoord),
+ Other(KCoord),
 }
 
 impl OneShotState {
@@ -717,7 +743,7 @@ impl OneShotState {
  /// Returns true if the caller should handle the release normally and false otherwise.
  /// The second value in the tuple represents an overflow of released one shot keys and should
  /// be released is it is `Some`.
- fn handle_release(&mut self, (i, j): (u8, u16)) -> (bool, Option<(u8, u16)>) {
+ fn handle_release(&mut self, (i, j): KCoord) -> (bool, Option<KCoord>) {
  if self.keys.is_empty() {
  return (true, None);
  }
@@ -777,7 +803,7 @@ impl Queued {
 
 #[derive(Default)]
 pub struct LastPressTracker {
- pub coord: (u8, u16),
+ pub coord: KCoord,
  pub tap_hold_timeout: u16,
 }
 
@@ -834,7 +860,7 @@ impl<'a, const C: usize, const R: usize, const L: usize, T: 'a + Copy + std::fmt
  CustomEvent::NoEvent
  }
  }
- fn waiting_into_tap(&mut self) -> CustomEvent<'a, T> {
+ fn waiting_into_tap(&mut self, pq: Option<PressedQueue>) -> CustomEvent<'a, T> {
  if let Some(w) = &self.waiting {
  let tap = w.tap;
  let coord = w.coord;
@@ -843,7 +869,26 @@ impl<'a, const C: usize, const R: usize, const L: usize, T: 'a + Copy + std::fmt
  WaitingConfig::TapDance(_) => 0,
  };
  self.waiting = None;
- self.do_action(tap, coord, delay, false)
+ let ret = self.do_action(tap, coord, delay, false);
+ if let Some(pq) = pq {
+ if matches!(
+ tap,
+ Action::KeyCode(_)
+ | Action::MultipleKeyCodes(_)
+ | Action::OneShot(_)
+ | Action::Layer(_)
+ ) {
+ // The current intent of this block is to ensure that simple actions like
+ // key presses or layer-while-held remain pressed as long as a single key from
+ // the input chord remains held. The behaviour of these actions is correct in
+ // the case of repeating do_action, so there is currently no harm in doing
+ // this. Other action types are more problematic though.
+ for other_coord in pq.iter().copied() {
+ self.do_action(tap, other_coord, delay, false);
+ }
+ }
+ }
+ ret
  } else {
  CustomEvent::NoEvent
  }
@@ -904,10 +949,10 @@ impl<'a, const C: usize, const R: usize, const L: usize, T: 'a + Copy + std::fmt
 
  custom.update(match &mut self.waiting {
  Some(w) => match w.tick(&mut self.queue, &mut self.action_queue) {
- Some(WaitingAction::Hold) => self.waiting_into_hold(),
- Some(WaitingAction::Tap) => self.waiting_into_tap(),
- Some(WaitingAction::Timeout) => self.waiting_into_timeout(),
- Some(WaitingAction::NoOp) => self.drop_waiting(),
+ Some((WaitingAction::Hold, _)) => self.waiting_into_hold(),
+ Some((WaitingAction::Tap, pq)) => self.waiting_into_tap(pq),
+ Some((WaitingAction::Timeout, _)) => self.waiting_into_timeout(),
+ Some((WaitingAction::NoOp, _)) => self.drop_waiting(),
  None => CustomEvent::NoEvent,
  },
  None => match self.queue.pop_front() {
@@ -1086,7 +1131,7 @@ impl<'a, const C: usize, const R: usize, const L: usize, T: 'a + Copy + std::fmt
  self.dequeue(queued);
  }
  }
- fn press_as_action(&self, coord: (u8, u16), layer: usize) -> &'a Action<'a, T> {
+ fn press_as_action(&self, coord: KCoord, layer: usize) -> &'a Action<'a, T> {
  use crate::action::Action::*;
  let action = self
  .layers
@@ -1108,7 +1153,7 @@ impl<'a, const C: usize, const R: usize, const L: usize, T: 'a + Copy + std::fmt
  fn do_action(
  &mut self,
  action: &'a Action<'a, T>,
- coord: (u8, u16),
+ coord: KCoord,
  delay: u16,
  is_oneshot: bool,
  ) -> CustomEvent<'a, T> {
@@ -2941,8 +2986,10 @@ mod test {
  assert_keys(&[Kb5], layout.keycodes());
  layout.event(Release(0, 2));
  assert_eq!(CustomEvent::NoEvent, layout.tick());
- assert_keys(&[], layout.keycodes());
+ assert_keys(&[Kb5], layout.keycodes());
  layout.event(Release(0, 3));
+ assert_eq!(CustomEvent::NoEvent, layout.tick());
+ assert_keys(&[], layout.keycodes());
 
  // timeout on terminal chord with no action associated
  // combo like (h j k) -> (h j) (k)

src/cfg/mod.rs

@@ -1579,7 +1579,7 @@ fn parse_one_shot(
  action,
  Action::Layer(..) | Action::KeyCode(..) | Action::MultipleKeyCodes(..)
  ) {
- bail!("one-shot is only allowed to contain layer-toggle, a keycode, or a chord");
+ bail!("one-shot is only allowed to contain layer-while-held, a keycode, or a chord");
  }
 
  Ok(s.a.sref(Action::OneShot(s.a.sref(OneShot {