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.

Sign up for GitHub

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

Jump to bottom

Mask off keycode/layer/mod where possible in LT(), MT(), etc. #3430

Merged
merged 4 commits into from
Aug 30, 2018
Merged

Mask off keycode/layer/mod where possible in LT(), MT(), etc. #3430

merged 4 commits into from
Aug 30, 2018

Conversation

fauxpark
Copy link
Member

Somewhat related to #3265 (but does not solve it).

LT(), MT(), TO() and friends need to prevent their arguments spilling over into other sections of the keycode and potentially turning it into something else. Modified keycodes in a LT() remain unmodified, but the modifier is simply stripped away so it can't interfere with the layer bits.

@drashna drashna requested a review from jackhumbert July 18, 2018 01:26
@drashna drashna requested a review from fredizzimo July 18, 2018 17:15
@fauxpark fauxpark mentioned this pull request Aug 5, 2018
Merged
@fauxpark
Copy link
Member Author

It occurs to me that mod and kc may contain bitwise ORs (eg. LCTL(kc)) to combine modifiers, which the AND masking takes precedence over. It will AND the mask only with everything after the last OR, but we want it to OR first, then AND to clamp it to 0-255 (or 0-15). So to preserve order of operations, I've put back the parentheses and added some more where things were ORed with kc.

@fauxpark
Copy link
Member Author

Hey @drashna, what's the status on this? Is there something else I need to do or are we just waiting on the review?

@jackhumbert jackhumbert merged commit 097f1a2 into qmk:master Aug 30, 2018
@jackhumbert
Copy link
Member

Looks good! Thanks :)

@fauxpark fauxpark deleted the macro-masking branch August 30, 2018 01:06
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Aug 31, 2018
@Shinichi-Ohki
Merge branch 'master' of https://github.com/qmk/qmk_firmware
afc4c71 
* 'master' of https://github.com/qmk/qmk_firmware: (73 commits)
  Keymap: Updated keymap with the "pretty" layout (qmk#3812)
  Keyboard: CTRL and ALT updates (qmk#3810)
  Docs: Tabulate Modifier & Mod-Tap keycode listings in advanced keycodes docs (qmk#3799)
  Keymap: Wanleg updates (qmk#3794)
  Keymap: Added new tada68 keymap (qmk#3788)
  Tweak the wording in "Becoming a QMK Collaborator"
  Docs: add process_terminal() and update links to other functions (qmk#3778)
  Keymap: Update keymap including LAYOUT_planck_grid (qmk#3779)
  Keyboard: remove old comment from keyboards/helix/rules.mk (qmk#3795)
  Keyboard: Update CTRL and ALT keyboard readme (qmk#3796)
  Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
  Autodetect lack of screen presence
  Massdrop keyboards readme update (qmk#3791)
  move massdrop boards into its own directory for configurator visibility
  STM32 EEPROM Emulation (qmk#3741)
  Massdrop keyboard support (qmk#3780)
  Keymap: Add german layout for redox keyboard (qmk#3695)
  Keyboard: Adding the AT101 PCB (qmk#3785)
  Updated templates for use by new_project.sh (qmk#3783)
  Fixed bootloader target Fixed matching grep matches (for PRODUCT in particular) Fixed " Bootloader" concatenation for WSL (windows line-endings)
  ...
noj added a commit to noj/qmk_firmware that referenced this pull request Aug 31, 2018
@noj
Merge branch 'master' into nojjan_atreus
7c6de31 
* master: (485 commits)
  Keymap: Adding adnw_puq-Layout for Ergodox (qmk#3723)
  Keymap: I got my intern a KBParadise V60 Type R... (qmk#3813)
  Keyboard: Adding Z-150 PCB firmware (qmk#3793)
  Keymap: Add backlight controls for Planck Light in dshields keymap (qmk#3801)
  Keyboard: CTRL and ALT keyboard json fix (qmk#3816)
  Fix emulated EEPROM start address on STM32F303 (qmk#3819)
  Keymap: Updated keymap with the "pretty" layout (qmk#3812)
  Keyboard: CTRL and ALT updates (qmk#3810)
  Docs: Tabulate Modifier & Mod-Tap keycode listings in advanced keycodes docs (qmk#3799)
  Keymap: Wanleg updates (qmk#3794)
  Keymap: Added new tada68 keymap (qmk#3788)
  Tweak the wording in "Becoming a QMK Collaborator"
  Docs: add process_terminal() and update links to other functions (qmk#3778)
  Keymap: Update keymap including LAYOUT_planck_grid (qmk#3779)
  Keyboard: remove old comment from keyboards/helix/rules.mk (qmk#3795)
  Keyboard: Update CTRL and ALT keyboard readme (qmk#3796)
  Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
  Autodetect lack of screen presence
  Massdrop keyboards readme update (qmk#3791)
  move massdrop boards into its own directory for configurator visibility
  ...
alexey-danilov pushed a commit to alexey-danilov/qmk_firmware that referenced this pull request Sep 6, 2018
@fauxpark
Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
a82c73f 
* Mask off keycode/layer/mod where possible in LT(), MT(), etc.

* Don't need these parentheses

* Put back parentheses for order of operations
ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Sep 25, 2018
@fauxpark @ChrissiQ
Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
a8da18a 
* Mask off keycode/layer/mod where possible in LT(), MT(), etc.

* Don't need these parentheses

* Put back parentheses for order of operations
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
@fauxpark @yamad
Mask off keycode/layer/mod where possible in LT(), MT(), etc. (qmk#3430)
29bff36 
* Mask off keycode/layer/mod where possible in LT(), MT(), etc.

* Don't need these parentheses

* Put back parentheses for order of operations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackhumbert jackhumbert Awaiting requested review from jackhumbert

@fredizzimo fredizzimo Awaiting requested review from fredizzimo

Assignees
No one assigned
Labels
core enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fauxpark @jackhumbert @drashna