showing syntax errors in the ui

README.md

@@ -32,8 +32,8 @@ Documentation:
 - `h` executes the parameter as long as the key is pressed down
 - `.` executes two actions behind each other
 
-Syntax errors are logged to the console. each `k` function adds a short delay
-of 10ms that can be configured in `~/.config/key-mapper/config`.
+Syntax errors are shown in the ui. each `k` function adds a short delay of
+10ms that can be configured in `~/.config/key-mapper/config`.
 
 ##### Names
 

keymapper/dev/keycode_mapper.py

@@ -95,10 +95,12 @@ def handle_keycode(code_to_code, macros, event, uinput):
             # make sure that a duplicate key-down event won't make a
             # macro with a hold function run forever. there should always
             # be only one active.
-            # TODO test, throw in a ton of key-down events and one key up
-            #  event and check that no macro is writing stuff
+            # TODO test, throw in a ton of key-down events of various codes
+            #  and one key up event and check that no macro is writing stuff
             existing_macro.release_key()
 
+        # TODO test holding down two macros
+
         macro = macros[input_keycode]
         active_macros[input_keycode] = macro
         # TODO test that holding is true

keymapper/dev/macros.py

@@ -53,6 +53,10 @@ DEBUG = 6
 
 def is_this_a_macro(output):
     """Figure out if this is a macro."""
+    # TODO test
+    if not isinstance(output, str):
+        return False
+
     return '(' in output and ')' in output and len(output) >= 4
 
 
@@ -114,6 +118,7 @@ class _Macro:
     def press_key(self):
         """Tell all child macros that the key was pressed down."""
         # TODO test
+        print(id(self), 'hold')
         self.holding = True
         for macro in self.child_macros:
             macro.press_key()
@@ -121,6 +126,7 @@ class _Macro:
     def release_key(self):
         """Tell all child macros that the key was released."""
         # TODO test
+        print(id(self), 'release')
         self.holding = False
         for macro in self.child_macros:
             macro.release_key()
@@ -131,8 +137,8 @@ class _Macro:
         #  even with complicated macros and weird calls to press and release
         if not isinstance(macro, _Macro):
             raise ValueError(
-                'Expected the param for hold to be '
-                f'a macro, but got "{macro}"'
+                'Expected the param for h (hold) to be '
+                f'a macro (like k(a)), but got "{macro}"'
             )
 
         async def task():
@@ -155,8 +161,8 @@ class _Macro:
         """
         if not isinstance(macro, _Macro):
             raise ValueError(
-                'Expected the second param for repeat to be '
-                f'a macro, but got {macro}'
+                'Expected the second param for m (modify) to be '
+                f'a macro (like k(a)), but got {macro}'
             )
 
         modifier = str(modifier)
@@ -187,15 +193,15 @@ class _Macro:
         """
         if not isinstance(macro, _Macro):
             raise ValueError(
-                'Expected the second param for repeat to be '
-                f'a macro, but got "{macro}"'
+                'Expected the second param for r (repeat) to be '
+                f'a macro (like k(a)), but got "{macro}"'
             )
 
         try:
             repeats = int(repeats)
         except ValueError:
             raise ValueError(
-                'Expected the first param for repeat to be '
+                'Expected the first param for r (repeat) to be '
                 f'a number, but got "{repeats}"'
             )
 
@@ -237,7 +243,7 @@ class _Macro:
             sleeptime = int(sleeptime)
         except ValueError:
             raise ValueError(
-                'Expected the param for wait to be '
+                'Expected the param for w (wait) to be '
                 f'a number, but got "{sleeptime}"'
             )
 
@@ -339,6 +345,10 @@ def _parse_recurse(macro, macro_instance=None, depth=0):
     call_match = re.match(r'^(\w+)\(', macro)
     call = call_match[1] if call_match else None
     if call is not None:
+        if 'k(' not in macro:
+            # TODO test
+            raise Exception(f'"{macro}" doesn\'t write any keys')
+
         # available functions in the macro and the number of their
         # parameters
         functions = {
@@ -396,7 +406,7 @@ def _parse_recurse(macro, macro_instance=None, depth=0):
     return macro
 
 
-def parse(macro):
+def parse(macro, return_errors=False):
     """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
@@ -409,7 +419,10 @@ def parse(macro):
         "r(3, k(a).w(10))"
         "r(2, k(a).k(-)).k(b)"
         "w(1000).m(Shift_L, r(2, k(a))).w(10, 20).k(b)"
+    return_errors : bool
+        if True, returns errors as a string or None if parsing worked
     """
+    # TODO test return_errors
     # whitespaces, tabs, newlines and such don't serve a purpose. make
     # the log output clearer and the parsing easier.
     macro = re.sub(r'\s', '', macro)
@@ -418,9 +431,14 @@ def parse(macro):
         logger.info('Quotation marks in macros are not needed')
         macro = macro.replace('"', '').replace("'", '')
 
-    logger.spam('preparing macro %s for later execution', macro)
+    if return_errors:
+        logger.spam('checking the syntax of %s', macro)
+    else:
+        logger.spam('preparing macro %s for later execution', macro)
+
     try:
-        return _parse_recurse(macro)
+        macro_object = _parse_recurse(macro)
+        return macro_object if not return_errors else None
     except Exception as error:
         logger.error('Failed to parse macro "%s": %s', macro, error)
-        return None
+        return str(error) if return_errors else None

keymapper/gtk/window.py

@@ -37,6 +37,7 @@ from keymapper.gtk.unsaved import unsaved_changes_dialog, GO_BACK
 from keymapper.dev.reader import keycode_reader
 from keymapper.daemon import get_dbus_interface
 from keymapper.config import config
+from keymapper.dev.macros import is_this_a_macro, parse
 from keymapper.dev.permissions import can_read_devices
 
 
@@ -293,11 +294,36 @@ class Window:
         )
         GLib.timeout_add(10, self.show_device_mapping_status)
 
+    def show_status(self, context_id, message, tooltip=None):
+        """Show a status message and set its tooltip."""
+        if tooltip is None:
+            tooltip = message
+
+        status_bar = self.get('status_bar')
+        status_bar.push(context_id, message)
+        status_bar.set_tooltip_text(tooltip)
+
+    def check_macro_syntax(self):
+        """Check if the programmed macros are allright."""
+        # test macros for syntax errors
+        # TODO test
+        for (ev_type, keycode), output in custom_mapping:
+            if not is_this_a_macro(output):
+                continue
+
+            error = parse(output, return_errors=True)
+            if error is None:
+                continue
+
+            position = to_string(ev_type, keycode)
+            msg = f'Syntax error at {position}, hover for info'
+            self.show_status(CTX_ERROR, msg, error)
+
     def on_save_preset_clicked(self, button):
         """Save changes to a preset to the file system."""
         new_name = self.get('preset_name_input').get_text()
         try:
-            self.save_config()
+            self.save_preset()
             if new_name not in ['', self.selected_preset]:
                 rename_preset(
                     self.selected_device,
@@ -308,15 +334,11 @@ class Window:
             # newest, so populate_presets will automatically select the
             # right one again.
             self.populate_presets()
-            self.get('status_bar').push(
-                CTX_SAVE,
-                f'Saved "{self.selected_preset}"'
-            )
+            self.show_status(CTX_SAVE, f'Saved "{self.selected_preset}"')
+            self.check_macro_syntax()
+
         except PermissionError as error:
-            self.get('status_bar').push(
-                CTX_ERROR,
-                'Error: Permission denied!'
-            )
+            self.show_status(CTX_ERROR, 'Error: Permission denied!')
             logger.error(str(error))
 
     def on_delete_preset_clicked(self, _):
@@ -331,11 +353,10 @@ class Window:
 
         logger.debug('Applying preset "%s" for "%s"', preset, device)
 
-        push = self.get('status_bar').push
         if custom_mapping.changed:
-            push(CTX_APPLY, f'Applied outdated preset "{preset}"')
+            self.show_status(CTX_APPLY, f'Applied outdated preset "{preset}"')
         else:
-            push(CTX_APPLY, f'Applied preset "{preset}"')
+            self.show_status(CTX_APPLY, f'Applied preset "{preset}"')
 
         success = self.dbus.start_injecting(
             self.selected_device,
@@ -343,10 +364,7 @@ class Window:
         )
 
         if not success:
-            self.get('status_bar').push(
-                CTX_ERROR,
-                'Error: Could not grab devices!'
-            )
+            self.show_status(CTX_ERROR, 'Error: Could not grab devices!')
 
         # restart reading because after injecting the device landscape
         # changes a bit
@@ -406,10 +424,7 @@ class Window:
             self.get('preset_selection').append(new_preset, new_preset)
             self.get('preset_selection').set_active_id(new_preset)
         except PermissionError as error:
-            self.get('status_bar').push(
-                CTX_ERROR,
-                'Error: Permission denied!'
-            )
+            self.show_status(CTX_ERROR, 'Error: Permission denied!')
             logger.error(str(error))
 
     def on_select_preset(self, dropdown):
@@ -469,8 +484,8 @@ class Window:
         # https://stackoverflow.com/a/30329591/4417769
         key_list.remove(single_key_mapping)
 
-    def save_config(self):
-        """Write changes to disk."""
+    def save_preset(self):
+        """Write changes to presets to disk."""
         if self.selected_device is None or self.selected_preset is None:
             return