Hot keys specific to a tool have priority over global hot keys (TOOL_ACTION...

Hot keys specific to a tool have priority over global hot keys (TOOL_ACTION scope: AS_GLOBAL/AS_CONTEXT is finally handled properly).

common/tool/action_manager.cpp

@@ -26,6 +26,7 @@
 #include <tool/tool_manager.h>
 #include <tool/tool_event.h>
 #include <tool/tool_action.h>
+#include <boost/foreach.hpp>
 #include <cassert>
 
 ACTION_MANAGER::ACTION_MANAGER( TOOL_MANAGER* aToolManager ) :
@@ -43,7 +44,12 @@ ACTION_MANAGER::~ACTION_MANAGER()
 
 void ACTION_MANAGER::RegisterAction( TOOL_ACTION* aAction )
 {
-    assert( aAction->GetId() == -1 );    // Check if the TOOL_ACTION was not registered before
+    // Check if the TOOL_ACTION was not registered before
+    assert( aAction->GetId() == -1 );
+    // TOOL_ACTIONs are supposed to be named [appName.]toolName.actionName (with dots between)
+    // action name without specifying at least toolName is not valid
+    assert( aAction->GetName().find( '.', 0 ) != std::string::npos );
+
     assert( m_actionNameIndex.find( aAction->m_name ) == m_actionNameIndex.end() );
     assert( m_actionIdIndex.find( aAction->m_id ) == m_actionIdIndex.end() );
 
@@ -53,13 +59,7 @@ void ACTION_MANAGER::RegisterAction( TOOL_ACTION* aAction )
     m_actionIdIndex[aAction->m_id] = aAction;
 
     if( aAction->HasHotKey() )
-    {
-        // Duplication of hot keys leads to unexpected behaviour
-        // The right way to change a hotkey is to use ACTION_MANAGER::ClearHotKey() first
-        assert( m_actionHotKeys.find( aAction->m_currentHotKey ) == m_actionHotKeys.end() );
-
-        m_actionHotKeys[aAction->m_currentHotKey] = aAction;
-    }
+        m_actionHotKeys[aAction->m_currentHotKey].push_back( aAction );
 }
 
 
@@ -72,7 +72,15 @@ void ACTION_MANAGER::UnregisterAction( TOOL_ACTION* aAction )
     aAction->setId( -1 );
 
     if( aAction->HasHotKey() )
-        m_actionHotKeys.erase( aAction->m_currentHotKey );
+    {
+        std::list<TOOL_ACTION*>& actions = m_actionHotKeys[aAction->m_currentHotKey];
+        std::list<TOOL_ACTION*>::iterator action = std::find( actions.begin(), actions.end(), aAction );
+
+        if( action != actions.end() )
+            actions.erase( action );
+        else
+            assert( false );
+    }
 }
 
 
@@ -107,18 +115,52 @@ void ACTION_MANAGER::RunAction( const TOOL_ACTION* aAction ) const
 
 bool ACTION_MANAGER::RunHotKey( int aHotKey ) const
 {
-    std::map<int, TOOL_ACTION*>::const_iterator it = m_actionHotKeys.find( aHotKey );
+    HOTKEY_LIST::const_iterator it = m_actionHotKeys.find( aHotKey );
 
     if( it == m_actionHotKeys.end() )
         return false; // no appropriate action found for the hotkey
 
-    RunAction( it->second );
+    const std::list<TOOL_ACTION*>& actions = it->second;
 
-    return true;
-}
+    // Choose the action that has the highest priority on the active tools stack
+    // If there is none, run the global action associated with the hot key
+    int highestPriority = -1, priority = -1;
+    const TOOL_ACTION* context = NULL;  // pointer to context action of the highest priority tool
+    const TOOL_ACTION* global = NULL;   // pointer to global action, if there is no context action
+
+    BOOST_FOREACH( const TOOL_ACTION* action, actions )
+    {
+        if( action->GetScope() == AS_GLOBAL )
+        {
+            // Store the global action for the hot key in case there was no possible
+            // context actions to run
+            assert( global == NULL );       // there should be only one global action per hot key
+            global = action;
+
+            continue;
+        }
+
+        TOOL_BASE* tool = m_toolMgr->FindTool( action->GetToolName() );
+
+        if( tool )
+        {
+            priority = m_toolMgr->GetPriority( tool->GetId() );
+
+            if( priority >= 0 && priority > highestPriority )
+            {
+                highestPriority = priority;
+                context = action;
+            }
+        }
+    }
 
+    if( !global && !context )   // currently there is no valid action to run
+        return false;
 
-void ACTION_MANAGER::ClearHotKey( int aHotKey )
-{
-    m_actionHotKeys.erase( aHotKey );
+    if( context )
+        RunAction( context );
+    else if( global )
+        RunAction( global );
+
+    return true;
 }

common/tool/tool_manager.cpp

@@ -301,6 +301,25 @@ void TOOL_MANAGER::ResetTools( TOOL_BASE::RESET_REASON aReason )
 }
 
 
+int TOOL_MANAGER::GetPriority( int aToolId ) const
+{
+    int priority = 0;
+
+    for( std::deque<int>::const_iterator it = m_activeTools.begin(),
+            itEnd = m_activeTools.end(); it != itEnd; ++it )
+    {
+        std::cout << FindTool( *it )->GetName() << std::endl;
+
+        if( *it == aToolId )
+            return priority;
+
+        ++priority;
+    }
+
+    return -1;
+}
+
+
 void TOOL_MANAGER::ScheduleNextState( TOOL_BASE* aTool, TOOL_STATE_FUNC& aHandler,
                                       const TOOL_EVENT_LIST& aConditions )
 {

include/tool/action_manager.h

@@ -25,6 +25,7 @@
 #ifndef ACTION_MANAGER_H_
 #define ACTION_MANAGER_H_
 
+#include <list>
 #include <map>
 #include <string>
 
@@ -97,13 +98,6 @@ public:
      */
     bool RunHotKey( int aHotKey ) const;
 
-    /**
-     * Function ClearHotKey()
-     * Removes an action associated with a hotkey.
-     * @param aHotKey is the hotkey to be cleared.
-     */
-    void ClearHotKey( int aHotKey );
-
 private:
     ///> Tool manager needed to run actions
     TOOL_MANAGER* m_toolMgr;
@@ -115,7 +109,8 @@ private:
     std::map<std::string, TOOL_ACTION*> m_actionNameIndex;
 
     ///> Map for indexing actions by their hotkeys
-    std::map<int, TOOL_ACTION*> m_actionHotKeys;
+    typedef std::map<int, std::list<TOOL_ACTION*> > HOTKEY_LIST;
+    HOTKEY_LIST m_actionHotKeys;
 };
 
 #endif /* ACTION_MANAGER_H_ */

include/tool/tool_action.h

@@ -171,6 +171,21 @@ public:
         m_menuDescription = aDescription;
     }
 
+    TOOL_ACTION_SCOPE GetScope() const
+    {
+        return m_scope;
+    }
+
+    /**
+     * Returns name of the tool associated with the action. It is basically the action name
+     * stripped of the last part (e.g. for "pcbnew.InteractiveDrawing.drawCircle" it is
+     * "pcbnew.InteractiveDrawing").
+     */
+    std::string GetToolName() const
+    {
+        return m_name.substr( 0, m_name.rfind( '.' ) );
+    }
+
 private:
     friend class ACTION_MANAGER;
 

include/tool/tool_manager.h

@@ -206,6 +206,16 @@ public:
         return FindTool( GetCurrentToolId() );
     }
 
+    /**
+     * Returns priority of a given tool. Higher number means that the tool is closer to the
+     * beginning of the active tools queue (i.e. receives events earlier, tools with lower
+     * priority receive events later).
+     * @param aToolId is the id of queried tool.
+     * @return The priority of a given tool. If returned number is negative, then it means that
+     * the tool id is invalid or the tool is not active.
+     */
+    int GetPriority( int aToolId ) const;
+
     /**
      * Defines a state transition - the events that cause a given handler method in the tool
      * to be called. Called by TOOL_INTERACTIVE::Go(). May be called from a coroutine context.

pcbnew/router/router_tool.cpp

@@ -45,11 +45,11 @@
 using namespace KIGFX;
 using boost::optional;
 
-static TOOL_ACTION ACT_AutoEndRoute( "AutoEndRoute", AS_CONTEXT, 'G' );
-static TOOL_ACTION ACT_PlaceVia( "PlaceVia", AS_CONTEXT, 'V' );
-static TOOL_ACTION ACT_OpenRouteOptions( "OpenRouterOptions", AS_CONTEXT, 'Y' );
-static TOOL_ACTION ACT_SwitchPosture( "SwitchPosture", AS_CONTEXT, '/' );
-static TOOL_ACTION ACT_EndTrack( "EndTrack", AS_CONTEXT, WXK_END );
+static TOOL_ACTION ACT_AutoEndRoute( "pcbnew.InteractiveRouter.AutoEndRoute", AS_CONTEXT, 'G' );
+static TOOL_ACTION ACT_PlaceVia( "pcbnew.InteractiveRouter.PlaceVia", AS_CONTEXT, 'V' );
+static TOOL_ACTION ACT_OpenRouteOptions( "pcbnew.InteractiveRouter.OpenRouterOptions", AS_CONTEXT, 'T' );
+static TOOL_ACTION ACT_SwitchPosture( "pcbnew.InteractiveRouter.SwitchPosture", AS_CONTEXT, '/' );
+static TOOL_ACTION ACT_EndTrack( "pcbnew.InteractiveRouter.EndTrack", AS_CONTEXT, WXK_END );
 
 ROUTER_TOOL::ROUTER_TOOL() :
     TOOL_INTERACTIVE( "pcbnew.InteractiveRouter" )

pcbnew/tools/common_actions.cpp

@@ -43,11 +43,11 @@ TOOL_ACTION COMMON_ACTIONS::editActivate( "pcbnew.InteractiveEdit",
         "Move", "Moves the selected item(s)" );
 
 TOOL_ACTION COMMON_ACTIONS::rotate( "pcbnew.rotate",
-        AS_CONTEXT, 'R',
+        AS_GLOBAL, 'R',
         "Rotate", "Rotates selected item(s)" );
 
 TOOL_ACTION COMMON_ACTIONS::flip( "pcbnew.flip",
-        AS_CONTEXT, 'F',
+        AS_GLOBAL, 'F',
         "Flip", "Flips selected item(s)" );
 
 TOOL_ACTION COMMON_ACTIONS::remove( "pcbnew.InteractiveEdit.remove",
@@ -58,6 +58,7 @@ TOOL_ACTION COMMON_ACTIONS::properties( "pcbnew.InteractiveEdit.properties",
         AS_GLOBAL, 'E',
         "Properties...", "Displays properties window" );
 
+
 // Drawing tool actions
 TOOL_ACTION COMMON_ACTIONS::drawLine( "pcbnew.InteractiveDrawing.line",
         AS_GLOBAL, 0,