Hopefully fix plugins not actually binding keyboard shortcuts.

src/cthulhu/cthulhu.py

@@ -34,108 +34,6 @@ __copyright__ = "Copyright (c) 2004-2009 Sun Microsystems Inc." \
 __license__   = "LGPL"
 
 import faulthandler
-
-class APIHelper:
-    """Helper class for plugin API interactions, including keybindings."""
-    
-    def __init__(self, app):
-        """Initialize the APIHelper.
-        
-        Arguments:
-        - app: the Cthulhu application
-        """
-        self.app = app
-        self._gestureBindings = {}
-        
-    def registerGestureByString(self, function, name, gestureString, 
-                                inputEventType='default', normalizer='cthulhu',
-                                learnModeEnabled=True, contextName=None):
-        """Register a gesture by string.
-        
-        Arguments:
-        - function: the function to call when the gesture is performed
-        - name: a human-readable name for this gesture
-        - gestureString: string representation of the gesture (e.g., 'kb:cthulhu+z')
-        - inputEventType: the type of input event
-        - normalizer: the normalizer to use
-        - learnModeEnabled: whether this should be available in learn mode
-        - contextName: the context for this gesture (e.g., plugin name)
-        
-        Returns the binding ID or None if registration failed
-        """
-        if not gestureString.startswith("kb:"):
-            return None
-            
-        # Extract the key portion from the gesture string
-        key = gestureString.split(":", 1)[1]
-        
-        # Handle Cthulhu modifier specially
-        if "cthulhu+" in key.lower():
-            from . import keybindings
-            key_parts = key.lower().split("+")
-            
-            # Determine appropriate modifier mask
-            modifiers = keybindings.CTHULHU_MODIFIER_MASK
-            
-            # Extract the final key (without modifiers)
-            final_key = key_parts[-1]
-            
-            # Check for additional modifiers
-            if "shift" in key_parts:
-                modifiers = keybindings.CTHULHU_SHIFT_MODIFIER_MASK
-            elif "ctrl" in key_parts or "control" in key_parts:
-                modifiers = keybindings.CTHULHU_CTRL_MODIFIER_MASK
-            elif "alt" in key_parts:
-                modifiers = keybindings.CTHULHU_ALT_MODIFIER_MASK
-                
-            # Create a keybinding handler
-            class GestureHandler:
-                def __init__(self, function, description):
-                    self.function = function
-                    self.description = description
-                
-                def __call__(self, script, inputEvent):
-                    return self.function(script, inputEvent)
-            
-            handler = GestureHandler(function, name)
-            
-            # Register the binding with the active script
-            from . import cthulhu_state
-            if cthulhu_state.activeScript:
-                bindings = cthulhu_state.activeScript.getKeyBindings()
-                binding = keybindings.KeyBinding(
-                    final_key, 
-                    keybindings.defaultModifierMask,
-                    modifiers,
-                    handler)
-                bindings.add(binding)
-                
-                # Store binding for later reference
-                if contextName not in self._gestureBindings:
-                    self._gestureBindings[contextName] = []
-                self._gestureBindings[contextName].append(binding)
-                
-                return binding
-        
-        return None
-        
-    def unregisterShortcut(self, binding, contextName=None):
-        """Unregister a previously registered shortcut.
-        
-        Arguments:
-        - binding: the binding to unregister
-        - contextName: the context for this gesture
-        """
-        # Remove from script's keybindings
-        from . import cthulhu_state
-        if cthulhu_state.activeScript:
-            bindings = cthulhu_state.activeScript.getKeyBindings()
-            bindings.remove(binding)
-            
-        # Remove from our tracking
-        if contextName in self._gestureBindings:
-            if binding in self._gestureBindings[contextName]:
-                self._gestureBindings[contextName].remove(binding)
 import gi
 import importlib
 import os
@@ -190,6 +88,250 @@ from . import translation_manager
 from . import resource_manager
 
 
+class APIHelper:
+    """Helper class for plugin API interactions, including keybindings."""
+    
+    def __init__(self, app):
+        """Initialize the APIHelper.
+        
+        Arguments:
+        - app: the Cthulhu application
+        """
+        self.app = app
+        self._gestureBindings = {}  # contextName -> [bindings]
+        
+        # Register for script change notifications
+        self.app.getSignalManager().connectSignal(
+            'active-script-changed', 
+            self._handleScriptChanged
+        )
+        
+    def _handleScriptChanged(self, script):
+        """Handle when the active script changes by updating bindings.
+        
+        Arguments:
+        - script: the new active script
+        """
+        if not script:
+            return
+            
+        # Sync all known bindings with the new script
+        self._syncBindingsWithScript(script)
+    
+    def _syncBindingsWithScript(self, script):
+        """Sync all registered bindings with the given script.
+        
+        Arguments:
+        - script: the script to sync bindings with
+        """
+        from . import debug
+        if not script:
+            return
+            
+        debug.printMessage(debug.LEVEL_INFO, f"Syncing {sum(len(v) for v in self._gestureBindings.values())} bindings to script {script}", True)
+            
+        # Get the script's key bindings
+        try:
+            bindings = script.getKeyBindings()
+            if not bindings:
+                debug.printMessage(debug.LEVEL_WARNING, "Script has no key bindings manager", True)
+                return
+                
+            # First, remove any plugin bindings that might already exist
+            # (in case we're re-registering with the same script)
+            for contextName, contextBindings in self._gestureBindings.items():
+                for binding in contextBindings:
+                    try:
+                        bindings.remove(binding)
+                    except:
+                        # Binding might not be in this script, that's OK
+                        pass
+            
+            # Then add all our stored bindings to the script
+            for contextName, contextBindings in self._gestureBindings.items():
+                for binding in contextBindings:
+                    try:
+                        bindings.add(binding)
+                        debug.printMessage(debug.LEVEL_INFO, f"Added binding for {binding.handler.description} to script", True)
+                    except Exception as e:
+                        debug.printMessage(debug.LEVEL_WARNING, f"Error adding binding to script: {e}", True)
+        except Exception as e:
+            debug.printMessage(debug.LEVEL_WARNING, f"Failed to sync bindings with script: {e}", True)
+        
+    def registerGestureByString(self, function, name, gestureString, 
+                                inputEventType='default', normalizer='cthulhu',
+                                learnModeEnabled=True, contextName=None):
+        """Register a gesture by string.
+        
+        Arguments:
+        - function: the function to call when the gesture is performed
+        - name: a human-readable name for this gesture
+        - gestureString: string representation of the gesture (e.g., 'kb:cthulhu+z')
+        - inputEventType: the type of input event
+        - normalizer: the normalizer to use
+        - learnModeEnabled: whether this should be available in learn mode
+        - contextName: the context for this gesture (e.g., plugin name)
+        
+        Returns the binding ID or None if registration failed
+        """
+        from . import debug
+        debug.printMessage(debug.LEVEL_INFO, f"Registering gesture: {gestureString} for context: {contextName}", True)
+        
+        if not gestureString.startswith("kb:"):
+            debug.printMessage(debug.LEVEL_WARNING, f"Invalid gesture string format: {gestureString}", True)
+            return None
+            
+        # Extract the key portion from the gesture string
+        key = gestureString.split(":", 1)[1]
+        
+        # Handle Cthulhu modifier specially
+        if "cthulhu+" in key.lower():
+            from . import keybindings
+            key_parts = key.lower().split("+")
+            
+            # Determine appropriate modifier mask
+            modifiers = keybindings.CTHULHU_MODIFIER_MASK
+            
+            # Extract the final key (without modifiers)
+            final_key = key_parts[-1]
+            
+            # Check for additional modifiers
+            if "shift" in key_parts:
+                modifiers = keybindings.CTHULHU_SHIFT_MODIFIER_MASK
+            elif "ctrl" in key_parts or "control" in key_parts:
+                modifiers = keybindings.CTHULHU_CTRL_MODIFIER_MASK
+            elif "alt" in key_parts:
+                modifiers = keybindings.CTHULHU_ALT_MODIFIER_MASK
+                
+            # Create a keybinding handler
+            class GestureHandler:
+                def __init__(self, function, description):
+                    self.function = function
+                    self.description = description
+                
+                def __call__(self, script, inputEvent):
+                    return self.function(script, inputEvent)
+            
+            handler = GestureHandler(function, name)
+            
+            # Create the binding
+            binding = keybindings.KeyBinding(
+                final_key, 
+                keybindings.defaultModifierMask,
+                modifiers,
+                handler)
+                
+            # Store binding for later reference (do this BEFORE registering with script)
+            if not contextName:
+                debug.printMessage(debug.LEVEL_WARNING, "No context name provided, using 'default'", True)
+                contextName = "default"
+                
+            if contextName not in self._gestureBindings:
+                self._gestureBindings[contextName] = []
+            self._gestureBindings[contextName].append(binding)
+                
+            # Register with active script if available
+            from . import cthulhu_state
+            if cthulhu_state.activeScript:
+                try:
+                    bindings = cthulhu_state.activeScript.getKeyBindings()
+                    if bindings:
+                        bindings.add(binding)
+                        debug.printMessage(debug.LEVEL_INFO, 
+                            f"Added binding {binding} to active script {cthulhu_state.activeScript}", True)
+                except Exception as e:
+                    debug.printMessage(debug.LEVEL_WARNING, 
+                        f"Failed to add binding to active script: {e}", True)
+            
+            return binding
+        
+        debug.printMessage(debug.LEVEL_WARNING, f"Gesture doesn't use Cthulhu modifier: {gestureString}", True)
+        return None
+        
+    def unregisterShortcut(self, binding, contextName=None):
+        """Unregister a previously registered shortcut.
+        
+        Arguments:
+        - binding: the binding to unregister
+        - contextName: the context for this gesture
+        """
+        from . import debug
+        
+        # For compatibility with old code that passes a string instead of a binding
+        if isinstance(binding, str):
+            debug.printMessage(debug.LEVEL_WARNING, 
+                f"Deprecated: unregisterShortcut called with string '{binding}' instead of binding object", True)
+            # Try to find the binding by its description in all context bindings
+            found = False
+            for ctx, bindings in self._gestureBindings.items():
+                for b in bindings:
+                    if binding in b.handler.description:
+                        debug.printMessage(debug.LEVEL_INFO, 
+                            f"Found binding {b} matching '{binding}', removing", True)
+                        self.unregisterShortcut(b, ctx)
+                        found = True
+            if not found and contextName:
+                # If we have a context, try to clean up all bindings for that context
+                debug.printMessage(debug.LEVEL_INFO, 
+                    f"Could not find binding for '{binding}', unregistering all for context {contextName}", True)
+                self.unregisterAllForContext(contextName)
+            return
+            
+        debug.printMessage(debug.LEVEL_INFO, f"Unregistering shortcut for context: {contextName}", True)
+        
+        # Remove from all scripts
+        from . import script_manager
+        for script in script_manager.getManager().getAllScripts():
+            try:
+                bindings = script.getKeyBindings()
+                if bindings and binding in bindings:
+                    bindings.remove(binding)
+                    debug.printMessage(debug.LEVEL_INFO, f"Removed binding from script: {script}", True)
+            except Exception as e:
+                debug.printMessage(debug.LEVEL_WARNING, f"Error removing binding from script {script}: {e}", True)
+            
+        # Remove from active script explicitly (in case it wasn't in getAllScripts)
+        from . import cthulhu_state
+        if cthulhu_state.activeScript:
+            try:
+                bindings = cthulhu_state.activeScript.getKeyBindings()
+                if bindings:
+                    bindings.remove(binding)
+            except Exception as e:
+                debug.printMessage(debug.LEVEL_WARNING, 
+                    f"Error removing binding from active script: {e}", True)
+            
+        # Remove from our tracking
+        if contextName in self._gestureBindings:
+            if binding in self._gestureBindings[contextName]:
+                self._gestureBindings[contextName].remove(binding)
+                debug.printMessage(debug.LEVEL_INFO, 
+                    f"Removed binding from _gestureBindings for context: {contextName}", True)
+                
+    def unregisterAllForContext(self, contextName):
+        """Unregister all shortcuts for a specific context.
+        
+        Arguments:
+        - contextName: the context to unregister all shortcuts for
+        """
+        from . import debug
+        debug.printMessage(debug.LEVEL_INFO, f"Unregistering all shortcuts for context: {contextName}", True)
+        
+        if contextName not in self._gestureBindings:
+            debug.printMessage(debug.LEVEL_INFO, f"No bindings found for context: {contextName}", True)
+            return
+            
+        # Get a copy of the bindings
+        bindings = self._gestureBindings[contextName].copy()
+        
+        # Unregister each binding
+        for binding in bindings:
+            self.unregisterShortcut(binding, contextName)
+            
+        # Ensure the context is empty
+        self._gestureBindings[contextName] = []
+
+
 _eventManager = event_manager.getManager()
 _scriptManager = script_manager.getManager()
 _settingsManager = settings_manager.getManager()
@@ -1017,7 +1159,9 @@ class Cthulhu(GObject.Object):
         "setup-inputeventhandlers-completed": (GObject.SignalFlags.RUN_LAST, None, ()), # compat signal for register input event handlers
         "request-cthulhu-preferences": (GObject.SignalFlags.RUN_LAST, None, ()),
         "request-application-preferences": (GObject.SignalFlags.RUN_LAST, None, ()),
+        "active-script-changed": (GObject.SignalFlags.RUN_LAST, None, (object,))
     }
+
     def __init__(self):
         GObject.Object.__init__(self)
         # add members
@@ -1029,11 +1173,38 @@ class Cthulhu(GObject.Object):
         self.dynamicApiManager = dynamic_api_manager.DynamicApiManager(self)
         self.translationManager = translation_manager.TranslationManager(self)
         self.debugManager = debug
+        self._setupScriptChangeMonitoring()
         self.APIHelper = APIHelper(self)
         self.createCompatAPI()
         self.pluginSystemManager = plugin_system_manager.PluginSystemManager(self)
         # Scan for available plugins at startup
         self.pluginSystemManager.rescanPlugins()
+
+    def _setupScriptChangeMonitoring(self):
+        """Set up monitoring for script changes to emit signals."""
+        from . import script_manager
+        mgr = script_manager.getManager()
+    
+        # Save the original setActiveScript method
+        original_setActiveScript = mgr.setActiveScript
+    
+        # Create a replacement that emits our signal
+
+        def setActiveScript_with_signal(script, reason=None):
+            result = original_setActiveScript(script, reason)
+            # Emit our signal with the new script
+            try:
+                self.emit("active-script-changed", script)
+                from . import debug
+                debug.printMessage(debug.LEVEL_INFO, f"Emitted active-script-changed signal with {script}", True)
+            except Exception as e:
+                from . import debug
+                debug.printMessage(debug.LEVEL_WARNING, f"Error emitting script change signal: {e}", True)
+            return result
+    
+        # Replace the method
+        mgr.setActiveScript = setActiveScript_with_signal
+
     def getAPIHelper(self):
         return self.APIHelper
     def getPluginSystemManager(self):

src/cthulhu/plugin.py

@@ -47,6 +47,7 @@ class Plugin:
         self.name = ''
         self.version = ''
         self.description = ''
+        self._registered_bindings = []  # Track registered bindings for cleanup
 
     def set_app(self, app):
         """Set the application reference."""
@@ -73,14 +74,31 @@ class Plugin:
         """Deactivate the plugin. Override this in subclasses."""
         if plugin is not None and plugin is not self:
             return
+            
         logger.info(f"Deactivating plugin: {self.name}")
         
-    def registerGestureByString(self, function, name, gestureString, learnModeEnabled=True):
+        # Clean up all registered shortcuts
-        """Register a gesture by string."""
         if self.app:
             api_helper = self.app.getAPIHelper()
             if api_helper:
-                return api_helper.registerGestureByString(
+                api_helper.unregisterAllForContext(self.module_name)
+                self._registered_bindings = []  # Clear our binding list
+
+    def registerGestureByString(self, function, name, gestureString, learnModeEnabled=True):
+        """Register a gesture by string.
+        
+        Arguments:
+        - function: the function to call when the gesture is performed
+        - name: a human-readable name for this gesture
+        - gestureString: string representation of the gesture (e.g., 'kb:cthulhu+z')
+        - learnModeEnabled: whether this should be available in learn mode
+        
+        Returns the binding object or None if registration failed
+        """
+        if self.app:
+            api_helper = self.app.getAPIHelper()
+            if api_helper:
+                binding = api_helper.registerGestureByString(
                     function, 
                     name, 
                     gestureString, 
@@ -89,4 +107,40 @@ class Plugin:
                     learnModeEnabled, 
                     contextName=self.module_name
                 )
+                
+                if binding and binding not in self._registered_bindings:
+                    self._registered_bindings.append(binding)
+                    
+                return binding
+                
+        logger.warning(f"Could not register gesture: {gestureString}")
         return None
+        
+    def unregisterGestureByString(self, gestureString):
+        """Backwards compatibility method to unregister a gesture by string.
+        
+        This method is provided for compatibility with existing plugins,
+        but new plugins should store and use the binding objects.
+        
+        Arguments:
+        - gestureString: the gesture string to unregister
+        """
+        if self.app:
+            api_helper = self.app.getAPIHelper()
+            if api_helper:
+                # We'll let the APIHelper try to find the binding
+                api_helper.unregisterShortcut(gestureString, self.module_name)
+                
+    def unregisterBinding(self, binding):
+        """Unregister a specific binding.
+        
+        Arguments:
+        - binding: the binding object to unregister
+        """
+        if binding in self._registered_bindings:
+            self._registered_bindings.remove(binding)
+            
+        if self.app:
+            api_helper = self.app.getAPIHelper()
+            if api_helper:
+                api_helper.unregisterShortcut(binding, self.module_name)

src/cthulhu/plugin_system_manager.py

@@ -431,6 +431,7 @@ class PluginSystemManager:
             return False
 
         module_name = pluginInfo.get_module_name()
+        logger.info(f"Unloading plugin: {module_name}")
 
         try:
             # Not loaded?
@@ -441,22 +442,43 @@ class PluginSystemManager:
             plugin_instance = pluginInfo.instance
             if plugin_instance:
                 try:
+                    logger.info(f"Calling deactivate hook for plugin: {module_name}")
                     self.plugin_manager.hook.deactivate(plugin=plugin_instance)
                 except Exception as e:
                     logger.error(f"Error deactivating plugin {module_name}: {e}")
+                    import traceback
+                    logger.error(traceback.format_exc())
+    
+                # Make sure any registered bindings are cleaned up
+                try:
+                    if self.getApp():
+                        api_helper = self.getApp().getAPIHelper()
+                        if api_helper:
+                            logger.info(f"Cleaning up any remaining bindings for {module_name}")
+                            api_helper.unregisterAllForContext(module_name)
+                except Exception as e:
+                    logger.error(f"Failed to clean up bindings for {module_name}: {e}")
+                    import traceback
+                    logger.error(traceback.format_exc())
 
                 # Unregister from pluggy
+                try:
                     self.plugin_manager.unregister(plugin_instance)
+                    logger.info(f"Unregistered plugin {module_name} from pluggy")
+                except Exception as e:
+                    logger.error(f"Error unregistering plugin from pluggy: {e}")
     
             # Clean up
             pluginInfo.instance = None
             pluginInfo.loaded = False
 
-            logger.info(f"Unloaded plugin: {module_name}")
+            logger.info(f"Successfully unloaded plugin: {module_name}")
             return True
 
         except Exception as e:
             logger.error(f"Failed to unload plugin {module_name}: {e}")
+            import traceback
+            logger.error(traceback.format_exc())
             return False
 
     def unloadAllPlugins(self, ForceAllPlugins=False):