Merge 975a4fff31 into 507c948ed8

docs/config_options.md

@@ -190,6 +190,8 @@ If you define these options you will enable the associated feature, which may in
   * how long for the Combo keys to be detected. Defaults to `TAPPING_TERM` if not defined.
 * `#define COMBO_MUST_HOLD_MODS`
   * Flag for enabling extending timeout on Combos containing modifiers
+* `#define COMBO_HOLD_STRICT`
+  * Flag for requiring a combo's term to elapse without any other keys being pressed.
 * `#define COMBO_MOD_TERM 200`
   * Allows for extending COMBO_TERM for mod keys while mid-combo.
 * `#define COMBO_MUST_HOLD_PER_COMBO`

docs/features/combo.md

@@ -140,6 +140,8 @@ Processing combos has two buffers, one for the key presses, another for the comb
 ### Modifier Combos
 If a combo resolves to a Modifier, the window for processing the combo can be extended independently from normal combos. By default, this is disabled but can be enabled with `#define COMBO_MUST_HOLD_MODS`, and the time window can be configured with `#define COMBO_HOLD_TERM 150` (default: `TAPPING_TERM`). With `COMBO_MUST_HOLD_MODS`, you cannot tap the combo any more which makes the combo less prone to misfires.
 
+By default, `COMBO_MUST_HOLD_MODS` ignores the `COMBO_HOLD_TERM` and fires the combo if another key is pressed before the term elapses. You can alter this behavior with `#define COMBO_HOLD_STRICT`. With `COMBO_HOLD_STRICT`, the combo will be discarded if another key is pressed before the term elapses. (For example, imagine you have a combo that makes `a`+`b` trigger `ctrl`. You press `a` and `b` together and then press `c` before `COMBO_HOLD_TERM` has elapsed. Without `COMBO_HOLD_STRICT`, you would get `ctrl`+`c`. With `COMBO_HOLD_STRICT`, you would get `abc`.)
+
 ### Strict key press order
 By defining `COMBO_MUST_PRESS_IN_ORDER` combos only activate when the keys are pressed in the same order as they are defined in the key array.
 

quantum/process_keycode/process_combo.c

@@ -357,7 +357,7 @@ void apply_combo(uint16_t combo_index, combo_t *combo) {
     drop_combo_from_buffer(combo_index);
 }
 
-static inline void apply_combos(void) {
+static inline void apply_combos(bool triggered_by_timer) {
     // Apply all buffered normal combos.
     for (uint8_t i = combo_buffer_read; i != combo_buffer_write; INCREMENT_MOD(i)) {
         queued_combo_t *buffered_combo = &combo_buffer[i];
@@ -369,6 +369,13 @@ static inline void apply_combos(void) {
             drop_combo_from_buffer(buffered_combo->combo_index);
             continue;
         }
+#endif
+#ifdef COMBO_HOLD_STRICT
+        if (!triggered_by_timer && _get_combo_must_hold(buffered_combo->combo_index, combo)) {
+            // When strict, hold-only combos are applied only if triggered by timer
+            drop_combo_from_buffer(buffered_combo->combo_index);
+            continue;
+        }
 #endif
         apply_combo(buffered_combo->combo_index, combo);
     }
@@ -515,7 +522,7 @@ static combo_key_action_t process_single_combo(combo_t *combo, uint16_t keycode,
             else if (get_combo_must_tap(combo_index, combo)) {
                 // immediately apply tap-only combo
                 apply_combo(combo_index, combo);
-                apply_combos(); // also apply other prepared combos and dump key buffer
+                apply_combos(false); // also apply other prepared combos and dump key buffer
 #    ifdef COMBO_PROCESS_KEY_RELEASE
                 if (process_combo_key_release(combo_index, combo, key_index, keycode)) {
                     release_combo(combo_index, combo);
@@ -614,7 +621,7 @@ bool process_combo(uint16_t keycode, keyrecord_t *record) {
     } else {
         if (combo_buffer_read != combo_buffer_write) {
             // some combo is prepared
-            apply_combos();
+            apply_combos(false);
         } else {
             // reset state if there are no combo keys pressed at all
             dump_key_buffer();
@@ -635,7 +642,7 @@ void combo_task(void) {
 #ifndef COMBO_NO_TIMER
     if (timer && timer_elapsed(timer) > longest_term) {
         if (combo_buffer_read != combo_buffer_write) {
-            apply_combos();
+            apply_combos(true);
             longest_term = 0;
             timer        = 0;
         } else {

tests/combo/combo_hold_strict/config.h

@@ -0,0 +1,10 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#include "test_common.h"
+
+#define COMBO_MUST_HOLD_MODS
+#define COMBO_HOLD_TERM 150
+#define COMBO_HOLD_STRICT

tests/combo/combo_hold_strict/test.mk

@@ -0,0 +1,6 @@
+# Copyright 2024 @Filios92
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+COMBO_ENABLE = yes
+
+INTROSPECTION_KEYMAP_C = test_combo_hold_strict.c

tests/combo/combo_hold_strict/test_combo.cpp

@@ -0,0 +1,73 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "gmock/gmock.h"
+#include "keyboard_report_util.hpp"
+#include "keycode.h"
+#include "test_common.h"
+#include "test_common.hpp"
+#include "test_driver.hpp"
+#include "test_fixture.hpp"
+#include "test_keymap_key.hpp"
+
+using testing::_;
+using testing::InSequence;
+
+class ComboHoldStrict : public TestFixture {};
+
+TEST_F(ComboHoldStrict, combo_hold_strict_held_full_term) {
+    TestDriver driver;
+    KeymapKey  key_h(0, 0, 0, KC_H);
+    KeymapKey  key_j(0, 0, 1, KC_J);
+    set_keymap({key_h, key_j});
+
+    EXPECT_REPORT(driver, (KC_LCTL));
+    key_h.press();
+    run_one_scan_loop();
+    key_j.press();
+    run_one_scan_loop();
+    idle_for(COMBO_HOLD_TERM);
+    combo_task();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ComboHoldStrict, combo_hold_strict_tap_other_key_before_term) {
+    TestDriver driver;
+    KeymapKey  key_h(0, 0, 0, KC_H);
+    KeymapKey  key_j(0, 0, 1, KC_J);
+    KeymapKey  key_f(0, 0, 2, KC_F);
+    set_keymap({key_h, key_j, key_f});
+
+    EXPECT_REPORT(driver, (KC_H));
+    EXPECT_REPORT(driver, (KC_H, KC_J));
+    EXPECT_REPORT(driver, (KC_H, KC_J, KC_F));
+    key_h.press();
+    run_one_scan_loop();
+    key_j.press();
+    run_one_scan_loop();
+    idle_for(COMBO_HOLD_TERM / 2);
+    combo_task();
+    key_f.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}
+
+TEST_F(ComboHoldStrict, combo_hold_strict_tap_other_key_after_term) {
+    TestDriver driver;
+    KeymapKey  key_h(0, 0, 0, KC_H);
+    KeymapKey  key_j(0, 0, 1, KC_J);
+    KeymapKey  key_f(0, 0, 2, KC_F);
+    set_keymap({key_h, key_j, key_f});
+
+    EXPECT_REPORT(driver, (KC_LCTL));
+    EXPECT_REPORT(driver, (KC_LCTL, KC_F));
+    key_h.press();
+    run_one_scan_loop();
+    key_j.press();
+    run_one_scan_loop();
+    idle_for(COMBO_HOLD_TERM);
+    combo_task();
+    key_f.press();
+    run_one_scan_loop();
+    VERIFY_AND_CLEAR(driver);
+}

tests/combo/combo_hold_strict/test_combo_hold_strict.c

@@ -0,0 +1,11 @@
+// Copyright 2024 @Filios92
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include "quantum.h"
+
+enum combos { ctrl };
+
+uint16_t const ctrl_combo[]    = {KC_H, KC_J, COMBO_END};
+
+combo_t key_combos[] = {
+    [ctrl]     = COMBO(ctrl_combo, KC_LCTL)
+};