Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make MAGIC_SWAP_* work with mod-tap #18349

Closed
1 task
osamuaoki opened this issue Sep 13, 2022 · 5 comments
Closed
1 task

make MAGIC_SWAP_* work with mod-tap #18349

osamuaoki opened this issue Sep 13, 2022 · 5 comments
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.

Comments

@osamuaoki
Copy link
Contributor

Currently, we have 2 nicely working features:

  • mod-tap such as MT(MOD_LCTL, KC_ESC)
  • MAGIC such as MAGIC_SWAP_ESCAPE_CAPS (Thiaks for including this feature request from me.)

They work as expected as long as they are not used together.

But if they are used together, SWAP is not used since switch in quantum/keycode_config.c uses bare keycode without mask.

This is not just for this particular MAGIC but seems to be a generic issue around MAGIC handling code.

Feature Request Type

  • [v ] Core functionality
  • Add-on hardware support (eg. audio, RGB, OLED screen, etc.)
  • [v ] Alteration (enhancement/optimization) of existing feature(s)
  • [] New behavior

Description

For local workaround, I patched as below by masking high 8 bits for some MAGIC handling. (devel branch)

 quantum/keycode_config.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/quantum/keycode_config.c b/quantum/keycode_config.c
index 5b5cc5d28e..cbb6fc2bf2 100644
--- a/quantum/keycode_config.c
+++ b/quantum/keycode_config.c
@@ -90,28 +90,36 @@ uint16_t keycode_config(uint16_t keycode) {
                 return KC_NO;
             }
             return KC_RIGHT_GUI;
+    }
+    switch (keycode & ~(0xff <<8)) { // ignore action code
+        case KC_CAPS_LOCK:
+        case KC_LOCKING_CAPS_LOCK:
+            if (keymap_config.swap_escape_capslock) {
+                return KC_ESCAPE | (keycode & (0xff <<8));
+            }
+            return keycode;
         case KC_GRAVE:
             if (keymap_config.swap_grave_esc) {
-                return KC_ESCAPE;
+                return KC_ESCAPE | (keycode & (0xff <<8));
             }
-            return KC_GRAVE;
+            return keycode;
         case KC_ESCAPE:
             if (keymap_config.swap_grave_esc) {
-                return KC_GRAVE;
+                return KC_GRAVE | (keycode & (0xff <<8));
             } else if (keymap_config.swap_escape_capslock) {
-                return KC_CAPS_LOCK;
+                return KC_CAPS_LOCK | (keycode & (0xff <<8));
             }
-            return KC_ESCAPE;
+            return keycode;
         case KC_BACKSLASH:
             if (keymap_config.swap_backslash_backspace) {
-                return KC_BACKSPACE;
+                return KC_BACKSPACE | (keycode & (0xff <<8));
             }
-            return KC_BACKSLASH;
+            return keycode;
         case KC_BACKSPACE:
             if (keymap_config.swap_backslash_backspace) {
-                return KC_BACKSLASH;
+                return KC_BACKSLASH | (keycode & (0xff <<8));
             }
-            return KC_BACKSPACE;
+            return keycode;
         default:
             return keycode;
     }
-- 
2.35.1

This seems to fix situation for me as I just played but I don't know side effect to mouse key and other possible regressions. Besides, I didn't test this much. This is merely here to indicate issues to be solved..

Someone with good understanding of action code needs to think through this problem.

Regards,

Osamu

@osamuaoki
Copy link
Contributor Author

FYI: I have a bit over complicated equation for mask value since I initially tested equation for mask with 0x1f. That didn't work well. May be replace them with 0xff00 and 0x00ff..

@sigprof
Copy link
Contributor

sigprof commented Sep 13, 2022

There is a pending PR that does a similar thing:

Maybe that PR should be moved forward (there are some concerns that it also changes the behavior of keycodes with modifiers, which may look strange, but then if you are doing things like OS-specific layers, you probably don't need those magic switches at all).

@osamuaoki osamuaoki mentioned this issue Sep 13, 2022
13 tasks
@osamuaoki
Copy link
Contributor Author

After some trials, I cleaned up my modification. At least some of my initial concern points were addressed.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs.
For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Dec 13, 2022
@github-actions
Copy link

This issue has been automatically closed because it has not had activity in the last 30 days. If this issue is still valid, re-open the issue and let us know.
// [stale-action-closed]

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

No branches or pull requests

2 participants