kicad-developers team mailing list archive

[Ian McInerney <Ian.S.McInerney@xxxxxxxx>]

This patch modifies the event dispatch in the tool manager to return
whether a hotkey has been handled with an action (instead of just returning
true if an action with the hotkey exists). This will allow the handler to
continue processing the event if the action with the hotkey does not have
any active transitions.

This is something that was discussed on the list before with the hotkey
handling (https://lists.launchpad.net/kicad-developers/msg41725.html), and
it seemed the discussion said this was a change that should be made (and
then fix any fallout from it). I don't think this will break anything, but
it will make more keyboard events being passed into the interactive tools.

-Ian

From d1a5e31a7349152bddb029d14bae68fd1983a6bf Mon Sep 17 00:00:00 2001
From: Ian McInerney <Ian.S.McInerney@xxxxxxxx>
Date: Mon, 5 Aug 2019 14:19:44 +0200
Subject: [PATCH] Return handled status for actions run from hotkeys

---
 common/tool/action_manager.cpp |  6 ++----
 common/tool/tool_manager.cpp   | 37 ++++++++++++++++++++--------------
 include/tool/tool_manager.h    | 21 +++++++++++--------
 3 files changed, 37 insertions(+), 27 deletions(-)

diff --git a/common/tool/action_manager.cpp b/common/tool/action_manager.cpp
index 9e26c49b4..96e1eabd0 100644
--- a/common/tool/action_manager.cpp
+++ b/common/tool/action_manager.cpp
@@ -152,8 +152,7 @@ bool ACTION_MANAGER::RunHotKey( int aHotKey ) const
                 "ACTION_MANAGER::RunHotKey Running action %s for hotkey %s", context->GetName(),
                 KeyNameFromKeyCode( aHotKey ) );
 
-        m_toolMgr->RunAction( *context, true );
-        return true;
+        return m_toolMgr->RunAction( *context, true );
     }
     else if( global )
     {
@@ -161,8 +160,7 @@ bool ACTION_MANAGER::RunHotKey( int aHotKey ) const
                 "ACTION_MANAGER::RunHotKey Running action: %s for hotkey %s", global->GetName(),
                 KeyNameFromKeyCode( aHotKey ) );
 
-        m_toolMgr->RunAction( *global, true );
-        return true;
+        return m_toolMgr->RunAction( *global, true );
     }
 
     wxLogTrace( kicadTraceToolStack, "ACTION_MANAGER::RunHotKey No action found for key %s",
diff --git a/common/tool/tool_manager.cpp b/common/tool/tool_manager.cpp
index 71870b267..f56f6ce37 100644
--- a/common/tool/tool_manager.cpp
+++ b/common/tool/tool_manager.cpp
@@ -278,8 +278,9 @@ bool TOOL_MANAGER::RunAction( const std::string& aActionName, bool aNow, void* a
 }
 
 
-void TOOL_MANAGER::RunAction( const TOOL_ACTION& aAction, bool aNow, void* aParam )
+bool TOOL_MANAGER::RunAction( const TOOL_ACTION& aAction, bool aNow, void* aParam )
 {
+    bool       handled = false;
     TOOL_EVENT event = aAction.MakeEvent();
 
     // Allow to override the action parameter
@@ -289,7 +290,7 @@ void TOOL_MANAGER::RunAction( const TOOL_ACTION& aAction, bool aNow, void* aPara
     if( aNow )
     {
         TOOL_STATE* current = m_activeState;
-        processEvent( event );
+        handled = processEvent( event );
         setActiveState( current );
         UpdateUI( event );
     }
@@ -297,6 +298,8 @@ void TOOL_MANAGER::RunAction( const TOOL_ACTION& aAction, bool aNow, void* aPara
     {
         PostEvent( event );
     }
+
+    return handled;
 }
 
 
@@ -956,22 +959,26 @@ bool TOOL_MANAGER::processEvent( const TOOL_EVENT& aEvent )
 {
     wxLogTrace( kicadTraceToolStack, "TOOL_MANAGER::processEvent %s", aEvent.Format() );
 
-    if( dispatchHotKey( aEvent ) )
-        return true;
-
-    bool handled = false;
+    // First try to dispatch the action associated with the event if it is a key press event
+    bool handled = dispatchHotKey( aEvent );
 
-    handled |= dispatchInternal( aEvent );
-    handled |= dispatchActivation( aEvent );
+    if( !handled )
+    {
+        // If the event is not handled through a hotkey activation, pass it to the currently
+        // running tool loops
+        handled |= dispatchInternal( aEvent );
+        handled |= dispatchActivation( aEvent );
 
-    DispatchContextMenu( aEvent );
+        // Open the context menu if requested by a tool
+        DispatchContextMenu( aEvent );
 
-    // Dispatch queue
-    while( !m_eventQueue.empty() )
-    {
-        TOOL_EVENT event = m_eventQueue.front();
-        m_eventQueue.pop_front();
-        processEvent( event );
+        // Dispatch any remaining events in the event queue
+        while( !m_eventQueue.empty() )
+        {
+            TOOL_EVENT event = m_eventQueue.front();
+            m_eventQueue.pop_front();
+            processEvent( event );
+        }
     }
 
     wxLogTrace( kicadTraceToolStack, "TOOL_MANAGER::processEvent handled: %s  %s",
diff --git a/include/tool/tool_manager.h b/include/tool/tool_manager.h
index d1056bdba..14622d994 100644
--- a/include/tool/tool_manager.h
+++ b/include/tool/tool_manager.h
@@ -122,30 +122,35 @@ public:
      * Function RunAction()
      * Runs the specified action.
      *
+     * This function will only return if the action has been handled when the action is run
+     * immediately (aNow = true), otherwise it will always return false.
+     *
      * @param aAction is the action to be invoked.
      * @param aNow decides if the action has to be run immediately or after the current coroutine
      * is preemptied.
      * @param aParam is an optional parameter that might be used by the invoked action. Its meaning
      * depends on the action.
+     *
+     * @return True if the action was handled immediately
      */
-    template<typename T>
-    void RunAction( const TOOL_ACTION& aAction, bool aNow = false, T aParam = NULL )
+    template <typename T>
+    bool RunAction( const TOOL_ACTION& aAction, bool aNow = false, T aParam = NULL )
     {
-        RunAction( aAction, aNow, reinterpret_cast<void*>( aParam ) );
+        return RunAction( aAction, aNow, reinterpret_cast<void*>( aParam ) );
     }
 
-    void RunAction( const TOOL_ACTION& aAction, bool aNow, void* aParam );
+    bool RunAction( const TOOL_ACTION& aAction, bool aNow, void* aParam );
 
-    void RunAction( const TOOL_ACTION& aAction, bool aNow = false )
+    bool RunAction( const TOOL_ACTION& aAction, bool aNow = false )
     {
-        RunAction( aAction, aNow, (void*) NULL );
+        return RunAction( aAction, aNow, (void*) NULL );
     }
 
     const std::map<std::string, TOOL_ACTION*>& GetActions();
-    
+
     ///> @copydoc ACTION_MANAGER::GetHotKey()
     int GetHotKey( const TOOL_ACTION& aAction );
-    
+
     ACTION_MANAGER* GetActionManager() { return m_actionMgr; }
 
     /**
-- 
2.21.0