qmk / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
18.24k stars 39.32k forks source link

[Bug] Combo which produces `KC_ESC` does not break out of Caps Word #19381

Closed adiron closed 1 year ago

adiron commented 1 year ago

Describe the Bug

I have the combo of J+K set up to perform KC_ESC. The characters J and K are of course not stop keys for Caps Word.

Inputting the following: <Both Shifts to enter Caps Word>hello<J+K combo to perform Escape>world will produce HELLOWORLD

Whereas: <Both Shifts to enter Caps Word>hello<a key straight up bound to KC_ESC>world will produce HELLOworld (as expected)

I suspect that Caps Word does not recognize J+K as stop chars since both KC_J and KC_K are acceptable, and does not check for the fact that KC_ESC was fired.

Keyboard Used

xiudi/xd75:adi

Link to product page (if applicable)

No response

Operating System

No response

qmk doctor Output

Ψ QMK Doctor is checking your environment. Ψ CLI version: 1.1.1 Ψ QMK home: /Users/adi.ron/LocalDev/qmk_firmware Ψ Detected macOS 12.5.1 (Intel). Ψ Git branch: adi Ψ Repo version: 0.19.4 Ψ - Latest adi: 2022-12-19 21:10:18 +0200 (e109e2a847) -- Updated some rules Ψ - Latest upstream/master: 2022-12-19 10:59:48 +0100 (8d43f20624) -- Adding Duck Viper/Eagle V2/V3 Replacement PCB Rev B (#18584) Ψ - Latest upstream/develop: 2022-12-19 10:00:37 +0000 (174263445f) -- Merge remote-tracking branch 'origin/master' into develop Ψ - Common ancestor with upstream/master: 2022-12-19 00:28:30 -0700 (8655b100b1) -- keebio/quefrency — add missing LAYOUT_all's to info.json (#19373) Ψ - Common ancestor with upstream/develop: 2022-12-19 00:28:30 -0700 (8655b100b1) -- keebio/quefrency — add missing LAYOUT_all's to info.json (#19373) Ψ CLI installed in virtualenv. Ψ All dependencies are installed. Ψ Found arm-none-eabi-gcc version 10.3.1 Ψ Found avr-gcc version 9.4.0 ⚠ We do not recommend avr-gcc newer than 8. Downgrading to 8.x is recommended. Ψ Found avrdude version 7.0 Ψ Found dfu-util version 0.11 Ψ Found dfu-programmer version 0.7.2 Ψ Submodules are up to date. Ψ Submodule status: Ψ - lib/chibios: 2022-09-18 10:01:17 +0000 -- (0e9d558b52) Ψ - lib/chibios-contrib: 2022-10-03 18:09:41 +0200 -- (bb8356fb) Ψ - lib/googletest: 2021-06-11 06:37:43 -0700 -- (e2239ee6) Ψ - lib/lufa: 2022-08-26 12:09:55 +1000 -- (549b97320) Ψ - lib/pico-sdk: 2022-09-19 18:02:44 +0200 -- (8d56ea3) Ψ - lib/printf: 2022-06-29 23:59:58 +0300 -- (c2e3b4e) Ψ - lib/vusb: 2022-06-13 09:18:17 +1000 -- (819dbc1) Ψ QMK is ready to go, but minor problems were found

Is AutoHotKey / Karabiner installed

Other keyboard-related software installed

No response

Additional Context

Exact keymap to reproduce this keyboard exists in my own fork of qmk_firmware

sigprof commented 1 year ago

This happens because you have [COMBO_ESC] = COMBO_ACTION(combo_esc) and use process_combo_event() to implement the combo as tap_code(KC_ESC). Any direct tap_code() calls are not processed by Caps Word or any other event processing code.

You should use [COMBO_ESC] = COMBO(combo_esc, KC_ESC) instead and drop the handling in process_combo_event(); in this case the resulting KC_ESC will be processed by process_record_user() and any other event processing functions, including the Caps Word feature.

BTW, using process_combo_event() may be problematic if you also have dual-role keys like LT() or MT() in your keymap, because process_combo_event() may be called too early in some cases (if you press a combo quickly after pressing a dual-role key, process_combo_event() may be called before the dual-role key is actually handled). To avoid this problem, you should map the combo to a custom keycode (combos now support any keycodes as their outputs including even things like LT() or MT()), and then handle that keycode in process_record_user() as if it was on a real key; then process_combo_event() could be removed completely.

adiron commented 1 year ago

Oh, didn't know that. Thank you for the advice, very solid stuff!

Maybe this pitfall should be stated in the docs, but I leave that up to someone more knowledgeable than me

Thanks for he help!