From 032ebd09c6df0f9ba1c0c8436c22f6973c277ce2 Mon Sep 17 00:00:00 2001
From: sezanzeb <proxima@hip70890b.de>
Date: Fri, 4 Dec 2020 14:52:08 +0100
Subject: [PATCH] improved macro parsing error logging

---
 keymapper/dev/injector.py      |  7 +++++--
 keymapper/dev/macros.py        | 20 ++++++++++++++++++++
 tests/testcases/test_macros.py |  5 +++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/keymapper/dev/injector.py b/keymapper/dev/injector.py
index 8be8ad84..363524e7 100644
--- a/keymapper/dev/injector.py
+++ b/keymapper/dev/injector.py
@@ -294,8 +294,11 @@ class KeycodeInjector:
             macros = {}
             for (ev_type, keycode), output in self.mapping:
                 if is_this_a_macro(output):
-                    macros[keycode] = parse(output)
-                    continue
+                    macro = parse(output)
+                    if macro is None:
+                        continue
+
+                    macros[keycode] = macro
 
             # certain capabilities can have side effects apparently. with an
             # EV_ABS capability, EV_REL won't move the mouse pointer anymore.
diff --git a/keymapper/dev/macros.py b/keymapper/dev/macros.py
index 72d8acae..d661a27f 100644
--- a/keymapper/dev/macros.py
+++ b/keymapper/dev/macros.py
@@ -119,6 +119,12 @@ class _Macro:
         modifier : str
         macro : _Macro
         """
+        if not isinstance(macro, _Macro):
+            raise ValueError(
+                'Expected the second param for repeat to be '
+                f'a macro, but got {macro}'
+            )
+
         modifier = str(modifier)
         code = system_mapping.get(modifier)
 
@@ -143,7 +149,14 @@ class _Macro:
         repeats : int
         macro : _Macro
         """
+        if not isinstance(macro, _Macro):
+            raise ValueError(
+                'Expected the second param for repeat to be '
+                f'a macro, but got "{macro}"'
+            )
+
         repeats = int(repeats)
+
         for _ in range(repeats):
             self.tasks.append((CHILD_MACRO, macro))
         return self
@@ -310,9 +323,12 @@ def _parse_recurse(macro, macro_instance=None, depth=0):
 
     # probably a parameter for an outer function
     try:
+        # if possible, parse as int
         macro = int(macro)
     except ValueError:
+        # use as string instead
         pass
+
     logger.spam('%s%s %s', space, type(macro), macro)
     return macro
 
@@ -320,6 +336,10 @@ def _parse_recurse(macro, macro_instance=None, depth=0):
 def parse(macro):
     """parse and generate a _Macro that can be run as often as you want.
 
+    You need to use set_handler on it before running. If it could not
+    be parsed, possibly due to syntax errors, will log the error and
+    return None.
+
     Parameters
     ----------
     macro : string
diff --git a/tests/testcases/test_macros.py b/tests/testcases/test_macros.py
index de476e10..b5ec6eed 100644
--- a/tests/testcases/test_macros.py
+++ b/tests/testcases/test_macros.py
@@ -52,6 +52,11 @@ class TestMacros(unittest.TestCase):
         self.loop.run_until_complete(macro.run())
         self.assertListEqual(self.result, [(one_code, 1), (one_code, 0)])
 
+    def test_fails(self):
+        self.assertIsNone(parse('r(1, a)'))
+        self.assertIsNone(parse('r(a, k(b))'))
+        self.assertIsNone(parse('m(a, b)'))
+
     def test_0(self):
         macro = parse('k(1)')
         macro.set_handler(self.handler)