From a294e8d6c444a143b8b92868001ce8b29f835030 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@gmail.com>
Date: Thu, 27 Sep 2018 13:31:24 +0100
Subject: [PATCH] Check for conflicts when reseting/undoing hotkey changes

It was possible to get conflicting hotkeys when undoing and resetting hotkeys
to defaults.

This uses the same logic as when setting hotkeys to avoid conflcits in these
other two cases.

Fixes: lp:1794730
* https://bugs.launchpad.net/kicad/+bug/1794730
---
 common/widgets/widget_hotkey_list.cpp | 57 ++++++++++++++++-----------
 include/hotkey_store.h                |  9 +++--
 include/hotkeys_basic.h               |  5 +++
 include/widgets/widget_hotkey_list.h  | 11 ++++++
 4 files changed, 55 insertions(+), 27 deletions(-)

diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp
index 55cd903e1c..62f78f75fc 100644
--- a/common/widgets/widget_hotkey_list.cpp
+++ b/common/widgets/widget_hotkey_list.cpp
@@ -321,6 +321,31 @@ void WIDGET_HOTKEY_LIST::UpdateFromClientData()
             SetItemText( i, 1, key_text);
         }
     }
+
+    // Trigger a resize in case column widths have changed
+    wxSizeEvent dummy_evt;
+    TWO_COLUMN_TREE_LIST::OnSize( dummy_evt );
+}
+
+
+void WIDGET_HOTKEY_LIST::changeHotkey( CHANGED_HOTKEY& aHotkey, long aKey )
+{
+    // See if this key code is handled in hotkeys names list
+    bool exists;
+    KeyNameFromKeyCode( aKey, &exists );
+
+    auto& curr_hk = aHotkey.GetCurrentValue();
+
+    if( exists && curr_hk.m_KeyCode != aKey )
+    {
+        const auto& tag = aHotkey.GetSectionTag();
+        bool can_update = ResolveKeyConflicts( aKey, tag );
+
+        if( can_update )
+        {
+            curr_hk.m_KeyCode = aKey;
+        }
+    }
 }
 
 
@@ -342,29 +367,8 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
 
     if( hkdata && key )
     {
-        // See if this key code is handled in hotkeys names list
-        bool exists;
-        KeyNameFromKeyCode( key, &exists );
-
-        auto& changed_hk = hkdata->GetChangedHotkey();
-        auto& curr_hk = changed_hk.GetCurrentValue();
-
-        if( exists && curr_hk.m_KeyCode != key )
-        {
-            wxString tag = changed_hk.GetSectionTag();
-            bool canUpdate = ResolveKeyConflicts( key, tag );
-
-            if( canUpdate )
-            {
-                curr_hk.m_KeyCode = key;
-            }
-        }
-
+        changeHotkey( hkdata->GetChangedHotkey(), key );
         UpdateFromClientData();
-
-        // Trigger a resize in case column widths have changed
-        wxSizeEvent dummy_evt;
-        TWO_COLUMN_TREE_LIST::OnSize( dummy_evt );
     }
 }
 
@@ -372,8 +376,11 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
 void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
 {
     WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
-    hkdata->GetChangedHotkey().ResetHotkey();
 
+    auto& changed_hk = hkdata->GetChangedHotkey();
+    const auto& orig_hk = changed_hk.GetOriginalValue();
+
+    changeHotkey( changed_hk, orig_hk.m_KeyCode );
     UpdateFromClientData();
 }
 
@@ -381,8 +388,10 @@ void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
 void WIDGET_HOTKEY_LIST::ResetItemToDefault( wxTreeListItem aItem )
 {
     WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
-    hkdata->GetChangedHotkey().GetCurrentValue().ResetKeyCodeToDefault();
 
+    auto& changed_hk = hkdata->GetChangedHotkey();
+
+    changeHotkey( changed_hk, changed_hk.GetCurrentValue().GetDefaultKeyCode() );
     UpdateFromClientData();
 }
 
diff --git a/include/hotkey_store.h b/include/hotkey_store.h
index b025ce8915..fd5b5ec908 100644
--- a/include/hotkey_store.h
+++ b/include/hotkey_store.h
@@ -56,11 +56,14 @@ public:
     }
 
     /**
-     * Reset the changed hotkey back to the original value.
+     * Gets the original value of the hotkey. This is what the hotkey used
+     * to be, and what it would be set to if reset.
+     *
+     * @return reference to the original hotkey.
      */
-    void ResetHotkey()
+    const EDA_HOTKEY& GetOriginalValue() const
     {
-        m_changed = m_orig;
+        return m_orig;
     }
 
     /**
diff --git a/include/hotkeys_basic.h b/include/hotkeys_basic.h
index afccc55d7b..21e0dc5a8c 100644
--- a/include/hotkeys_basic.h
+++ b/include/hotkeys_basic.h
@@ -71,6 +71,11 @@ public:
     EDA_HOTKEY( const wxChar* infomsg, int idcommand, int keycode, int idmenuevent = 0 );
     EDA_HOTKEY( const EDA_HOTKEY* base);
     void ResetKeyCodeToDefault() { m_KeyCode = m_defaultKeyCode; }
+
+    int GetDefaultKeyCode() const
+    {
+        return m_defaultKeyCode;
+    }
 };
 
 
diff --git a/include/widgets/widget_hotkey_list.h b/include/widgets/widget_hotkey_list.h
index 78a6216b17..a0f93a8614 100644
--- a/include/widgets/widget_hotkey_list.h
+++ b/include/widgets/widget_hotkey_list.h
@@ -76,6 +76,17 @@ class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST
      */
     void updateShownItems( const wxString& aFilterStr );
 
+    /**
+     * Attempt to change the given hotkey to the given key code.
+     *
+     * If the hotkey conflicts, the user is prompted to change anyway (and
+     * in doing so, unset the conflicting key), or cancel the attempt.
+     *
+     * @param aHotkey the change-able hotkey to try to change
+     * @param aKey the key code to change it to
+     */
+    void changeHotkey( CHANGED_HOTKEY& aHotkey, long aKey );
+
 protected:
 
     /**