Improve test invocation, fix Retro Shift bugs, and add Auto+Retro Shift test cases (#15889)

4 files changed, 58 insertions, 52 deletions

diff --git a/quantum/action.c b/quantum/action.c
index 6368f7398c..3b89431cea 100644
--- a/quantum/action.c
+++ b/quantum/action.c
@@ -497,7 +497,7 @@ void process_action(keyrecord_t *record, action_t action) {
                 default:
                     if (event.pressed) {
                         if (tap_count > 0) {
-#    ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
+#    ifdef HOLD_ON_OTHER_KEY_PRESS
                             if (
 #        ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
                                 get_hold_on_other_key_press(get_event_keycode(record->event, false), record) &&
diff --git a/quantum/action_tapping.c b/quantum/action_tapping.c
index f94e5e6f69..8f238490f2 100644
--- a/quantum/action_tapping.c
+++ b/quantum/action_tapping.c
@@ -116,25 +116,26 @@ void action_tapping_process(keyrecord_t record) {
  * readable. The conditional definition of tapping_keycode and all the
  * conditional uses of it are hidden inside macros named TAP_...
  */
-#    if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)) || defined(PERMISSIVE_HOLD_PER_KEY) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
-#        define TAP_DEFINE_KEYCODE const uint16_t tapping_keycode = get_record_keycode(&tapping_key, false)
-#    else
-#        define TAP_DEFINE_KEYCODE
-#    endif
+#    define TAP_DEFINE_KEYCODE const uint16_t tapping_keycode = get_record_keycode(&tapping_key, false)
 
 #    if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
 #        ifdef RETRO_TAPPING_PER_KEY
-#            define TAP_GET_RETRO_TAPPING get_retro_tapping(tapping_keycode, &tapping_key)
+#            define TAP_GET_RETRO_TAPPING(keyp) get_auto_shifted_key(tapping_keycode, keyp) && get_retro_tapping(tapping_keycode, &tapping_key)
 #        else
-#            define TAP_GET_RETRO_TAPPING true
+#            define TAP_GET_RETRO_TAPPING(keyp) get_auto_shifted_key(tapping_keycode, keyp)
 #        endif
-#        define MAYBE_RETRO_SHIFTING(ev) (TAP_GET_RETRO_TAPPING && (RETRO_SHIFT + 0) != 0 && TIMER_DIFF_16((ev).time, tapping_key.event.time) < (RETRO_SHIFT + 0))
+/* Used to extend TAPPING_TERM:
+ *     indefinitely if RETRO_SHIFT does not have a value
+ *     to RETRO_SHIFT if RETRO_SHIFT is set
+ * for possibly retro shifted keys.
+ */
+#        define MAYBE_RETRO_SHIFTING(ev, keyp) (get_auto_shifted_key(tapping_keycode, keyp) && TAP_GET_RETRO_TAPPING(keyp) && ((RETRO_SHIFT + 0) == 0 || TIMER_DIFF_16((ev).time, tapping_key.event.time) < (RETRO_SHIFT + 0)))
 #        define TAP_IS_LT IS_QK_LAYER_TAP(tapping_keycode)
 #        define TAP_IS_MT IS_QK_MOD_TAP(tapping_keycode)
 #        define TAP_IS_RETRO IS_RETRO(tapping_keycode)
 #    else
-#        define TAP_GET_RETRO_TAPPING false
-#        define MAYBE_RETRO_SHIFTING(ev) false
+#        define TAP_GET_RETRO_TAPPING(keyp) false
+#        define MAYBE_RETRO_SHIFTING(ev, kp) false
 #        define TAP_IS_LT false
 #        define TAP_IS_MT false
 #        define TAP_IS_RETRO false
@@ -187,20 +188,19 @@ bool process_tapping(keyrecord_t *keyp) {
         return true;
     }
 
+#    if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)) || defined(PERMISSIVE_HOLD_PER_KEY) || defined(HOLD_ON_OTHER_KEY_PRESS_PER_KEY)
     TAP_DEFINE_KEYCODE;
+#    endif
 
     // process "pressed" tapping key state
     if (tapping_key.event.pressed) {
-        if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event)) {
+        if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event, keyp)) {
             if (IS_NOEVENT(event)) {
                 // early return for tick events
                 return true;
             }
             if (tapping_key.tap.count == 0) {
                 if (IS_TAPPING_RECORD(keyp) && !event.pressed) {
-#    if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
-                    retroshift_swap_times();
-#    endif
                     // first tap!
                     ac_dprintf("Tapping: First tap(0->1).\n");
                     tapping_key.tap.count = 1;
@@ -218,28 +218,12 @@ bool process_tapping(keyrecord_t *keyp) {
                  */
                 // clang-format off
                 else if (
+                    !event.pressed && waiting_buffer_typed(event) &&
                     (
-                        !event.pressed && waiting_buffer_typed(event) &&
-                        TAP_GET_PERMISSIVE_HOLD
-                    )
-                    // Causes nested taps to not wait past TAPPING_TERM/RETRO_SHIFT
-                    // unnecessarily and fixes them for Layer Taps.
-                    || (TAP_GET_RETRO_TAPPING &&
-                        (
-                            // Rolled over the two keys.
-                            (tapping_key.tap.interrupted == true && (
-                                (TAP_IS_LT && TAP_GET_HOLD_ON_OTHER_KEY_PRESS) ||
-                                (TAP_IS_MT && TAP_GET_HOLD_ON_OTHER_KEY_PRESS)
-                                )
-                            )
-                            // Makes Retro Shift ignore the default behavior of
-                            // MTs and LTs on nested taps below TAPPING_TERM or RETRO_SHIFT
-                            || (
-                                TAP_IS_RETRO
-                                && (event.key.col != tapping_key.event.key.col || event.key.row != tapping_key.event.key.row)
-                                && !event.pressed && waiting_buffer_typed(event)
-                            )
-                        )
+                        TAP_GET_PERMISSIVE_HOLD ||
+                        // Causes nested taps to not wait past TAPPING_TERM/RETRO_SHIFT
+                        // unnecessarily and fixes them for Layer Taps.
+                        TAP_GET_RETRO_TAPPING(keyp)
                     )
                 ) {
                     // clang-format on
@@ -284,10 +268,16 @@ bool process_tapping(keyrecord_t *keyp) {
                     process_record(keyp);
                     return true;
                 } else {
-                    // set interrupted flag when other key preesed during tapping
+                    // set interrupted flag when other key pressed during tapping
                     if (event.pressed) {
                         tapping_key.tap.interrupted = true;
-                        if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS) {
+                        if (TAP_GET_HOLD_ON_OTHER_KEY_PRESS
+#    if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
+                            // Auto Shift cannot evaluate this early
+                            // Retro Shift uses the hold action for all nested taps even without HOLD_ON_OTHER_KEY_PRESS, so this is fine to skip
+                            && !(MAYBE_RETRO_SHIFTING(event, keyp) && get_auto_shifted_key(get_record_keycode(keyp, false), keyp))
+#    endif
+                        ) {
                             ac_dprintf("Tapping: End. No tap. Interfered by pressed key\n");
                             process_record(&tapping_key);
                             tapping_key = (keyrecord_t){0};
@@ -332,6 +322,9 @@ bool process_tapping(keyrecord_t *keyp) {
                     return true;
                 } else {
                     ac_dprintf("Tapping: key event while last tap(>0).\n");
+#    if defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT)
+                    retroshift_swap_times();
+#    endif
                     process_record(keyp);
                     return true;
                 }
@@ -388,7 +381,7 @@ bool process_tapping(keyrecord_t *keyp) {
     }
     // process "released" tapping key state
     else {
-        if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event)) {
+        if (WITHIN_TAPPING_TERM(event) || MAYBE_RETRO_SHIFTING(event, keyp)) {
             if (IS_NOEVENT(event)) {
                 // early return for tick events
                 return true;
@@ -506,9 +499,16 @@ void waiting_buffer_scan_tap(void) {
         return;
     }
 
+#    if (defined(AUTO_SHIFT_ENABLE) && defined(RETRO_SHIFT))
+    TAP_DEFINE_KEYCODE;
+#    endif
     for (uint8_t i = waiting_buffer_tail; i != waiting_buffer_head; i = (i + 1) % WAITING_BUFFER_SIZE) {
         keyrecord_t *candidate = &waiting_buffer[i];
-        if (IS_EVENT(candidate->event) && KEYEQ(candidate->event.key, tapping_key.event.key) && !candidate->event.pressed && WITHIN_TAPPING_TERM(candidate->event)) {
+        // clang-format off
+        if (IS_EVENT(candidate->event) && KEYEQ(candidate->event.key, tapping_key.event.key) && !candidate->event.pressed && (
+            WITHIN_TAPPING_TERM(waiting_buffer[i].event) || MAYBE_RETRO_SHIFTING(waiting_buffer[i].event, &tapping_key)
+        )) {
+            // clang-format on
             tapping_key.tap.count = 1;
             candidate->tap.count  = 1;
             process_record(&tapping_key);
diff --git a/quantum/process_keycode/process_auto_shift.c b/quantum/process_keycode/process_auto_shift.c
index 9b78214e43..28a21c4b67 100644
--- a/quantum/process_keycode/process_auto_shift.c
+++ b/quantum/process_keycode/process_auto_shift.c
@@ -66,7 +66,7 @@ __attribute__((weak)) bool get_custom_auto_shifted_key(uint16_t keycode, keyreco
     return false;
 }
 
-/** \brief Called on physical press, returns whether is Auto Shift key */
+/** \brief Called on physical press, returns whether key is an Auto Shift key */
 __attribute__((weak)) bool get_auto_shifted_key(uint16_t keycode, keyrecord_t *record) {
     switch (keycode) {
 #ifndef NO_AUTO_SHIFT_ALPHA
@@ -178,9 +178,8 @@ static bool autoshift_press(uint16_t keycode, uint16_t now, keyrecord_t *record)
     }
 
     // Store record to be sent to user functions if there's no release record then.
-    autoshift_lastrecord               = *record;
-    autoshift_lastrecord.event.pressed = false;
-    autoshift_lastrecord.event.time    = 0;
+    autoshift_lastrecord            = *record;
+    autoshift_lastrecord.event.time = 0;
     // clang-format off
 #if defined(AUTO_SHIFT_REPEAT) || defined(AUTO_SHIFT_REPEAT_PER_KEY)
     if (keycode == autoshift_lastkey &&
@@ -409,8 +408,12 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
             // If Retro Shift is disabled, possible custom actions shouldn't happen.
             // clang-format off
 #if defined(RETRO_SHIFT) && !defined(NO_ACTION_TAPPING)
-#    ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
-            const bool is_hold_on_interrupt = get_hold_on_other_key_press(keycode, record);
+#    ifdef HOLD_ON_OTHER_KEY_PRESS
+            const bool is_hold_on_interrupt = (IS_QK_MOD_TAP(keycode)
+#        ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
+                && get_hold_on_other_key_press(keycode, record)
+#        endif
+            );
 #    else
             const bool is_hold_on_interrupt = false;
 #    endif
@@ -450,8 +453,12 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
 #endif
         ) {
             // Fixes modifiers not being applied to rolls with AUTO_SHIFT_MODIFIERS set.
-#ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
-            if (autoshift_flags.in_progress && get_hold_on_other_key_press(keycode, record)) {
+#ifdef HOLD_ON_OTHER_KEY_PRESS
+            if (autoshift_flags.in_progress
+#    ifdef HOLD_ON_OTHER_KEY_PRESS_PER_KEY
+                && get_hold_on_other_key_press(keycode, record)
+#    endif
+            ) {
                 autoshift_end(KC_NO, now, false, &autoshift_lastrecord);
             }
 #endif
@@ -488,10 +495,8 @@ void retroshift_poll_time(keyevent_t *event) {
 }
 // Used to swap the times of Retro Shifted key and Auto Shift key that interrupted it.
 void retroshift_swap_times(void) {
-    if (last_retroshift_time != 0 && autoshift_flags.in_progress) {
-        uint16_t temp        = retroshift_time;
-        retroshift_time      = last_retroshift_time;
-        last_retroshift_time = temp;
+    if (autoshift_flags.in_progress) {
+        autoshift_time = last_retroshift_time;
     }
 }
 #endif
diff --git a/quantum/process_keycode/process_auto_shift.h b/quantum/process_keycode/process_auto_shift.h
index 885a47b533..1353548aa6 100644
--- a/quantum/process_keycode/process_auto_shift.h
+++ b/quantum/process_keycode/process_auto_shift.h
@@ -56,4 +56,5 @@ uint16_t (get_autoshift_timeout)(uint16_t keycode, keyrecord_t *record);
 void     set_autoshift_timeout(uint16_t timeout);
 void     autoshift_matrix_scan(void);
 bool     get_custom_auto_shifted_key(uint16_t keycode, keyrecord_t *record);
+bool     get_auto_shifted_key(uint16_t keycode, keyrecord_t *record);
 // clang-format on