Tap mod swap magic

[osamuaoki]

This is for #18349

This worked for my case of combination of

• mod-tap MT(MOD_LCTL, KC_ESC)
• MAGIC MAGIC_SWAP_ESCAPE_CAPS

At least some effort is made to avoid applying swap for non-keycode
Other case are included and made to be synmetrical.
Code may need some reformatting. (/* ...*/ comment)
This is more-or-less proof-of-concept patch.
So there may be shortcomings.

Description

See issue. #18349

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

This solves #18349

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.
• [NO] 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). --- Yes tested but not exhaustive yet.

[sigprof]

Did you look at another implementation of basically the same thing in #9126?

One thing that looks strange in the resulting API is that keycode_config() remaps the embedded basic keycodes in the passed value, but does not remap the embedded mod bits in some of those keycodes, therefore the conversion performed by this function ends up incomplete. Maybe a more consistent API would include that conversion too (e.g., rename the existing keycode_config() to uint8_t basic_keycode_config(uint8_t keycode), and then implement uint16_t keycode_config(uint16_t keycode) in terms of basic_keycode_config() and mod_config(); also remove the mod_config() calls from action_for_keycode()). Although I'm not 100% sure about this part.

[osamuaoki]

Thanks for the correction. Yah, that's makes sense.

[osamuaoki]

I don't understand why "bootmagic config" is used to describe "magic" in the original source code. I don't see this expression elsewhere. My new comment followed sigprof's text thus use "magic"

[osamuaoki]

Hmmm... I now guess why "boot" in "bootmagic". Magic is persistent and boot time default. It has little or nothing in common with "BOOTMAGIC LITE" ESC-power-on programming. Still confusing.

[drashna]

I don't understand why "bootmagic config" is used to describe "magic" in the original source code. I don't see this expression elsewhere. My new comment followed sigprof's text thus use "magic"

Because bootmagic also controlled a lot of this functionality, too. However, because it is too easy to accidentally trigger, we removed the bootmagic code to do this completely. (a LOT of tickets of "why are these keys switched?!?!")

[osamuaoki]

I don't understand why "bootmagic config" is used to describe "magic" in the original source code. I don't see this expression elsewhere. My new comment followed sigprof's text thus use "magic"

Because bootmagic also controlled a lot of this functionality, too. However, because it is too easy to accidentally trigger, we removed the bootmagic code to do this completely. (a LOT of tickets of "why are these keys switched?!?!")

I see. But if so, why we use persistent memory to store magic. For me, MAGIC_SWAP_FOO feature is useful as a temporary work around when I plugged in to a machine with some special input handling. We can program QMK default map for our persistent preference anyway if you programmed it. For end user products, they probably add DIP switch to set it.

I now see this persistent variables for magic is defined in bootmagic. Why not making this keymap_config struct as a simple global variable. This is easily done by adding a compiler flag with a few lines of code so keeping backward compatibility option is easily implemented. So no one get confused if you accidentally change magic swap setting. Re-plug keyboard will always reset magic swap settings.

[osamuaoki]

Hi,

Applied extra patch to address possible issues:
#9126 (comment)

I think all these can be squashed

[sigprof]

The standard practice in QMK is to squash commits when merging a PR, so you don't need to worry about that — use separate commits if it's easier for you.

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[github-actions]

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]