Re-factor the event whitelist stuff so that it doesn't promise what it can't deliver.

common/dialog_shim.cpp

@@ -511,13 +511,8 @@ void DIALOG_SHIM::OnButton( wxCommandEvent& aEvent )
 
 void DIALOG_SHIM::OnCharHook( wxKeyEvent& aEvt )
 {
-    if( m_parentFrame )
+    if( m_parentFrame && m_parentFrame->DispatchBehindModalDialog( aEvt ) )
-    {
+        return;
-        m_parentFrame->DispatchBehindModalDialog( aEvt );
-
-        if( !aEvt.GetSkipped() )
-            return;
-    }
 
     // shift-return (Mac default) or Ctrl-Return (GTK) for OK
     if( aEvt.GetKeyCode() == WXK_RETURN && ( aEvt.ShiftDown() || aEvt.ControlDown() ) )

common/eda_draw_frame.cpp

@@ -193,13 +193,19 @@ void EDA_DRAW_FRAME::unitsChangeRefresh()
 }
 
 
-void EDA_DRAW_FRAME::DispatchBehindModalDialog( wxKeyEvent& aEvent )
+bool EDA_DRAW_FRAME::DispatchBehindModalDialog( wxKeyEvent& aEvent )
 {
     static std::set<const TOOL_ACTION*> whiteList = { &ACTIONS::toggleUnits,
                                                       &ACTIONS::imperialUnits,
                                                       &ACTIONS::metricUnits };
 
-    m_toolDispatcher->DispatchWxEvent( aEvent, &whiteList );
+    bool dummy;
+    OPT<TOOL_EVENT> evt = m_toolDispatcher->GetToolEvent( &aEvent, &dummy );
+
+    if( evt && evt->Action() == TA_KEY_PRESSED )
+        return m_toolManager->DispatchHotKey( *evt, &whiteList );
+
+    return false;
 }
 
 

common/tool/tool_dispatcher.cpp

@@ -325,13 +325,87 @@ int translateSpecialCode( int aKeyCode )
 }
 
 
-void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent )
+OPT<TOOL_EVENT> TOOL_DISPATCHER::GetToolEvent( wxKeyEvent* aKeyEvent, bool* keyIsSpecial )
 {
-    DispatchWxEvent( aEvent, nullptr );
+    OPT<TOOL_EVENT> evt;
+    int             key = aKeyEvent->GetKeyCode();
+    int             unicode_key = aKeyEvent->GetUnicodeKey();
+
+    // This wxEVT_CHAR_HOOK event can be ignored: not useful in Kicad
+    if( isKeyModifierOnly( key ) )
+    {
+        aKeyEvent->Skip();
+        return evt;
+    }
+
+    wxLogTrace( kicadTraceKeyEvent, "TOOL_DISPATCHER::DispatchWxEvent %s", dump( *aKeyEvent ) );
+
+    // if the key event must be skipped, skip it here if the event is a wxEVT_CHAR_HOOK
+    // and do nothing.
+    *keyIsSpecial = isKeySpecialCode( key );
+
+    if( aKeyEvent->GetEventType() == wxEVT_CHAR_HOOK )
+        key = translateSpecialCode( key );
+
+    int mods = decodeModifiers( aKeyEvent );
+
+    if( mods & MD_CTRL )
+    {
+        // wxWidgets maps key codes related to Ctrl+letter handled by CHAR_EVT
+        // (http://docs.wxwidgets.org/trunk/classwx_key_event.html):
+        // char events for ASCII letters in this case carry codes corresponding to the ASCII
+        // value of Ctrl-Latter, i.e. 1 for Ctrl-A, 2 for Ctrl-B and so on until 26 for Ctrl-Z.
+        // They are remapped here to be more easy to handle in code
+        // Note also on OSX wxWidgets has a differnt behavior and the mapping is made
+        // only for ctrl+'A' to ctlr+'Z' (unicode code return 'A' to 'Z').
+        // Others OS return WXK_CONTROL_A to WXK_CONTROL_Z, and Ctrl+'M' returns the same code as
+        // the return key, so the remapping does not use the unicode key value.
+#ifdef __APPLE__
+        if( unicode_key >= 'A' && unicode_key <= 'Z' && key >= WXK_CONTROL_A && key <= WXK_CONTROL_Z )
+#else
+        (void) unicode_key; //not used: avoid compil warning
+
+        if( key >= WXK_CONTROL_A && key <= WXK_CONTROL_Z )
+#endif
+            key += 'A' - 1;
+    }
+
+#ifdef __APPLE__
+    if( mods & MD_ALT )
+    {
+        // OSX maps a bunch of commonly used extended-ASCII characters onto the keyboard
+        // using the ALT key.  Since we use ALT for some of our hotkeys, we need to map back
+        // to the underlying keys.  The kVK_ANSI_* values come from Apple and are said to be
+        // hardware independant.
+        switch( aKeyEvent->GetRawKeyCode() )
+        {
+        case /* kVK_ANSI_1     */ 0x12: key = '1'; break;
+        case /* kVK_ANSI_2     */ 0x13: key = '2'; break;
+        case /* kVK_ANSI_3     */ 0x14: key = '3'; break;
+        case /* kVK_ANSI_4     */ 0x15: key = '4'; break;
+        case /* kVK_ANSI_6     */ 0x16: key = '6'; break;
+        case /* kVK_ANSI_5     */ 0x17: key = '5'; break;
+        case /* kVK_ANSI_Equal */ 0x18: key = '='; break;
+        case /* kVK_ANSI_9     */ 0x19: key = '9'; break;
+        case /* kVK_ANSI_7     */ 0x1A: key = '7'; break;
+        case /* kVK_ANSI_Minus */ 0x1B: key = '-'; break;
+        case /* kVK_ANSI_8     */ 0x1C: key = '8'; break;
+        case /* kVK_ANSI_0     */ 0x1D: key = '0'; break;
+        default: ;
+        }
+    }
+#endif
+
+    if( key == WXK_ESCAPE ) // ESC is the special key for canceling tools
+        evt = TOOL_EVENT( TC_COMMAND, TA_CANCEL_TOOL );
+    else
+        evt = TOOL_EVENT( TC_KEYBOARD, TA_KEY_PRESSED, key | mods );
+
+    return evt;
 }
 
 
-void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent, std::set<const TOOL_ACTION*>* aWhiteList )
+void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent )
 {
     bool            motion = false;
     bool            buttonEvents = false;
@@ -406,79 +480,7 @@ void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent, std::set<const TOOL_ACTI
     }
     else if( type == wxEVT_CHAR_HOOK || type == wxEVT_CHAR )
     {
-        wxKeyEvent* ke = static_cast<wxKeyEvent*>( &aEvent );
+        evt = GetToolEvent( static_cast<wxKeyEvent*>( &aEvent ), &keyIsSpecial );
-        key = ke->GetKeyCode();
-        int unicode_key = ke->GetUnicodeKey();
-
-        // This wxEVT_CHAR_HOOK event can be ignored: not useful in Kicad
-        if( isKeyModifierOnly( key ) )
-        {
-            aEvent.Skip();
-            return;
-        }
-
-        wxLogTrace( kicadTraceKeyEvent, "TOOL_DISPATCHER::DispatchWxEvent %s", dump( *ke ) );
-
-        // if the key event must be skipped, skip it here if the event is a wxEVT_CHAR_HOOK
-        // and do nothing.
-        keyIsSpecial = isKeySpecialCode( key );
-
-        if( type == wxEVT_CHAR_HOOK )
-            key = translateSpecialCode( key );
-
-        int mods = decodeModifiers( ke );
-
-        if( mods & MD_CTRL )
-        {
-            // wxWidgets maps key codes related to Ctrl+letter handled by CHAR_EVT
-            // (http://docs.wxwidgets.org/trunk/classwx_key_event.html):
-            // char events for ASCII letters in this case carry codes corresponding to the ASCII
-            // value of Ctrl-Latter, i.e. 1 for Ctrl-A, 2 for Ctrl-B and so on until 26 for Ctrl-Z.
-            // They are remapped here to be more easy to handle in code
-            // Note also on OSX wxWidgets has a differnt behavior and the mapping is made
-            // only for ctrl+'A' to ctlr+'Z' (unicode code return 'A' to 'Z').
-            // Others OS return WXK_CONTROL_A to WXK_CONTROL_Z, and Ctrl+'M' returns the same code as
-            // the return key, so the remapping does not use the unicode key value.
-#ifdef __APPLE__
-            if( unicode_key >= 'A' && unicode_key <= 'Z' && key >= WXK_CONTROL_A && key <= WXK_CONTROL_Z )
-#else
-            (void) unicode_key; //not used: avoid compil warning
-
-            if( key >= WXK_CONTROL_A && key <= WXK_CONTROL_Z )
-#endif
-                key += 'A' - 1;
-        }
-
-#ifdef __APPLE__
-        if( mods & MD_ALT )
-        {
-            // OSX maps a bunch of commonly used extended-ASCII characters onto the keyboard
-            // using the ALT key.  Since we use ALT for some of our hotkeys, we need to map back
-            // to the underlying keys.  The kVK_ANSI_* values come from Apple and are said to be
-            // hardware independant.
-            switch( ke->GetRawKeyCode() )
-            {
-            case /* kVK_ANSI_1     */ 0x12: key = '1'; break;
-            case /* kVK_ANSI_2     */ 0x13: key = '2'; break;
-            case /* kVK_ANSI_3     */ 0x14: key = '3'; break;
-            case /* kVK_ANSI_4     */ 0x15: key = '4'; break;
-            case /* kVK_ANSI_6     */ 0x16: key = '6'; break;
-            case /* kVK_ANSI_5     */ 0x17: key = '5'; break;
-            case /* kVK_ANSI_Equal */ 0x18: key = '='; break;
-            case /* kVK_ANSI_9     */ 0x19: key = '9'; break;
-            case /* kVK_ANSI_7     */ 0x1A: key = '7'; break;
-            case /* kVK_ANSI_Minus */ 0x1B: key = '-'; break;
-            case /* kVK_ANSI_8     */ 0x1C: key = '8'; break;
-            case /* kVK_ANSI_0     */ 0x1D: key = '0'; break;
-            default: ;
-            }
-        }
-#endif
-
-        if( key == WXK_ESCAPE ) // ESC is the special key for canceling tools
-            evt = TOOL_EVENT( TC_COMMAND, TA_CANCEL_TOOL );
-        else
-            evt = TOOL_EVENT( TC_KEYBOARD, TA_KEY_PRESSED, key | mods );
     }
     else if( type == wxEVT_MENU_OPEN || type == wxEVT_MENU_CLOSE || type == wxEVT_MENU_HIGHLIGHT )
     {
@@ -529,10 +531,10 @@ void TOOL_DISPATCHER::DispatchWxEvent( wxEvent& aEvent, std::set<const TOOL_ACTI
     {
         wxLogTrace( kicadTraceToolStack, "TOOL_DISPATCHER::DispatchWxEvent %s", evt->Format() );
 
-        handled = m_toolMgr->ProcessEvent( *evt, aWhiteList );
+        handled = m_toolMgr->ProcessEvent( *evt );
 
         // ESC is the special key for canceling tools, and is therefore seen as handled
-        if( key == WXK_ESCAPE && ( !aWhiteList || aWhiteList.count( ACTIONS::cancelInteractive ) ) )
+        if( key == WXK_ESCAPE )
             handled = true;
     }
 

common/tool/tool_manager.cpp

@@ -767,7 +767,7 @@ bool TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent )
 }
 
 
-bool TOOL_MANAGER::dispatchHotKey( const TOOL_EVENT& aEvent,
+bool TOOL_MANAGER::DispatchHotKey( const TOOL_EVENT& aEvent,
                                    std::set<const TOOL_ACTION*>* aWhiteList )
 {
     if( aEvent.Action() == TA_KEY_PRESSED )
@@ -925,10 +925,9 @@ TOOL_MANAGER::ID_LIST::iterator TOOL_MANAGER::finishTool( TOOL_STATE* aState )
 }
 
 
-bool TOOL_MANAGER::ProcessEvent( const TOOL_EVENT& aEvent,
+bool TOOL_MANAGER::ProcessEvent( const TOOL_EVENT& aEvent )
-                                 std::set<const TOOL_ACTION*>* aWhiteList )
 {
-    bool handled = processEvent( aEvent, aWhiteList );
+    bool handled = processEvent( aEvent );
 
     TOOL_STATE* activeTool = GetCurrentToolState();
 
@@ -1079,13 +1078,12 @@ void TOOL_MANAGER::applyViewControls( TOOL_STATE* aState )
 }
 
 
-bool TOOL_MANAGER::processEvent( const TOOL_EVENT& aEvent,
+bool TOOL_MANAGER::processEvent( const TOOL_EVENT& aEvent )
-                                 std::set<const TOOL_ACTION*>* aWhiteList )
 {
     wxLogTrace( kicadTraceToolStack, "TOOL_MANAGER::processEvent - %s", aEvent.Format() );
 
     // First try to dispatch the action associated with the event if it is a key press event
-    bool handled = dispatchHotKey( aEvent, aWhiteList );
+    bool handled = DispatchHotKey( aEvent );
 
     if( !handled )
     {

include/eda_base_frame.h

@@ -217,7 +217,7 @@ public:
 
     void ChangeUserUnits( EDA_UNITS aUnits );
 
-    virtual void DispatchBehindModalDialog( wxKeyEvent& aEvent ) { }
+    virtual bool DispatchBehindModalDialog( wxKeyEvent& aEvent ) { return false; }
 
     SETTINGS_MANAGER* GetSettingsManager() const { return m_settingsManager; }
 

include/eda_draw_frame.h

@@ -122,7 +122,7 @@ protected:
 
     void unitsChangeRefresh() override;
 
-    void DispatchBehindModalDialog( wxKeyEvent& aEvent ) override;
+    bool DispatchBehindModalDialog( wxKeyEvent& aEvent ) override;
 
     void CommonSettingsChanged( bool aEnvVarsChanged ) override;
 

include/tool/tool_dispatcher.h

@@ -72,19 +72,14 @@ public:
      * Processes wxEvents (mostly UI events), translates them to TOOL_EVENTs, and makes tools
      * handle those.
      * @param aEvent is the wxWidgets event to be processed.
-     * @param aWhiteList an optional list of allowed TOOL_ACTIONs
      */
     virtual void DispatchWxEvent( wxEvent& aEvent );
 
     /**
-     * Function DispatchWxEvent()
+     * Function GetToolEvent()
-     * Processes wxEvents (mostly UI events), translates them to TOOL_EVENTs, and makes tools
+     * Maps a wxWidgets key event to a TOOL_EVENT.
-     * handle those.
-     * @param aEvent is the wxWidgets event to be processed.
-     * @param aWhiteList a list of allowable TOOL_ACTIONs.  Those not appearing on the list
-     *                   will not be dispatched.
      */
-    virtual void DispatchWxEvent( wxEvent& aEvent, std::set<const TOOL_ACTION*>* aWhiteList );
+    OPT<TOOL_EVENT> GetToolEvent( wxKeyEvent* aKeyEvent, bool* aSpecialKeyFlag );
 
     /**
      * Function DispatchWxCommand()

include/tool/tool_manager.h

@@ -256,11 +256,9 @@ public:
     /**
      * Propagates an event to tools that requested events of matching type(s).
      * @param aEvent is the event to be processed.
-     * @param aWhiteList an optional list of allowed TOOL_ACTIONs
      * @return true if the event is a managed hotkey
      */
-    bool ProcessEvent( const TOOL_EVENT& aEvent,
+    bool ProcessEvent( const TOOL_EVENT& aEvent );
-                       std::set<const TOOL_ACTION*>* aWhiteList = nullptr );
 
     /**
      * Puts an event to the event queue to be processed at the end of event processing cycle.
@@ -421,6 +419,16 @@ public:
      */
     void DispatchContextMenu( const TOOL_EVENT& aEvent );
 
+    /**
+     * Function dispatchHotKey()
+     * Handles specific events, that are intended for TOOL_MANAGER rather than tools.
+     * @param aEvent is the event to be processed.
+     * @param aWhiteList an optional list of allowed actions to run
+     * @return true if the event was processed and should not go any further.
+     */
+    bool DispatchHotKey( const TOOL_EVENT& aEvent,
+                         std::set<const TOOL_ACTION*>* aWhiteList = nullptr );
+
 private:
     typedef std::pair<TOOL_EVENT_LIST, TOOL_STATE_FUNC> TRANSITION;
 
@@ -430,15 +438,6 @@ private:
      */
     bool dispatchInternal( const TOOL_EVENT& aEvent );
 
-    /**
-     * Function dispatchHotKey()
-     * Handles specific events, that are intended for TOOL_MANAGER rather than tools.
-     * @param aEvent is the event to be processed.
-     * @return true if the event was processed and should not go any further.
-     */
-    bool dispatchHotKey( const TOOL_EVENT& aEvent,
-                         std::set<const TOOL_ACTION*>* aWhiteList = nullptr );
-
     /**
      * Function dispatchActivation()
      * Checks if it is a valid activation event and invokes a proper tool.
@@ -510,11 +509,9 @@ private:
 
     /**
      * Main function for event processing.
-     * @param aWhiteList an optional list of allowed TOOL_ACTIONs
      * @return true if a hotkey was handled
      */
-    bool processEvent( const TOOL_EVENT& aEvent,
+    bool processEvent( const TOOL_EVENT& aEvent );
-                       std::set<const TOOL_ACTION*>* aWhiteList = nullptr );
 
     /**
      * Saves the previous active state and sets a new one.