refactor and address PR comments

qmk · Mar 1, 2024 · 88d04fc · 88d04fc
1 parent b1a83d2
commit 88d04fc
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 73 deletions.
diff --git a/docs/feature_autocorrect.md b/docs/feature_autocorrect.md
@@ -84,13 +84,13 @@ The `qmk generate-autocorrect-data` commands can make an effort to check for ent
 
 ?> Unfortunately, this is limited to just english words, at this point.
 
-### Using multiple dictionaries
+### Using two dictionaries
 
 Including an additional dictionary under the file `autocorrect_data_alt.h` allows for on the fly switching between two sets of autocorrection rules,
 useful for bilingual users or for running context-specific rulesets.
 `QK_AUTOCORRECT_BANK_TOGGLE` can then be used to toggle the active dictionary and persist the selection to eeprom.
 
-To use this feature, `autocorrect_data_alt.h` should be generated using the flag `-a`, which constructs an alternate dictionary with appropriately named constants
+To use this feature, `autocorrect_data_alt.h` should be generated using the flag `-a`, which constructs an alternate dictionary with appropriately named constants.
 
 ## Overriding Autocorrect
 
@@ -254,14 +254,13 @@ bool apply_autocorrect(uint8_t backspaces, const char *str, char *typo, char *co
 
 Additional user callback functions to manipulate Autocorrect:
 
-| Function                   | Description                                                                      |
-|----------------------------|----------------------------------------------------------------------------------|
-| `autocorrect_enable()`     | Turns Autocorrect on.                                                            |
-| `autocorrect_disable()`    | Turns Autocorrect off.                                                           |
-| `autocorrect_toggle()`     | Toggles Autocorrect.                                                             |
-| `autocorrect_is_enabled()` | Returns true if Autocorrect is currently on.                                     |
-| `autocorrect_bank_toggle()`| Toggles and initializes Autocorrect dictionary (if multiple present)             |
-| `autocorrect_init_bank()`  | Initializes current dictionary according so selected bank  (if multiple present) |
+| Function                   | Description                                                                           |
+|----------------------------|---------------------------------------------------------------------------------------|
+| `autocorrect_enable()`     | Turns Autocorrect on.                                                                 |
+| `autocorrect_disable()`    | Turns Autocorrect off.                                                                |
+| `autocorrect_toggle()`     | Toggles Autocorrect.                                                                  |
+| `autocorrect_is_enabled()` | Returns true if Autocorrect is currently on.                                          |
+| `autocorrect_bank_toggle()`| Toggles the dictionary in use by the Autocorrect feature (if an alternate is present) |
 
 
 ## Appendix: Trie binary data format :id=appendix

diff --git a/lib/python/qmk/cli/generate/autocorrect_data.py b/lib/python/qmk/cli/generate/autocorrect_data.py
@@ -265,8 +265,7 @@ def generate_autocorrect_data(cli):
     current_keymap = cli.args.keymap or cli.config.user.keymap or cli.config.generate_autocorrect_data.keymap
 
     file_name = 'autocorrect_data_alt.h' if cli.args.alternate else 'autocorrect_data.h'
-    defines_suffix = '_ALT' if cli.args.alternate else ''
-    static_suffix = '_alt' if cli.args.alternate else ''
+    suffix = '_ALT' if cli.args.alternate else ''
 
     if current_keyboard and current_keymap:
         cli.args.output = locate_keymap(current_keyboard, current_keymap).parent / file_name
@@ -284,11 +283,11 @@ def generate_autocorrect_data(cli):
         autocorrect_data_h_lines.append(f'//   {typo:<{len(max_typo)}} -> {correction}')
 
     autocorrect_data_h_lines.append('')
-    autocorrect_data_h_lines.append(f'#define AUTOCORRECT_MIN_LENGTH{defines_suffix} {len(min_typo)} // "{min_typo}"')
-    autocorrect_data_h_lines.append(f'#define AUTOCORRECT_MAX_LENGTH{defines_suffix} {len(max_typo)} // "{max_typo}"')
-    autocorrect_data_h_lines.append(f'#define DICTIONARY_SIZE{defines_suffix} {len(data)}')
+    autocorrect_data_h_lines.append(f'#define AUTOCORRECT_MIN_LENGTH{suffix} {len(min_typo)} // "{min_typo}"')
+    autocorrect_data_h_lines.append(f'#define AUTOCORRECT_MAX_LENGTH{suffix} {len(max_typo)} // "{max_typo}"')
+    autocorrect_data_h_lines.append(f'#define DICTIONARY_SIZE{suffix} {len(data)}')
     autocorrect_data_h_lines.append('')
-    autocorrect_data_h_lines.append(f'static const uint8_t autocorrect_data{static_suffix}[DICTIONARY_SIZE{defines_suffix}] PROGMEM = {{')
+    autocorrect_data_h_lines.append(f'static const uint8_t autocorrect_data{suffix.lower()}[DICTIONARY_SIZE{suffix}] PROGMEM = {{')
     autocorrect_data_h_lines.append(textwrap.fill('    %s' % (', '.join(map(to_hex, data))), width=100, subsequent_indent='    '))
     autocorrect_data_h_lines.append('};')
 

diff --git a/quantum/process_keycode/process_autocorrect.c b/quantum/process_keycode/process_autocorrect.c
@@ -19,23 +19,48 @@
 #    include "autocorrect_data_default.h"
 #endif
 
-#ifdef AUTOCORRECT_MULTI_BANK
-#    include "autocorrect_data_alt.h"
-#    pragma message "Autocorrect is using multibank."
-#    define AML ((AUTOCORRECT_MAX_LENGTH > AUTOCORRECT_MAX_LENGTH_ALT) ? AUTOCORRECT_MAX_LENGTH : AUTOCORRECT_MAX_LENGTH_ALT)
-#    define AUTOCORRECT_DATA bank_data
+#ifdef AUTOCORRECT_DUAL_BANK
+#   define TYPO_BUFFER_SIZE ((AUTOCORRECT_MAX_LENGTH > AUTOCORRECT_MAX_LENGTH_ALT) ? AUTOCORRECT_MAX_LENGTH : AUTOCORRECT_MAX_LENGTH_ALT)
+
+typedef struct {
+    const uint8_t* data;
+    uint16_t min_length;
+    uint16_t max_length;
+    uint16_t size;
+} bank_params_t;
+
+#define PRIMARY_BANK (bank_params_t){ \
+    .data = autocorrect_data, \
+    .min_length = AUTOCORRECT_MIN_LENGTH, \
+    .max_length = AUTOCORRECT_MAX_LENGTH, \
+    .size = DICTIONARY_SIZE, \
+}
+
+#define ALT_BANK (bank_params_t){ \
+    .data = autocorrect_data_alt, \
+    .min_length = AUTOCORRECT_MIN_LENGTH_ALT, \
+    .max_length = AUTOCORRECT_MAX_LENGTH_ALT, \
+    .size = DICTIONARY_SIZE_ALT, \
+}
+
+static bank_params_t bank = PRIMARY_BANK;
 
-static const uint8_t *bank_data       = autocorrect_data;
-static uint16_t       bank_min_length = AUTOCORRECT_MIN_LENGTH;
-static uint16_t       bank_max_length = AUTOCORRECT_MAX_LENGTH;
-static uint16_t       bank_size       = DICTIONARY_SIZE;
+#   define CURRENT_DATA bank.data
+#   define CURRENT_MIN_LENGTH bank.min_length
+#   define CURRENT_MAX_LENGTH bank.max_length
+#   define CURRENT_DICTIONARY_SIZE bank.size
 
 #else
-#    define AUTOCORRECT_DATA autocorrect_data
-#    define AML AUTOCORRECT_MAX_LENGTH
+#    define TYPO_BUFFER_SIZE AUTOCORRECT_MAX_LENGTH
+
+#define CURRENT_DATA autocorrect_data
+#define CURRENT_MIN_LENGTH AUTOCORRECT_MIN_LENGTH
+#define CURRENT_MAX_LENGTH AUTOCORRECT_MAX_LENGTH
+#define CURRENT_DICTIONARY_SIZE DICTIONARY_SIZE
+
 #endif
 
-static uint8_t typo_buffer[AML] = {KC_SPC};
+static uint8_t typo_buffer[TYPO_BUFFER_SIZE] = {KC_SPC};
 static uint8_t typo_buffer_size = 1;
 
 /**
@@ -77,11 +102,11 @@ void autocorrect_toggle(void) {
     eeconfig_update_keymap(keymap_config.raw);
 }
 
+#ifdef AUTOCORRECT_DUAL_BANK
 /**
  * @brief Toggles autocorrect's bank and save state to eeprom
  *
  */
-#ifdef AUTOCORRECT_MULTI_BANK
 void autocorrect_bank_toggle(void) {
     keymap_config.autocorrect_bank = !keymap_config.autocorrect_bank;
     autocorrect_init_bank();
@@ -90,17 +115,7 @@ void autocorrect_bank_toggle(void) {
 
 void autocorrect_init_bank(void) {
     typo_buffer_size = 0;
-    if (keymap_config.autocorrect_bank) {
-        bank_data       = autocorrect_data_alt;
-        bank_min_length = AUTOCORRECT_MIN_LENGTH_ALT;
-        bank_max_length = AUTOCORRECT_MAX_LENGTH_ALT;
-        bank_size       = DICTIONARY_SIZE_ALT;
-    } else {
-        bank_data       = autocorrect_data;
-        bank_min_length = AUTOCORRECT_MIN_LENGTH;
-        bank_max_length = AUTOCORRECT_MAX_LENGTH;
-        bank_size       = DICTIONARY_SIZE;
-    }
+    bank = keymap_config.autocorrect_bank ? ALT_BANK : PRIMARY_BANK;
 }
 #endif
 
@@ -252,7 +267,7 @@ bool process_autocorrect(uint16_t keycode, keyrecord_t *record) {
             autocorrect_disable();
         } else if (keycode == QK_AUTOCORRECT_TOGGLE) {
             autocorrect_toggle();
-#ifdef AUTOCORRECT_MULTI_BANK
+#ifdef AUTOCORRECT_DUAL_BANK
         } else if (keycode == QK_AUTOCORRECT_BANK_TOGGLE) {
             autocorrect_bank_toggle();
 #endif
@@ -313,71 +328,56 @@ bool process_autocorrect(uint16_t keycode, keyrecord_t *record) {
     }
 
         // Rotate oldest character if buffer is full.
-#ifdef AUTOCORRECT_MULTI_BANK
-    if (typo_buffer_size >= bank_max_length) {
-        memmove(typo_buffer, typo_buffer + 1, bank_max_length - 1);
-        typo_buffer_size = bank_max_length - 1;
+    if (typo_buffer_size >= CURRENT_MAX_LENGTH) {
+        memmove(typo_buffer, typo_buffer + 1, CURRENT_MAX_LENGTH - 1);
+        typo_buffer_size = CURRENT_MAX_LENGTH - 1;
     }
-#else
-    if (typo_buffer_size >= AUTOCORRECT_MAX_LENGTH) {
-        memmove(typo_buffer, typo_buffer + 1, AUTOCORRECT_MAX_LENGTH - 1);
-        typo_buffer_size = AUTOCORRECT_MAX_LENGTH - 1;
-    }
-#endif
 
     // Append `keycode` to buffer.
     typo_buffer[typo_buffer_size++] = keycode;
     // Return if buffer is smaller than the shortest word.
-#ifdef AUTOCORRECT_MULTI_BANK
-    if (typo_buffer_size < bank_min_length) {
-#else
-    if (typo_buffer_size < AUTOCORRECT_MIN_LENGTH) {
-#endif
+    if (typo_buffer_size < CURRENT_MIN_LENGTH) {
         return true;
     }
 
-    // Check for typo in buffer using a trie stored in `AUTOCORRECT_DATA`.
+    // Check for typo in buffer using a trie stored in `CURRENT_DATA`.
     uint16_t state = 0;
-    uint8_t  code  = pgm_read_byte(AUTOCORRECT_DATA + state);
+    uint8_t  code  = pgm_read_byte(CURRENT_DATA + state);
     for (int8_t i = typo_buffer_size - 1; i >= 0; --i) {
         uint8_t const key_i = typo_buffer[i];
 
         if (code & 64) { // Check for match in node with multiple children.
             code &= 63;
-            for (; code != key_i; code = pgm_read_byte(AUTOCORRECT_DATA + (state += 3))) {
+            for (; code != key_i; code = pgm_read_byte(CURRENT_DATA + (state += 3))) {
                 if (!code) return true;
             }
             // Follow link to child node.
-            state = (pgm_read_byte(AUTOCORRECT_DATA + state + 1) | pgm_read_byte(AUTOCORRECT_DATA + state + 2) << 8);
+            state = (pgm_read_byte(CURRENT_DATA + state + 1) | pgm_read_byte(CURRENT_DATA + state + 2) << 8);
             // Check for match in node with single child.
         } else if (code != key_i) {
             return true;
-        } else if (!(code = pgm_read_byte(AUTOCORRECT_DATA + (++state)))) {
+        } else if (!(code = pgm_read_byte(CURRENT_DATA + (++state)))) {
             ++state;
         }
 
         // Stop if `state` becomes an invalid index. This should not normally
         // happen, it is a safeguard in case of a bug, data corruption, etc.
-#ifdef AUTOCORRECT_MULTI_BANK
-        if (state >= bank_size) {
-#else
-        if (state >= DICTIONARY_SIZE) {
-#endif
+        if (state >= CURRENT_DICTIONARY_SIZE) {
             return true;
         }
 
-        code = pgm_read_byte(AUTOCORRECT_DATA + state);
+        code = pgm_read_byte(CURRENT_DATA + state);
 
         if (code & 128) { // A typo was found! Apply autocorrect.
             const uint8_t backspaces = (code & 63) + !record->event.pressed;
-            const char   *changes    = (const char *)(AUTOCORRECT_DATA + state + 1);
+            const char   *changes    = (const char *)(CURRENT_DATA + state + 1);
 
             /* Gather info about the typo'd word
              *
              * Since buffer may contain several words, delimited by spaces, we
              * iterate from the end to find the start and length of the typo
              */
-            char typo[AML + 1] = {0}; // extra char for null terminator
+            char typo[TYPO_BUFFER_SIZE + 1] = {0}; // extra char for null terminator
 
             uint8_t typo_len   = 0;
             uint8_t typo_start = 0;
@@ -410,7 +410,7 @@ bool process_autocorrect(uint16_t keycode, keyrecord_t *record) {
              *
              * B) When correcting 'typo' -- Need extra offset for terminator
              */
-            char correct[AML + 10] = {0}; // let's hope this is big enough
+            char correct[TYPO_BUFFER_SIZE + 10] = {0}; // let's hope this is big enough
 
             uint8_t offset = space_last ? backspaces : backspaces + 1;
             strcpy(correct, typo);

diff --git a/quantum/process_keycode/process_autocorrect.h b/quantum/process_keycode/process_autocorrect.h
@@ -11,7 +11,7 @@
 #include "action.h"
 
 #if __has_include("autocorrect_data_alt.h")
-#    define AUTOCORRECT_MULTI_BANK
+#    define AUTOCORRECT_DUAL_BANK
 #endif
 
 bool process_autocorrect(uint16_t keycode, keyrecord_t *record);
@@ -24,7 +24,7 @@ void autocorrect_enable(void);
 void autocorrect_disable(void);
 void autocorrect_toggle(void);
 
-#ifdef AUTOCORRECT_MULTI_BANK
+#ifdef AUTOCORRECT_DUAL_BANK
 void autocorrect_bank_toggle(void);
 void autocorrect_init_bank(void);
 #endif
diff --git a/quantum/quantum.c b/quantum/quantum.c
@@ -556,7 +556,7 @@ __attribute__((weak)) void suspend_wakeup_init_quantum(void) {
     backlight_init();
 #endif
 
-#ifdef AUTOCORRECT_MULTI_BANK
+#ifdef AUTOCORRECT_DUAL_BANK
     // refresh autocorrect bank
     autocorrect_init_bank();
 #endif