-
-
Notifications
You must be signed in to change notification settings - Fork 40.7k
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
Improvement of Space Cadet Shift by preventing to automatically apply… #3856
Conversation
… a modifier on the key and allow to override the default modifier. Closes qmk#3815
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 👍
|`LSPO_MOD` |`KC_LSFT` |The keycode to send when Left Shift is tapped | | ||
|`RSPC_MOD` |`KC_RSFT` |The keycode to send when Right Shift is tapped | | ||
|`DISABLE_SPACE_CADET_ROLLOVER`|*Not defined*|If defined, use the opposite Shift key to cancel Space Cadet | | ||
|`DISABLE_SPACE_CADET_MODIFIER`|*Not defined*|If defined, prevent the Space Cadet to apply a modifier to LSPO_KEY and RSPC_KEY| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to have corresponding options (DISABLE_LSPO_MOD, DISABLE_LSPC_MOD) for both sides? I am not sure if this is necessary for any language if your goal is to have matching pairs of parentheses or brackets on the left and right shift key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's not needed. But it can be changed later if required.
@@ -627,14 +634,21 @@ bool process_record_quantum(keyrecord_t *record) { | |||
} | |||
else { | |||
#ifdef DISABLE_SPACE_CADET_ROLLOVER | |||
if (get_mods() & MOD_BIT(KC_RSFT)) { | |||
if (get_mods() & MOD_BIT(RSPC_MOD)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested this with DISABLE_SPACE_CADET_ROLLOVER on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And yes as well 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question was repeated accidentally, not to emphasize it ;-).
…egistering KC_LSFT when equals to LSPO_MOD
@frederik-h if you have some spare time to review, I edited the code to avoid to unregister the mod if KC_SLFT == LSPO_MOD as well as addressed your other remarks. |
@anthonyrichir Looks fine to me |
Thanks a lot. |
I don't know, I guess either you could wait until someone who could merge this notices it or you could ask. @drashna has recently reviewed and merged a PR of mine. |
change #if to if statement
@drashna you seems to be the only one very active around here, how can I can get a review for this PR ? |
That's because I mostly handle keyboards, keymaps, and documentation. I'm not as skilled or as good with programming as they are, so I'm hesitant to merge "core" code. Sorry. |
Yes, don't be sorry, I understand and I'm not requesting you to do it, I was more wondering what was going on as it's been quite some time since I opened this PR. |
@jackhumbert @skullydazed |
What a difference that time makes. Just to make sure, this is tested and does work, correct? |
I use it on a daily basis on my Planck, I don't know if it's good enough for you guys. |
Thanks! |
Congrats! |
* Improvement of Space Cadet Shift by preventing to automatically apply a modifier on the key and allow to override the default modifier. Closes qmk#3815 * Improve the use of the DISABLE_SPACE_CADET_MODIFIER flag to avoid unregistering KC_LSFT when equals to LSPO_MOD * change #if to if statement
* Improvement of Space Cadet Shift by preventing to automatically apply a modifier on the key and allow to override the default modifier. Closes qmk#3815 * Improve the use of the DISABLE_SPACE_CADET_MODIFIER flag to avoid unregistering KC_LSFT when equals to LSPO_MOD * change #if to if statement
… a modifier on the key and allow to override the default modifier. Closes #3815.
I added LSPO_MOD and RSPC_MOD which, by default, are set to KC_LSFT and KC_RSFT.
I also added the possibility to define DISABLE_SPACE_CADET_MODIFIER to prevent the firmware to apply a modifier to LSPO_KEY and RSPC_KEY.