refactor Key and ModifierKey __call__ methods

- Add tests for ModifierKey chaining
- Rewrite ModifierKey.__call__ for correctness and readability
- __call__ now maintains handlers and meta, like clone()

kmk/keys.py

@@ -398,11 +398,14 @@ class Key:
         if no_press is None and no_release is None:
             return self
 
-        return Key(
+        return type(self)(
             code=self.code,
             has_modifiers=self.has_modifiers,
             no_press=no_press,
             no_release=no_release,
+            on_press=self._handle_press,
+            on_release=self._handle_release,
+            meta=self.meta,
         )
 
     def __repr__(self):
@@ -557,43 +560,36 @@ class Key:
 
 
 class ModifierKey(Key):
-    # FIXME this is atrocious to read. Please, please, please, strike down upon
-    # this with great vengeance and furious anger.
-
     FAKE_CODE = const(-1)
 
-    def __call__(self, modified_code=None, no_press=None, no_release=None):
+    def __call__(self, modified_key=None, no_press=None, no_release=None):
-        if modified_code is None and no_press is None and no_release is None:
+        if modified_key is None:
-            return self
+            return super().__call__(no_press=no_press, no_release=no_release)
 
-        if modified_code is not None:
+        modifiers = set()
-            if isinstance(modified_code, ModifierKey):
+        code = modified_key.code
-                new_keycode = ModifierKey(
-                    ModifierKey.FAKE_CODE,
-                    set() if self.has_modifiers is None else self.has_modifiers,
-                    no_press=no_press,
-                    no_release=no_release,
-                )
 
-                if self.code != ModifierKey.FAKE_CODE:
+        if self.code != ModifierKey.FAKE_CODE:
-                    new_keycode.has_modifiers.add(self.code)
+            modifiers.add(self.code)
+        if self.has_modifiers:
+            modifiers |= self.has_modifiers
+        if modified_key.has_modifiers:
+            modifiers |= modified_key.has_modifiers
 
-                if modified_code.code != ModifierKey.FAKE_CODE:
+        if isinstance(modified_key, ModifierKey):
-                    new_keycode.has_modifiers.add(modified_code.code)
+            if modified_key.code != ModifierKey.FAKE_CODE:
-            else:
+                modifiers.add(modified_key.code)
-                new_keycode = Key(
+            code = ModifierKey.FAKE_CODE
-                    modified_code.code,
-                    {self.code},
-                    no_press=no_press,
-                    no_release=no_release,
-                )
 
-            if modified_code.has_modifiers:
+        return type(modified_key)(
-                new_keycode.has_modifiers |= modified_code.has_modifiers
+            code=code,
-        else:
+            has_modifiers=modifiers,
-            new_keycode = Key(self.code, no_press=no_press, no_release=no_release)
+            no_press=no_press,
-
+            no_release=no_release,
-        return new_keycode
+            on_press=modified_key._handle_press,
+            on_release=modified_key._handle_release,
+            meta=modified_key.meta,
+        )
 
     def __repr__(self):
         return 'ModifierKey(code={}, has_modifiers={})'.format(

tests/test_kmk_keys.py

@@ -0,0 +1,87 @@
+import unittest
+
+from kmk.keys import KC, Key, ModifierKey
+from tests.keyboard_test import KeyboardTest
+
+
+class TestKmkKeys(unittest.TestCase):
+    def test_basic_kmk_keyboard(self):
+        keyboard = KeyboardTest(
+            [],
+            [
+                [
+                    KC.HASH,
+                    KC.RALT(KC.HASH),
+                    KC.RALT(KC.LSFT)(KC.N3),
+                    KC.RALT(KC.LSFT),
+                    KC.RALT,
+                ]
+            ],
+        )
+        keyboard.test(
+            'Shifted key',
+            [(0, True), (0, False)],
+            [
+                {
+                    KC.N3,
+                    KC.LSFT,
+                },
+                {},
+            ],
+        )
+        keyboard.test(
+            'Shift+AltGr+key',
+            [(1, True), (1, False)],
+            [
+                {
+                    KC.N3,
+                    KC.LSFT,
+                    KC.RALT,
+                },
+                {},
+            ],
+        )
+        keyboard.test(
+            'Shift+AltGr+key, alternate chaining',
+            [(2, True), (2, False)],
+            [
+                {
+                    KC.N3,
+                    KC.LSFT,
+                    KC.RALT,
+                },
+                {},
+            ],
+        )
+        keyboard.test(
+            'Shift+AltGr',
+            [(3, True), (3, False)],
+            [
+                {
+                    KC.LSFT,
+                    KC.RALT,
+                },
+                {},
+            ],
+        )
+        keyboard.test(
+            'AltGr',
+            [(4, True), (4, False)],
+            [
+                {
+                    KC.RALT,
+                },
+                {},
+            ],
+        )
+
+        assert isinstance(KC.RGUI(no_press=True), ModifierKey)
+        assert isinstance(KC.RALT(KC.RGUI), ModifierKey)
+        assert isinstance(KC.Q(no_press=True), Key)
+        assert not isinstance(KC.Q(no_press=True), ModifierKey)
+        assert isinstance(KC.RALT(KC.Q), Key)
+        assert not isinstance(KC.RALT(KC.Q), ModifierKey)
+
+
+if __name__ == '__main__':
+    unittest.main()