From 6a0d90f81a9b9938591b32f9321530e68e5cea0b Mon Sep 17 00:00:00 2001
From: Pascal Getreuer <50221757+getreuer@users.noreply.github.com>
Date: Sat, 13 Aug 2022 06:48:51 -0700
Subject: Fix Caps Word capitalization when used with Combos + Auto Shift.
(#17549)
---
quantum/process_keycode/process_auto_shift.c | 7 +-
quantum/process_keycode/process_caps_word.c | 4 +
quantum/process_keycode/process_combo.c | 9 +
.../test_caps_word_autoshift.cpp | 64 +++++--
tests/caps_word/caps_word_combo/config.h | 20 ++
tests/caps_word/caps_word_combo/test.mk | 19 ++
.../caps_word_combo/test_caps_word_combo.cpp | 212 +++++++++++++++++++++
tests/test_common/test_fixture.cpp | 16 ++
tests/test_common/test_fixture.hpp | 7 +
9 files changed, 343 insertions(+), 15 deletions(-)
create mode 100644 tests/caps_word/caps_word_combo/config.h
create mode 100644 tests/caps_word/caps_word_combo/test.mk
create mode 100644 tests/caps_word/caps_word_combo/test_caps_word_combo.cpp
diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c
index 8cb45bc0ae..3ff188ba7e 100644
--- a/quantum/process_keycode/process_auto_shift.c
+++ b/quantum/process_keycode/process_auto_shift.c
@@ -123,7 +123,12 @@ bool get_autoshift_shift_state(uint16_t keycode) {
/** \brief Restores the shift key if it was cancelled by Auto Shift */
static void autoshift_flush_shift(void) {
autoshift_flags.holding_shift = false;
- del_weak_mods(MOD_BIT(KC_LSFT));
+# ifdef CAPS_WORD_ENABLE
+ if (!is_caps_word_on())
+# endif // CAPS_WORD_ENABLE
+ {
+ del_weak_mods(MOD_BIT(KC_LSFT));
+ }
if (autoshift_flags.cancelling_lshift) {
autoshift_flags.cancelling_lshift = false;
add_mods(MOD_BIT(KC_LSFT));
diff --git a/quantum/process_keycode/process_caps_word.c b/quantum/process_keycode/process_caps_word.c
index ffd509a914..bc4369846c 100644
--- a/quantum/process_keycode/process_caps_word.c
+++ b/quantum/process_keycode/process_caps_word.c
@@ -131,7 +131,11 @@ bool process_caps_word(uint16_t keycode, keyrecord_t* record) {
#endif // SWAP_HANDS_ENABLE
}
+#ifdef AUTO_SHIFT_ENABLE
+ del_weak_mods(get_autoshift_state() ? ~MOD_BIT(KC_LSFT) : 0xff);
+#else
clear_weak_mods();
+#endif // AUTO_SHIFT_ENABLE
if (caps_word_press_user(keycode)) {
send_keyboard_report();
return true;
diff --git a/quantum/process_keycode/process_combo.c b/quantum/process_keycode/process_combo.c
index d5a649adb3..e5135e5a64 100644
--- a/quantum/process_keycode/process_combo.c
+++ b/quantum/process_keycode/process_combo.c
@@ -227,7 +227,16 @@ static inline void dump_key_buffer(void) {
#endif
}
record->event.time = 0;
+
+#if defined(CAPS_WORD_ENABLE) && defined(AUTO_SHIFT_ENABLE)
+ // Edge case: preserve the weak Left Shift mod if both Caps Word and
+ // Auto Shift are on. Caps Word capitalizes by setting the weak Left
+ // Shift mod during the press event, but Auto Shift doesn't send the
+ // key until it receives the release event.
+ del_weak_mods((is_caps_word_on() && get_autoshift_state()) ? ~MOD_BIT(KC_LSFT) : 0xff);
+#else
clear_weak_mods();
+#endif // defined(CAPS_WORD_ENABLE) && defined(AUTO_SHIFT_ENABLE)
#if TAP_CODE_DELAY > 0
// only delay once and for a non-tapping key
diff --git a/tests/caps_word/caps_word_autoshift/test_caps_word_autoshift.cpp b/tests/caps_word/caps_word_autoshift/test_caps_word_autoshift.cpp
index deb4d95766..ba21c527a6 100644
--- a/tests/caps_word/caps_word_autoshift/test_caps_word_autoshift.cpp
+++ b/tests/caps_word/caps_word_autoshift/test_caps_word_autoshift.cpp
@@ -19,6 +19,14 @@
#include "test_fixture.hpp"
#include "test_keymap_key.hpp"
+// Allow reports with no keys or only KC_LSFT.
+// clang-format off
+#define EXPECT_EMPTY_OR_LSFT(driver) \
+ EXPECT_CALL(driver, send_keyboard_mock(AnyOf( \
+ KeyboardReport(), \
+ KeyboardReport(KC_LSFT))))
+// clang-format on
+
using ::testing::_;
using ::testing::AnyNumber;
using ::testing::AnyOf;
@@ -39,13 +47,7 @@ TEST_F(CapsWord, AutoShiftKeys) {
KeymapKey key_spc(0, 1, 0, KC_SPC);
set_keymap({key_a, key_spc});
- // Allow any number of reports with no keys or only KC_LSFT.
- // clang-format off
- EXPECT_CALL(driver, send_keyboard_mock(AnyOf(
- KeyboardReport(),
- KeyboardReport(KC_LSFT))))
- .Times(AnyNumber());
- // clang-format on
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
{ // Expect: "A, A, space, a".
InSequence s;
EXPECT_REPORT(driver, (KC_LSFT, KC_A));
@@ -65,6 +67,46 @@ TEST_F(CapsWord, AutoShiftKeys) {
testing::Mock::VerifyAndClearExpectations(&driver);
}
+// Test Caps Word + Auto Shift where keys A and B are rolled.
+TEST_F(CapsWord, AutoShiftRolledShiftedKeys) {
+ TestDriver driver;
+ KeymapKey key_a(0, 0, 0, KC_A);
+ KeymapKey key_b(0, 0, 1, KC_B);
+ set_keymap({key_a, key_b});
+
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
+ { // Expect: "A, B, A, B".
+ InSequence s;
+ EXPECT_REPORT(driver, (KC_LSFT, KC_A));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_B));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_A));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_B));
+ }
+
+ caps_word_on();
+
+ key_a.press(); // Overlapping taps: A down, B down, A up, B up.
+ run_one_scan_loop();
+ key_b.press();
+ run_one_scan_loop();
+ key_a.release();
+ run_one_scan_loop();
+ key_b.release();
+ run_one_scan_loop();
+
+ key_a.press(); // Nested taps: A down, B down, B up, A up.
+ run_one_scan_loop();
+ key_b.press();
+ run_one_scan_loop();
+ key_b.release();
+ run_one_scan_loop();
+ key_a.release();
+ run_one_scan_loop();
+
+ caps_word_off();
+ testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
// Tests that with tap-hold keys with Retro Shift, letter keys are shifted by
// Caps Word regardless of whether they are retroshifted.
TEST_F(CapsWord, RetroShiftKeys) {
@@ -73,13 +115,7 @@ TEST_F(CapsWord, RetroShiftKeys) {
KeymapKey key_layertap_b(0, 1, 0, LT(1, KC_B));
set_keymap({key_modtap_a, key_layertap_b});
- // Allow any number of reports with no keys or only KC_LSFT.
- // clang-format off
- EXPECT_CALL(driver, send_keyboard_mock(AnyOf(
- KeyboardReport(),
- KeyboardReport(KC_LSFT))))
- .Times(AnyNumber());
- // clang-format on
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
{ // Expect: "B, A, B, A".
InSequence s;
EXPECT_REPORT(driver, (KC_LSFT, KC_B));
diff --git a/tests/caps_word/caps_word_combo/config.h b/tests/caps_word/caps_word_combo/config.h
new file mode 100644
index 0000000000..92dbe045b2
--- /dev/null
+++ b/tests/caps_word/caps_word_combo/config.h
@@ -0,0 +1,20 @@
+// Copyright 2022 Google LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see .
+
+#pragma once
+
+#include "test_common.h"
+
+#define TAPPING_TERM 200
diff --git a/tests/caps_word/caps_word_combo/test.mk b/tests/caps_word/caps_word_combo/test.mk
new file mode 100644
index 0000000000..9f2e157189
--- /dev/null
+++ b/tests/caps_word/caps_word_combo/test.mk
@@ -0,0 +1,19 @@
+# Copyright 2022 Google LLC
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see .
+
+CAPS_WORD_ENABLE = yes
+COMBO_ENABLE = yes
+AUTO_SHIFT_ENABLE = yes
+
diff --git a/tests/caps_word/caps_word_combo/test_caps_word_combo.cpp b/tests/caps_word/caps_word_combo/test_caps_word_combo.cpp
new file mode 100644
index 0000000000..3a0530b854
--- /dev/null
+++ b/tests/caps_word/caps_word_combo/test_caps_word_combo.cpp
@@ -0,0 +1,212 @@
+// Copyright 2022 Google LLC
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 2 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see .
+
+// Test Caps Word + Combos, with and without Auto Shift.
+
+#include
+#include
+#include
+
+#include "keyboard_report_util.hpp"
+#include "keycode.h"
+#include "test_common.hpp"
+#include "test_fixture.hpp"
+#include "test_keymap_key.hpp"
+
+// Allow reports with no keys or only KC_LSFT.
+// clang-format off
+#define EXPECT_EMPTY_OR_LSFT(driver) \
+ EXPECT_CALL(driver, send_keyboard_mock(AnyOf( \
+ KeyboardReport(), \
+ KeyboardReport(KC_LSFT))))
+// clang-format on
+
+using ::testing::AnyNumber;
+using ::testing::AnyOf;
+using ::testing::InSequence;
+using ::testing::TestParamInfo;
+
+extern "C" {
+// Define some combos to use for the test, including overlapping combos and
+// combos that chord tap-hold keys.
+enum combo_events { AB_COMBO, BC_COMBO, AD_COMBO, DE_COMBO, FGHI_COMBO, COMBO_LENGTH };
+uint16_t COMBO_LEN = COMBO_LENGTH;
+
+const uint16_t ab_combo[] PROGMEM = {KC_A, KC_B, COMBO_END};
+const uint16_t bc_combo[] PROGMEM = {KC_B, KC_C, COMBO_END};
+const uint16_t ad_combo[] PROGMEM = {KC_A, LCTL_T(KC_D), COMBO_END};
+const uint16_t de_combo[] PROGMEM = {LCTL_T(KC_D), LT(1, KC_E), COMBO_END};
+const uint16_t fghi_combo[] PROGMEM = {KC_F, KC_G, KC_H, KC_I, COMBO_END};
+
+// clang-format off
+combo_t key_combos[] = {
+ [AB_COMBO] = COMBO(ab_combo, KC_SPC), // KC_A + KC_B = KC_SPC
+ [BC_COMBO] = COMBO(bc_combo, KC_X), // KC_B + KC_C = KC_X
+ [AD_COMBO] = COMBO(ad_combo, KC_Y), // KC_A + LCTL_T(KC_D) = KC_Y
+ [DE_COMBO] = COMBO(de_combo, KC_Z), // LCTL_T(KC_D) + LT(1, KC_E) = KC_Z
+ [FGHI_COMBO] = COMBO(fghi_combo, KC_W) // KC_F + KC_G + KC_H + KC_I = KC_W
+};
+// clang-format on
+} // extern "C"
+
+namespace {
+
+// To test combos thorougly, we test them with pressing the chord keys with
+// a few different orders and timings.
+struct TestParams {
+ std::string name;
+ bool autoshift_on;
+
+ static const std::string& GetName(const TestParamInfo& info) {
+ return info.param.name;
+ }
+};
+
+class CapsWord : public ::testing::WithParamInterface, public TestFixture {
+ public:
+ void SetUp() override {
+ caps_word_off();
+ if (GetParam().autoshift_on) {
+ autoshift_enable();
+ } else {
+ autoshift_disable();
+ }
+ }
+};
+
+// Test pressing the keys in a combo with different orders and timings.
+TEST_P(CapsWord, SingleCombo) {
+ TestDriver driver;
+ KeymapKey key_b(0, 0, 1, KC_B);
+ KeymapKey key_c(0, 0, 2, KC_C);
+ set_keymap({key_b, key_c});
+
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
+ EXPECT_REPORT(driver, (KC_LSFT, KC_X));
+
+ caps_word_on();
+ tap_combo({key_b, key_c});
+
+ EXPECT_TRUE(is_caps_word_on());
+ caps_word_off();
+
+ testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
+// Test a longer 4-key combo.
+TEST_P(CapsWord, LongerCombo) {
+ TestDriver driver;
+ KeymapKey key_f(0, 0, 0, KC_F);
+ KeymapKey key_g(0, 0, 1, KC_G);
+ KeymapKey key_h(0, 0, 2, KC_H);
+ KeymapKey key_i(0, 0, 3, KC_I);
+ set_keymap({key_f, key_g, key_h, key_i});
+
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
+ EXPECT_REPORT(driver, (KC_LSFT, KC_W));
+
+ caps_word_on();
+ tap_combo({key_f, key_g, key_h, key_i});
+
+ EXPECT_TRUE(is_caps_word_on());
+ caps_word_off();
+
+ testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
+// Test with two overlapping combos on regular keys:
+// KC_A + KC_B = KC_SPC,
+// KC_B + KC_C = KC_X.
+TEST_P(CapsWord, ComboRegularKeys) {
+ TestDriver driver;
+ KeymapKey key_a(0, 0, 0, KC_A);
+ KeymapKey key_b(0, 0, 1, KC_B);
+ KeymapKey key_c(0, 0, 2, KC_C);
+ KeymapKey key_1(0, 0, 3, KC_1);
+ set_keymap({key_a, key_b, key_c, key_1});
+
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
+ { // Expect: "A, B, 1, X, 1, C, space, a".
+ InSequence s;
+ EXPECT_REPORT(driver, (KC_LSFT, KC_A));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_B));
+ EXPECT_REPORT(driver, (KC_1));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_X));
+ EXPECT_REPORT(driver, (KC_1));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_C));
+ EXPECT_REPORT(driver, (KC_SPC));
+ EXPECT_REPORT(driver, (KC_A));
+ }
+
+ caps_word_on();
+ tap_key(key_a);
+ tap_key(key_b);
+ tap_key(key_1);
+ tap_combo({key_b, key_c}); // BC combo types "x".
+ tap_key(key_1);
+ tap_key(key_c);
+ tap_combo({key_a, key_b}); // AB combo types space.
+ tap_key(key_a);
+
+ EXPECT_FALSE(is_caps_word_on());
+ testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
+// Test where combo chords involve tap-hold keys:
+// KC_A + LCTL_T(KC_D) = KC_Y,
+// LCTL_T(KC_D) + LT(1, KC_E) = KC_Z,
+TEST_P(CapsWord, ComboModTapKey) {
+ TestDriver driver;
+ KeymapKey key_a(0, 0, 0, KC_A);
+ KeymapKey key_modtap_d(0, 0, 1, LCTL_T(KC_D));
+ KeymapKey key_layertap_e(0, 0, 2, LT(1, KC_E));
+ set_keymap({key_a, key_modtap_d, key_layertap_e});
+
+ EXPECT_EMPTY_OR_LSFT(driver).Times(AnyNumber());
+ { // Expect: "A, D, E, Y, Z".
+ InSequence s;
+ EXPECT_REPORT(driver, (KC_LSFT, KC_A));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_D));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_E));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_Y));
+ EXPECT_REPORT(driver, (KC_LSFT, KC_Z));
+ }
+
+ caps_word_on();
+ tap_key(key_a);
+ tap_key(key_modtap_d);
+ tap_key(key_layertap_e);
+ tap_combo({key_a, key_modtap_d}); // AD combo types "y".
+ tap_combo({key_modtap_d, key_layertap_e}); // DE combo types "z".
+
+ EXPECT_TRUE(is_caps_word_on());
+ caps_word_off();
+
+ testing::Mock::VerifyAndClearExpectations(&driver);
+}
+
+// clang-format off
+INSTANTIATE_TEST_CASE_P(
+ Combos,
+ CapsWord,
+ ::testing::Values(
+ TestParams{"AutoshiftDisabled", false},
+ TestParams{"AutoshiftEnabled", true}
+ ),
+ TestParams::GetName
+ );
+// clang-format on
+
+} // namespace
diff --git a/tests/test_common/test_fixture.cpp b/tests/test_common/test_fixture.cpp
index 5fc6964054..44694cd390 100644
--- a/tests/test_common/test_fixture.cpp
+++ b/tests/test_common/test_fixture.cpp
@@ -108,6 +108,22 @@ void TestFixture::tap_key(KeymapKey key, unsigned delay_ms) {
run_one_scan_loop();
}
+void TestFixture::tap_combo(const std::vector& chord_keys, unsigned delay_ms) {
+ for (KeymapKey key : chord_keys) { // Press each key.
+ key.press();
+ run_one_scan_loop();
+ }
+
+ if (delay_ms > 1) {
+ idle_for(delay_ms - 1);
+ }
+
+ for (KeymapKey key : chord_keys) { // Release each key.
+ key.release();
+ run_one_scan_loop();
+ }
+}
+
void TestFixture::set_keymap(std::initializer_list keys) {
this->keymap.clear();
for (auto& key : keys) {
diff --git a/tests/test_common/test_fixture.hpp b/tests/test_common/test_fixture.hpp
index 81906f76c7..2590acd006 100644
--- a/tests/test_common/test_fixture.hpp
+++ b/tests/test_common/test_fixture.hpp
@@ -53,6 +53,13 @@ class TestFixture : public testing::Test {
}
}
+ /**
+ * @brief Taps a combo with `delay_ms` delay between press and release.
+ *
+ * Example: `tap_combo({key_a, key_b})` to tap the chord A + B.
+ */
+ void tap_combo(const std::vector& chord_keys, unsigned delay_ms = 1);
+
void run_one_scan_loop();
void idle_for(unsigned ms);
--
cgit v1.2.1