From 88d04fce1100b94a35635248be54c57abb4698f2 Mon Sep 17 00:00:00 2001 From: Sean Bocirnea Date: Thu, 29 Feb 2024 21:30:43 -0800 Subject: [PATCH] refactor and address PR comments --- docs/feature_autocorrect.md | 19 ++- .../qmk/cli/generate/autocorrect_data.py | 11 +- quantum/process_keycode/process_autocorrect.c | 108 +++++++++--------- quantum/process_keycode/process_autocorrect.h | 4 +- quantum/quantum.c | 2 +- 5 files changed, 71 insertions(+), 73 deletions(-) diff --git a/docs/feature_autocorrect.md b/docs/feature_autocorrect.md index 4cf7970e413c..f6b34c9feab2 100644 --- 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 index c82de2c7b18e..8d1f6ad6dfb0 100644 --- 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 index 976820a6f53f..3f89b4ffcc9b 100644 --- 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 index 59d52ade5e2c..51e678f73f01 100644 --- 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 index 2f2bba263f72..ca6bd00c55c5 100644 --- 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