Auto shift: support repeats and early registration (#9826)

Fixes #7048.

Co-authored-by: IsaacElenbaas <isaacelenbaas@gmail.com>

docs/feature_auto_shift.md

@@ -15,25 +15,31 @@ problem.
 When you tap a key, it stays depressed for a short period of time before it is
 then released. This depressed time is a different length for everyone. Auto Shift
 defines a constant `AUTO_SHIFT_TIMEOUT` which is typically set to twice your
-normal pressed state time. When you press a key, a timer starts and then stops
-when you release the key. If the time depressed is greater than or equal to the
-`AUTO_SHIFT_TIMEOUT`, then a shifted version of the key is emitted. If the time
-is less than the `AUTO_SHIFT_TIMEOUT` time, then the normal state is emitted.
+normal pressed state time. When you press a key, a timer starts, and if you
+have not released the key after the `AUTO_SHIFT_TIMEOUT` period, then a shifted
+version of the key is emitted. If the time is less than the `AUTO_SHIFT_TIMEOUT`
+time, or you press another key, then the normal state is emitted.
+
+If `AUTO_SHIFT_REPEAT` is defined, there is keyrepeat support. Holding the key
+down will repeat the shifted key, though this can be disabled with
+`AUTO_SHIFT_NO_AUTO_REPEAT`. If you want to repeat the normal key, then tap it
+once then immediately (within `TAPPING_TERM`) hold it down again (this works
+with the shifted value as well if auto-repeat is disabled).
 
 ## Are There Limitations to Auto Shift?
 
 Yes, unfortunately.
 
-1. Key repeat will cease to work. For example, before if you wanted 20 'a'
-   characters, you could press and hold the 'a' key for a second or two. This no
-   longer works with Auto Shift because it is timing your depressed time instead
-   of emitting a depressed key state to your operating system.
-2. You will have characters that are shifted when you did not intend on shifting, and
+You will have characters that are shifted when you did not intend on shifting, and
 other characters you wanted shifted, but were not. This simply comes down to
-   practice. As we get in a hurry, we think we have hit the key long enough
-   for a shifted version, but we did not. On the other hand, we may think we are
-   tapping the keys, but really we have held it for a little longer than
-   anticipated.
+practice. As we get in a hurry, we think we have hit the key long enough for a
+shifted version, but we did not. On the other hand, we may think we are tapping
+the keys, but really we have held it for a little longer than anticipated.
+
+Additionally, with keyrepeat the desired shift state can get mixed up. It will
+always 'belong' to the last key pressed. For example, keyrepeating a capital
+and then tapping something lowercase (whether or not it's an Auto Shift key)
+will result in the capital's *key* still being held, but shift not.
 
 ## How Do I Enable Auto Shift?
 
@@ -103,6 +109,14 @@ Do not Auto Shift numeric keys, zero through nine.
 
 Do not Auto Shift alpha characters, which include A through Z.
 
+### AUTO_SHIFT_REPEAT (simple define)
+
+Enables keyrepeat.
+
+### AUTO_SHIFT_NO_AUTO_REPEAT (simple define)
+
+Disables automatically keyrepeating when `AUTO_SHIFT_TIMEOUT` is exceeded.
+
 ## Using Auto Shift Setup
 
 This will enable you to define three keys temporarily to increase, decrease and report your `AUTO_SHIFT_TIMEOUT`.

quantum/process_keycode/process_auto_shift.c

@@ -16,48 +16,149 @@
 
 #ifdef AUTO_SHIFT_ENABLE
 
+#    include <stdbool.h>
 #    include <stdio.h>
 
 #    include "process_auto_shift.h"
 
-static bool     autoshift_enabled = true;
 static uint16_t autoshift_time    = 0;
 static uint16_t autoshift_timeout = AUTO_SHIFT_TIMEOUT;
 static uint16_t autoshift_lastkey = KC_NO;
+static struct {
+    // Whether autoshift is enabled.
+    bool enabled : 1;
+    // Whether the last auto-shifted key was released after the timeout.  This
+    // is used to replicate the last key for a tap-then-hold.
+    bool lastshifted : 1;
+    // Whether an auto-shiftable key has been pressed but not processed.
+    bool in_progress : 1;
+    // Whether the auto-shifted keypress has been registered.
+    bool holding_shift : 1;
+} autoshift_flags = {true, false, false, false};
 
-void autoshift_flush(void) {
-    if (autoshift_lastkey != KC_NO) {
-        uint16_t elapsed = timer_elapsed(autoshift_time);
+/** \brief Record the press of an autoshiftable key
+ *
+ *  \return Whether the record should be further processed.
+ */
+static bool autoshift_press(uint16_t keycode, uint16_t now, keyrecord_t *record) {
+    if (!autoshift_flags.enabled) {
+        return true;
+    }
 
-        if (elapsed > autoshift_timeout) {
-            tap_code16(LSFT(autoshift_lastkey));
+#    ifndef AUTO_SHIFT_MODIFIERS
+    if (get_mods() & (~MOD_BIT(KC_LSFT))) {
+        return true;
+    }
+#    endif
+#    ifdef AUTO_SHIFT_REPEAT
+    const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
+#        ifndef AUTO_SHIFT_NO_AUTO_REPEAT
+    if (!autoshift_flags.lastshifted) {
+#        endif
+        if (elapsed < TAPPING_TERM && keycode == autoshift_lastkey) {
+            // Allow a tap-then-hold for keyrepeat.
+            if (!autoshift_flags.lastshifted) {
+                register_code(autoshift_lastkey);
             } else {
-            tap_code(autoshift_lastkey);
+                // Simulate pressing the shift key.
+                add_weak_mods(MOD_BIT(KC_LSFT));
+                register_code(autoshift_lastkey);
             }
+            return false;
+        }
+#        ifndef AUTO_SHIFT_NO_AUTO_REPEAT
+    }
+#        endif
+#    endif
 
-        autoshift_time    = 0;
-        autoshift_lastkey = KC_NO;
-    }
-}
-
-void autoshift_on(uint16_t keycode) {
-    autoshift_time    = timer_read();
+    // Record the keycode so we can simulate it later.
     autoshift_lastkey           = keycode;
+    autoshift_time              = now;
+    autoshift_flags.in_progress = true;
+
+#    if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING)
+    clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED);
+#    endif
+    return false;
+}
+
+/** \brief Registers an autoshiftable key under the right conditions
+ *
+ * If the autoshift delay has elapsed, register a shift and the key.
+ *
+ * If the autoshift key is released before the delay has elapsed, register the
+ * key without a shift.
+ */
+static void autoshift_end(uint16_t keycode, uint16_t now, bool matrix_trigger) {
+    // Called on key down with KC_NO, auto-shifted key up, and timeout.
+    if (autoshift_flags.in_progress) {
+        // Process the auto-shiftable key.
+        autoshift_flags.in_progress = false;
+
+        // Time since the initial press was recorded.
+        const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
+        if (elapsed < autoshift_timeout) {
+            register_code(autoshift_lastkey);
+            autoshift_flags.lastshifted = false;
+        } else {
+            // Simulate pressing the shift key.
+            add_weak_mods(MOD_BIT(KC_LSFT));
+            register_code(autoshift_lastkey);
+            autoshift_flags.lastshifted = true;
+#    if defined(AUTO_SHIFT_REPEAT) && !defined(AUTO_SHIFT_NO_AUTO_REPEAT)
+            if (matrix_trigger) {
+                // Prevents release.
+                return;
+            }
+#    endif
+        }
+
+#    if TAP_CODE_DELAY > 0
+        wait_ms(TAP_CODE_DELAY);
+#    endif
+        unregister_code(autoshift_lastkey);
+        del_weak_mods(MOD_BIT(KC_LSFT));
+    } else {
+        // Release after keyrepeat.
+        unregister_code(keycode);
+        if (keycode == autoshift_lastkey) {
+            // This will only fire when the key was the last auto-shiftable
+            // pressed. That prevents aaaaBBBB then releasing a from unshifting
+            // later Bs (if B wasn't auto-shiftable).
+            del_weak_mods(MOD_BIT(KC_LSFT));
+        }
+    }
+    send_keyboard_report();  // del_weak_mods doesn't send one.
+    // Roll the autoshift_time forward for detecting tap-and-hold.
+    autoshift_time = now;
+}
+
+/** \brief Simulates auto-shifted key releases when timeout is hit
+ *
+ *  Can be called from \c matrix_scan_user so that auto-shifted keys are sent
+ *  immediately after the timeout has expired, rather than waiting for the key
+ *  to be released.
+ */
+void autoshift_matrix_scan(void) {
+    if (autoshift_flags.in_progress) {
+        const uint16_t now     = timer_read();
+        const uint16_t elapsed = TIMER_DIFF_16(now, autoshift_time);
+        if (elapsed >= autoshift_timeout) {
+            autoshift_end(autoshift_lastkey, now, true);
+        }
+    }
 }
 
 void autoshift_toggle(void) {
-    if (autoshift_enabled) {
-        autoshift_enabled = false;
-        autoshift_flush();
-    } else {
-        autoshift_enabled = true;
-    }
+    autoshift_flags.enabled = !autoshift_flags.enabled;
+    del_weak_mods(MOD_BIT(KC_LSFT));
 }
 
-void autoshift_enable(void) { autoshift_enabled = true; }
+void autoshift_enable(void) { autoshift_flags.enabled = true; }
+
 void autoshift_disable(void) {
-    autoshift_enabled = false;
-    autoshift_flush();
+    autoshift_flags.enabled = false;
+    del_weak_mods(MOD_BIT(KC_LSFT));
 }
 
 #    ifndef AUTO_SHIFT_NO_SETUP
@@ -70,19 +171,30 @@ void autoshift_timer_report(void) {
 }
 #    endif
 
-bool get_autoshift_state(void) { return autoshift_enabled; }
+bool get_autoshift_state(void) { return autoshift_flags.enabled; }
 
 uint16_t get_autoshift_timeout(void) { return autoshift_timeout; }
 
 void set_autoshift_timeout(uint16_t timeout) { autoshift_timeout = timeout; }
 
 bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
+    // Note that record->event.time isn't reliable, see:
+    // https://github.com/qmk/qmk_firmware/pull/9826#issuecomment-733559550
+    const uint16_t now = timer_read();
+
     if (record->event.pressed) {
+        if (autoshift_flags.in_progress) {
+            // Evaluate previous key if there is one. Doing this elsewhere is
+            // more complicated and easier to break.
+            autoshift_end(KC_NO, now, false);
+        }
+        // For pressing another key while keyrepeating shifted autoshift.
+        del_weak_mods(MOD_BIT(KC_LSFT));
+
         switch (keycode) {
             case KC_ASTG:
                 autoshift_toggle();
                 return true;
-
             case KC_ASON:
                 autoshift_enable();
                 return true;
@@ -102,6 +214,10 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
                 autoshift_timer_report();
                 return true;
 #    endif
+        }
+    }
+
+    switch (keycode) {
 #    ifndef NO_AUTO_SHIFT_ALPHA
         case KC_A ... KC_Z:
 #    endif
@@ -113,30 +229,13 @@ bool process_auto_shift(uint16_t keycode, keyrecord_t *record) {
         case KC_MINUS ... KC_SLASH:
         case KC_NONUS_BSLASH:
 #    endif
-                autoshift_flush();
-                if (!autoshift_enabled) return true;
-
-#    ifndef AUTO_SHIFT_MODIFIERS
-                if (get_mods()) {
-                    return true;
-                }
-#    endif
-                autoshift_on(keycode);
-
-                // We need some extra handling here for OSL edge cases
-#    if !defined(NO_ACTION_ONESHOT) && !defined(NO_ACTION_TAPPING)
-                clear_oneshot_layer_state(ONESHOT_OTHER_KEY_PRESSED);
-#    endif
-                return false;
-
-            default:
-                autoshift_flush();
-                return true;
-        }
+            if (record->event.pressed) {
+                return autoshift_press(keycode, now, record);
             } else {
-        autoshift_flush();
+                autoshift_end(keycode, now, false);
+                return false;
+            }
     }
-
     return true;
 }
 

quantum/process_keycode/process_auto_shift.h

@@ -30,3 +30,4 @@ void     autoshift_toggle(void);
 bool     get_autoshift_state(void);
 uint16_t get_autoshift_timeout(void);
 void     set_autoshift_timeout(uint16_t timeout);
+void     autoshift_matrix_scan(void);

quantum/quantum.c

@@ -58,6 +58,10 @@ float bell_song[][2] = SONG(TERMINAL_SOUND);
 #    endif
 #endif
 
+#ifdef AUTO_SHIFT_ENABLE
+#    include "process_auto_shift.h"
+#endif
+
 static void do_code16(uint16_t code, void (*f)(uint8_t)) {
     switch (code) {
         case QK_MODS ... QK_MODS_MAX:
@@ -671,6 +675,10 @@ void matrix_scan_quantum() {
     dip_switch_read(false);
 #endif
 
+#ifdef AUTO_SHIFT_ENABLE
+    autoshift_matrix_scan();
+#endif
+
     matrix_scan_kb();
 }