Don't clear keys on layer change unless STRICT_LAYER_RELEASE is enabled

[drashna]

Description

Previous behavior clears mousekeys on layer change, which is not expected/proper behavior.

Not sure if extrakeys should be moved too?

Types of Changes

• Core
• Bugfix
• Enhancement/optimization
• Documentation

Issues Fixed or Closed by This PR

• Fixes [Feature Request] Add option similar to STRICT_LAYER_RELEASE but for mouse keys. #18300

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[fauxpark]

clear_keyboard_but_mods_and_keys seems to imply it should clear everything but the normal keyboard report, hence why extrakeys and programmable buttons are in there too. So I think this function must be being used incorrectly.

[drashna]

Yeah, the naming here is ... messy.

And to be honest, I'm not sure that clearing the extrakeys here is the right behavior, either.

[sigprof]

Apparently clear_keyboard_but_mods_and_keys() is called only in 3 places:

• clear_keyboard_but_mods(), which is just a more extensive clear (and that looks like the correct place to clear mousekeys, extrakeys and programmable buttons);
• default_layer_state_set();
• layer_state_set().

The real question is why we even need to perform that clear on layer state changes if STRICT_LAYER_RELEASE is not defined? Was that a workaround for some cases when the source layer cache did not work properly?

If we move clearing of “unusual” keys to clear_keyboard_but_mods(), then the only code remaining in clear_keyboard_but_mods_and_keys() would be

    clear_weak_mods();
    send_keyboard_report();

Normally weak mods are cleared only on key press events (so that weak mods that were applied to some keypress stay active until another key is pressed). Should a layer state change affect them too?

[drashna]

Looks like this is basically left over from the switch to "prevent stuck mods" to being default.

#3905 added the checks for strict layer release, but original PR didn't change behaviors.

So, that kind of begs the question, how should this really be handled?

[KarlK90]

There is actually a unit-test https://github.com/qmk/qmk_firmware/blob/master/tests/basic/test_action_layer.cpp#L362-L402 that describes a use-case that should normally work but that fails because the modifiers are discarded during layer_state_set(). I have removed clear_keyboard_but_mods_and_keys() from this function in my personal fork and it works fine. but there is one edge case that I still have to debug: Pressing LCTL_T(KC_E) + LSFT_T(KC_SPC) + RSFT_T(KC_SPC) inside the tapping term somehow toggles caps lock. (can't reproduce atm)

[KarlK90]

LGTM but I wonder if we can just remove the SEMI_STRICT_LAYER_RELEASE case.

[drashna]

LGTM but I wonder if we can just remove the SEMI_STRICT_LAYER_RELEASE case.

Definitely can. Was there as a "just in case". Can remove now, but might be worth leaving for now and remove next cycle.