In #4337 we added the MOD_MASK_CTRL, MOD_MASK_SHIFT, MOD_MASK_CSA etc. constants to core code. These are shorthand for (MOD_BIT(KC_LCTRL) | MOD_BIT(KC_RCTRL)) etc. since expressions of this form are very common.
Would constants for single modifiers, e.g. #define MOD_MASK_LCTRL MOD_BIT(KC_LCTRL), be equally as useful? I think they could be. I see a lot of checks in the codebase that use MOD_BIT() with single modifiers. Adding these definitions would “complete the set”, so to speak. It could make code that uses mod masks more uniform and easier to understand, since it would essentially remove the need for MOD_BIT(). It could also help reduce the confusion between:
MOD_MASK_* constants (used with functions such as get_mods(), set_mods()); andMOD_* constants (used with MT(), LM(), OSM() and internally in tmk_core)by removing expressions with MOD_BIT() from the equation. Finally, MOD_MASK_LCTRL is also shorter and easier to type than MOD_BIT(KC_LCTRL).
The new constants could be implemented like this: https://github.com/qmk/qmk_firmware/compare/master...vomindoraan:mod_mask_single.
Another thing to consider are the names. Should the new constants use:
MOD_MASK_LSHIFT (consistent with MOD_MASK_SHIFT); orMOD_MASK_LSFT (consistent with MOD_LSFT, also shorter and easier to align); or/discuss
@drashna @noroadsleft — I'd appreciate your guys' input on this matter, since you seem to be using the MOD_MASK_* constants the most. @fauxpark too, since you were involved with the original PR. Also @skullydazed, since you seem to have the best idea of where QMK is heading in terms of code style.
I think it would be useful, yeah. I think it would be an edge case, though. But that's fine.
Also, I think that MEH and HYPER should be added, as well. :)
As for the naming, I think aliasing the various options would be best. That way either can be used, interchangeably. Which is the current behavior for keycodes, and the like.
I can't speak to masks for MEH or HYPER, but I agree with everything else drashna said.
This issue has been automatically marked as resolved 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.
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.
Most helpful comment
I think it would be useful, yeah. I think it would be an edge case, though. But that's fine.
Also, I think that MEH and HYPER should be added, as well. :)
As for the naming, I think aliasing the various options would be best. That way either can be used, interchangeably. Which is the current behavior for keycodes, and the like.