kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #37688
[PATCH] Hotkey edit dialog filter + fixes
Hi,
Here is a patch set for adding a filter control to the hotkey editor
dialog. Preview video: https://sendvid.com/je0cyg87
Most of the work in the first commit is separating out the hotkey data
from the UI widget code.
This also fixes a couple of other bugs (one crashing, and one able to
get the HK configs into a mess with conflicts) in that dialog, and
adds some mock objects to the common QA tests, which will make it
easier to get more of libcommon under test.
A 5.0 series fix for the crashing bug will follow.
This is also useful as groundwork for
https://bugs.launchpad.net/kicad/+bug/1778374 (a 5.1 milestone), but
that will need a further decision to choose between:
* Use a read-only variant of the hotkey editor TreeListView for the
hotkey list dialog (needs more code: a new dialog and adding a
read-only mode to the current widget), or
* Remove the hotkey list dialog altogether and bind the "show hotkey
list" command to simply opening the prefs dialog to the hotkey panel
Cheers,
John
From 6f219fc68c17d0648c0f32fd468174b34210a078 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 27 Sep 2018 15:48:52 +0100
Subject: [PATCH 2/6] Add some hotkey store tests
This also adds some mocks to the libcommon test executable, which
hopefully will allow it to work with more of libcommon.
---
common/hotkey_store.cpp | 25 ++++++
include/hotkey_store.h | 7 ++
qa/common/CMakeLists.txt | 6 ++
qa/common/common_mocks.cpp | 42 +++++++++
qa/common/test_hotkey_store.cpp | 148 ++++++++++++++++++++++++++++++++
5 files changed, 228 insertions(+)
create mode 100644 qa/common/common_mocks.cpp
create mode 100644 qa/common/test_hotkey_store.cpp
diff --git a/common/hotkey_store.cpp b/common/hotkey_store.cpp
index 62e9fc424..479d9c67a 100644
--- a/common/hotkey_store.cpp
+++ b/common/hotkey_store.cpp
@@ -53,6 +53,30 @@ std::vector<HOTKEY_SECTION>& HOTKEY_STORE::GetSections()
}
+CHANGED_HOTKEY* HOTKEY_STORE::FindHotkey( const wxString& aTag, int aCmdId )
+{
+ CHANGED_HOTKEY* found_key = nullptr;
+
+ for( auto& section: m_hk_sections )
+ {
+ if( *section.m_section.m_SectionTag != aTag)
+ continue;
+
+ for( auto& hotkey: section.m_hotkeys )
+ {
+ auto& curr_hk = hotkey.GetCurrentValue();
+ if( curr_hk.m_Idcommand == aCmdId )
+ {
+ found_key = &hotkey;
+ break;
+ }
+ }
+ }
+
+ return found_key;
+}
+
+
void HOTKEY_STORE::SaveAllHotkeys()
{
for( auto& section: m_hk_sections )
@@ -98,6 +122,7 @@ bool HOTKEY_STORE::CheckKeyConflicts( long aKey, const wxString& aSectionTag,
for( auto& section: m_hk_sections )
{
const auto& sectionTag = *section.m_section.m_SectionTag;
+
if( aSectionTag != g_CommonSectionTag
&& sectionTag != g_CommonSectionTag
&& sectionTag != aSectionTag )
diff --git a/include/hotkey_store.h b/include/hotkey_store.h
index 13b985b36..cbeb58d34 100644
--- a/include/hotkey_store.h
+++ b/include/hotkey_store.h
@@ -126,6 +126,13 @@ public:
*/
SECTION_LIST& GetSections();
+
+ /**
+ * Find a hotkey with the given command ID and in the given section
+ * @return pointer to the hotkey if found.
+ */
+ CHANGED_HOTKEY* FindHotkey( const wxString& aTag, int aCmdId );
+
/**
* Persist all changes to hotkeys in the store to the underlying
* data structures.
diff --git a/qa/common/CMakeLists.txt b/qa/common/CMakeLists.txt
index b5aae6c3c..e6c163296 100644
--- a/qa/common/CMakeLists.txt
+++ b/qa/common/CMakeLists.txt
@@ -26,8 +26,13 @@ find_package( wxWidgets 3.0.0 COMPONENTS gl aui adv html core net base xml stc R
add_definitions(-DBOOST_TEST_DYN_LINK)
add_executable( qa_common
+ # This is needed for the global mock objects
+ common_mocks.cpp
+
+ # The main test entry points
test_module.cpp
+ test_hotkey_store.cpp
test_utf8.cpp
geometry/test_fillet.cpp
@@ -43,6 +48,7 @@ include_directories(
target_link_libraries( qa_common
common
polygon
+ bitmaps
${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}
${wxWidgets_LIBRARIES}
)
diff --git a/qa/common/common_mocks.cpp b/qa/common/common_mocks.cpp
new file mode 100644
index 000000000..9b54ee6ec
--- /dev/null
+++ b/qa/common/common_mocks.cpp
@@ -0,0 +1,42 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+/**
+ * @file common_mocks.cpp
+ * @brief Mock objects for libcommon unit tests
+ */
+
+#include <pgm_base.h>
+
+
+struct PGM_TEST_FRAME : public PGM_BASE
+{
+ void MacOpenFile( const wxString& aFileName ) override
+ {}
+};
+
+PGM_BASE& Pgm()
+{
+ static PGM_TEST_FRAME program;
+ return program;
+}
\ No newline at end of file
diff --git a/qa/common/test_hotkey_store.cpp b/qa/common/test_hotkey_store.cpp
new file mode 100644
index 000000000..e6db29f10
--- /dev/null
+++ b/qa/common/test_hotkey_store.cpp
@@ -0,0 +1,148 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2018 KiCad Developers, see CHANGELOG.TXT for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include <boost/test/unit_test.hpp>
+#include <boost/test/test_case_template.hpp>
+
+#include <hotkey_store.h>
+
+// ----------------------------------------------------------------------------
+// Dummy Hotkey definitions
+
+static EDA_HOTKEY actionA1( _HKI("A1"), 1001, WXK_F1 );
+static EDA_HOTKEY actionA2( _HKI("A2"), 1002, WXK_F2 );
+
+static wxString sectionATag( "[a]" );
+static wxString sectionATitle( "Section A" );
+
+static EDA_HOTKEY actionB1( _HKI("B1"), 2001, WXK_F10 );
+static EDA_HOTKEY actionB2( _HKI("B2"), 2002, WXK_F11 );
+
+static wxString sectionBTag( "[b]" );
+static wxString sectionBTitle( "Section B" );
+
+// A keycode that is unused by any hotkey
+static const int unused_keycode = WXK_F5;
+
+// List of hotkey descriptors for library editor
+static EDA_HOTKEY* sectionAHotkeyList[] =
+{
+ &actionA1,
+ &actionA2,
+ NULL
+};
+
+static EDA_HOTKEY* sectionBHotkeyList[] =
+{
+ &actionB1,
+ &actionB2,
+ NULL
+};
+
+static EDA_HOTKEY_CONFIG test_hokeys_descr[] =
+{
+ { §ionATag, sectionAHotkeyList, §ionATitle },
+ { §ionBTag, sectionBHotkeyList, §ionBTitle },
+ { NULL, NULL, NULL }
+};
+
+// Number of sections in the above table
+static const unsigned num_cfg_sections =
+ sizeof( test_hokeys_descr ) / sizeof( EDA_HOTKEY_CONFIG ) - 1;
+
+// ----------------------------------------------------------------------------
+
+struct HotkeyStoreFixture
+{
+ HotkeyStoreFixture():
+ m_hotkey_store( test_hokeys_descr )
+ {}
+
+ HOTKEY_STORE m_hotkey_store;
+};
+
+
+/**
+ * Declares a struct as the Boost test fixture.
+ */
+BOOST_FIXTURE_TEST_SUITE( HotkeyStore, HotkeyStoreFixture )
+
+
+/**
+ * Check conflict detections
+ */
+BOOST_AUTO_TEST_CASE( StoreConstruction )
+{
+ // Should have ingested two sections (A and B)
+ BOOST_CHECK_EQUAL( m_hotkey_store.GetSections().size(), num_cfg_sections );
+}
+
+
+/**
+ * Check conflict detections
+ */
+BOOST_AUTO_TEST_CASE( HotkeyConflict )
+{
+ EDA_HOTKEY* conf_key = nullptr;
+ EDA_HOTKEY_CONFIG* conf_sect = nullptr;
+ bool conflict;
+
+ conflict = m_hotkey_store.CheckKeyConflicts( unused_keycode, sectionATag,
+ &conf_key, &conf_sect );
+
+ // No conflicts
+ BOOST_CHECK_EQUAL( conflict, true );
+
+ conflict = m_hotkey_store.CheckKeyConflicts( actionA1.m_KeyCode, sectionATag,
+ &conf_key, &conf_sect );
+
+ // See if we conflicted with the correct key in the correct section
+ BOOST_CHECK_EQUAL( conflict, false );
+ BOOST_CHECK_EQUAL( conf_key->m_Idcommand, actionA1.m_Idcommand );
+ BOOST_CHECK_EQUAL( conf_sect, &test_hokeys_descr[0] );
+}
+
+
+/**
+ * Check the undo works
+ */
+BOOST_AUTO_TEST_CASE( HotkeySetUndo )
+{
+ CHANGED_HOTKEY* hk = m_hotkey_store.FindHotkey( sectionATag, actionA1.m_Idcommand );
+
+ // Found something
+ BOOST_CHECK( hk );
+ BOOST_CHECK_EQUAL( hk->GetCurrentValue().m_Idcommand, actionA1.m_Idcommand );
+
+ // Change the Key code
+ hk->GetCurrentValue().m_KeyCode = unused_keycode;
+
+ // Change everything back
+ m_hotkey_store.ResetAllHotkeysToDefault();
+
+ // Check it went back
+ BOOST_CHECK_EQUAL( hk->GetCurrentValue().m_Idcommand, actionA1.m_Idcommand );
+}
+
+
+BOOST_AUTO_TEST_SUITE_END()
--
2.19.0
From de15f0e571452949b5799c6c98463a1037c9fa55 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 1 Aug 2018 10:15:05 +0100
Subject: [PATCH 3/6] Add a filter box to hotkey dialog and filter using it
This uses a simple case-insensitive partial match, which is
a good start for the relatively limited number of hotkeys
generally present.
---
common/dialogs/panel_hotkeys_editor.cpp | 8 ++
common/dialogs/panel_hotkeys_editor_base.cpp | 11 ++-
common/dialogs/panel_hotkeys_editor_base.fbp | 96 ++++++++++++++++++++
common/dialogs/panel_hotkeys_editor_base.h | 9 +-
common/widgets/widget_hotkey_list.cpp | 70 +++++++++++++-
include/panel_hotkeys_editor.h | 8 ++
include/widgets/widget_hotkey_list.h | 18 ++++
7 files changed, 211 insertions(+), 9 deletions(-)
diff --git a/common/dialogs/panel_hotkeys_editor.cpp b/common/dialogs/panel_hotkeys_editor.cpp
index 6059d5465..e74700777 100644
--- a/common/dialogs/panel_hotkeys_editor.cpp
+++ b/common/dialogs/panel_hotkeys_editor.cpp
@@ -35,6 +35,8 @@ PANEL_HOTKEYS_EDITOR::PANEL_HOTKEYS_EDITOR( EDA_BASE_FRAME* aFrame, wxWindow* aW
m_nickname( aNickname ),
m_hotkeyStore( aShowHotkeys )
{
+ m_filterSearch->SetDescriptiveText( _("Type filter text") );
+
m_hotkeyListCtrl = new WIDGET_HOTKEY_LIST( m_panelHotkeys, m_hotkeyStore );
m_hotkeyListCtrl->InstallOnPanel( m_panelHotkeys );
}
@@ -79,3 +81,9 @@ void PANEL_HOTKEYS_EDITOR::OnImport( wxCommandEvent& aEvent )
{
m_frame->ImportHotkeyConfigFromFile( m_hotkeys, m_nickname );
}
+
+void PANEL_HOTKEYS_EDITOR::OnFilterSearch( wxCommandEvent& aEvent )
+{
+ const auto searchStr = aEvent.GetString();
+ m_hotkeyListCtrl->ApplyFilterString( searchStr );
+}
diff --git a/common/dialogs/panel_hotkeys_editor_base.cpp b/common/dialogs/panel_hotkeys_editor_base.cpp
index c944fde74..2f00fd549 100644
--- a/common/dialogs/panel_hotkeys_editor_base.cpp
+++ b/common/dialogs/panel_hotkeys_editor_base.cpp
@@ -1,5 +1,5 @@
///////////////////////////////////////////////////////////////////////////
-// C++ code generated with wxFormBuilder (version Jun 5 2018)
+// C++ code generated with wxFormBuilder (version Jun 18 2018)
// http://www.wxformbuilder.org/
//
// PLEASE DO *NOT* EDIT THIS FILE!
@@ -16,6 +16,13 @@ PANEL_HOTKEYS_EDITOR_BASE::PANEL_HOTKEYS_EDITOR_BASE( wxWindow* parent, wxWindow
wxBoxSizer* bMargins;
bMargins = new wxBoxSizer( wxVERTICAL );
+ m_filterSearch = new wxSearchCtrl( this, wxID_ANY, wxEmptyString, wxDefaultPosition, wxDefaultSize, 0 );
+ #ifndef __WXMAC__
+ m_filterSearch->ShowSearchButton( false );
+ #endif
+ m_filterSearch->ShowCancelButton( true );
+ bMargins->Add( m_filterSearch, 0, wxBOTTOM|wxEXPAND|wxTOP, 5 );
+
m_panelHotkeys = new wxPanel( this, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTAB_TRAVERSAL );
m_panelHotkeys->SetMinSize( wxSize( -1,350 ) );
@@ -50,6 +57,7 @@ PANEL_HOTKEYS_EDITOR_BASE::PANEL_HOTKEYS_EDITOR_BASE( wxWindow* parent, wxWindow
this->Layout();
// Connect Events
+ m_filterSearch->Connect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::OnFilterSearch ), NULL, this );
m_resetButton->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::ResetClicked ), NULL, this );
m_defaultButton->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::DefaultsClicked ), NULL, this );
btnImport->Connect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::OnImport ), NULL, this );
@@ -59,6 +67,7 @@ PANEL_HOTKEYS_EDITOR_BASE::PANEL_HOTKEYS_EDITOR_BASE( wxWindow* parent, wxWindow
PANEL_HOTKEYS_EDITOR_BASE::~PANEL_HOTKEYS_EDITOR_BASE()
{
// Disconnect Events
+ m_filterSearch->Disconnect( wxEVT_COMMAND_TEXT_UPDATED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::OnFilterSearch ), NULL, this );
m_resetButton->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::ResetClicked ), NULL, this );
m_defaultButton->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::DefaultsClicked ), NULL, this );
btnImport->Disconnect( wxEVT_COMMAND_BUTTON_CLICKED, wxCommandEventHandler( PANEL_HOTKEYS_EDITOR_BASE::OnImport ), NULL, this );
diff --git a/common/dialogs/panel_hotkeys_editor_base.fbp b/common/dialogs/panel_hotkeys_editor_base.fbp
index b57da3323..f54a83ad6 100644
--- a/common/dialogs/panel_hotkeys_editor_base.fbp
+++ b/common/dialogs/panel_hotkeys_editor_base.fbp
@@ -93,6 +93,98 @@
<property name="name">bMargins</property>
<property name="orient">wxVERTICAL</property>
<property name="permission">none</property>
+ <object class="sizeritem" expanded="1">
+ <property name="border">5</property>
+ <property name="flag">wxBOTTOM|wxEXPAND|wxTOP</property>
+ <property name="proportion">0</property>
+ <object class="wxSearchCtrl" expanded="1">
+ <property name="BottomDockable">1</property>
+ <property name="LeftDockable">1</property>
+ <property name="RightDockable">1</property>
+ <property name="TopDockable">1</property>
+ <property name="aui_layer"></property>
+ <property name="aui_name"></property>
+ <property name="aui_position"></property>
+ <property name="aui_row"></property>
+ <property name="best_size"></property>
+ <property name="bg"></property>
+ <property name="cancel_button">1</property>
+ <property name="caption"></property>
+ <property name="caption_visible">1</property>
+ <property name="center_pane">0</property>
+ <property name="close_button">1</property>
+ <property name="context_help"></property>
+ <property name="context_menu">1</property>
+ <property name="default_pane">0</property>
+ <property name="dock">Dock</property>
+ <property name="dock_fixed">0</property>
+ <property name="docking">Left</property>
+ <property name="enabled">1</property>
+ <property name="fg"></property>
+ <property name="floatable">1</property>
+ <property name="font"></property>
+ <property name="gripper">0</property>
+ <property name="hidden">0</property>
+ <property name="id">wxID_ANY</property>
+ <property name="max_size"></property>
+ <property name="maximize_button">0</property>
+ <property name="maximum_size"></property>
+ <property name="min_size"></property>
+ <property name="minimize_button">0</property>
+ <property name="minimum_size"></property>
+ <property name="moveable">1</property>
+ <property name="name">m_filterSearch</property>
+ <property name="pane_border">1</property>
+ <property name="pane_position"></property>
+ <property name="pane_size"></property>
+ <property name="permission">protected</property>
+ <property name="pin_button">1</property>
+ <property name="pos"></property>
+ <property name="resize">Resizable</property>
+ <property name="search_button">0</property>
+ <property name="show">1</property>
+ <property name="size"></property>
+ <property name="style"></property>
+ <property name="subclass">; forward_declare</property>
+ <property name="toolbar_pane">0</property>
+ <property name="tooltip"></property>
+ <property name="validator_data_type"></property>
+ <property name="validator_style">wxFILTER_NONE</property>
+ <property name="validator_type">wxDefaultValidator</property>
+ <property name="validator_variable"></property>
+ <property name="value"></property>
+ <property name="window_extra_style"></property>
+ <property name="window_name"></property>
+ <property name="window_style"></property>
+ <event name="OnCancelButton"></event>
+ <event name="OnChar"></event>
+ <event name="OnEnterWindow"></event>
+ <event name="OnEraseBackground"></event>
+ <event name="OnKeyDown"></event>
+ <event name="OnKeyUp"></event>
+ <event name="OnKillFocus"></event>
+ <event name="OnLeaveWindow"></event>
+ <event name="OnLeftDClick"></event>
+ <event name="OnLeftDown"></event>
+ <event name="OnLeftUp"></event>
+ <event name="OnMiddleDClick"></event>
+ <event name="OnMiddleDown"></event>
+ <event name="OnMiddleUp"></event>
+ <event name="OnMotion"></event>
+ <event name="OnMouseEvents"></event>
+ <event name="OnMouseWheel"></event>
+ <event name="OnPaint"></event>
+ <event name="OnRightDClick"></event>
+ <event name="OnRightDown"></event>
+ <event name="OnRightUp"></event>
+ <event name="OnSearchButton"></event>
+ <event name="OnSetFocus"></event>
+ <event name="OnSize"></event>
+ <event name="OnText">OnFilterSearch</event>
+ <event name="OnTextEnter"></event>
+ <event name="OnUpdateUI"></event>
+ </object>
+ </object>
<object class="sizeritem" expanded="1">
<property name="border">2</property>
<property name="flag">wxEXPAND|wxTOP|wxBOTTOM|wxRIGHT</property>
@@ -216,6 +308,7 @@
<property name="hidden">0</property>
<property name="id">wxID_RESET</property>
<property name="label">Reset Hotkeys</property>
+ <property name="markup">0</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
<property name="maximum_size"></property>
@@ -304,6 +397,7 @@
<property name="hidden">0</property>
<property name="id">wxID_ANY</property>
<property name="label">Set to Defaults</property>
+ <property name="markup">0</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
<property name="maximum_size"></property>
@@ -402,6 +496,7 @@
<property name="hidden">0</property>
<property name="id">wxID_ANY</property>
<property name="label">Import...</property>
+ <property name="markup">0</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
<property name="maximum_size"></property>
@@ -490,6 +585,7 @@
<property name="hidden">0</property>
<property name="id">wxID_ANY</property>
<property name="label">Export...</property>
+ <property name="markup">0</property>
<property name="max_size"></property>
<property name="maximize_button">0</property>
<property name="maximum_size"></property>
diff --git a/common/dialogs/panel_hotkeys_editor_base.h b/common/dialogs/panel_hotkeys_editor_base.h
index 88a78905d..c199f8146 100644
--- a/common/dialogs/panel_hotkeys_editor_base.h
+++ b/common/dialogs/panel_hotkeys_editor_base.h
@@ -1,5 +1,5 @@
///////////////////////////////////////////////////////////////////////////
-// C++ code generated with wxFormBuilder (version Jun 5 2018)
+// C++ code generated with wxFormBuilder (version Jun 18 2018)
// http://www.wxformbuilder.org/
//
// PLEASE DO *NOT* EDIT THIS FILE!
@@ -11,12 +11,13 @@
#include <wx/artprov.h>
#include <wx/xrc/xmlres.h>
#include <wx/intl.h>
-#include <wx/panel.h>
+#include <wx/string.h>
+#include <wx/srchctrl.h>
#include <wx/gdicmn.h>
#include <wx/font.h>
#include <wx/colour.h>
#include <wx/settings.h>
-#include <wx/string.h>
+#include <wx/panel.h>
#include <wx/button.h>
#include <wx/sizer.h>
@@ -32,6 +33,7 @@ class PANEL_HOTKEYS_EDITOR_BASE : public wxPanel
protected:
wxBoxSizer* m_mainSizer;
+ wxSearchCtrl* m_filterSearch;
wxPanel* m_panelHotkeys;
wxButton* m_resetButton;
wxButton* m_defaultButton;
@@ -39,6 +41,7 @@ class PANEL_HOTKEYS_EDITOR_BASE : public wxPanel
wxButton* btnExport;
// Virtual event handlers, overide them in your derived class
+ virtual void OnFilterSearch( wxCommandEvent& event ) { event.Skip(); }
virtual void ResetClicked( wxCommandEvent& event ) { event.Skip(); }
virtual void DefaultsClicked( wxCommandEvent& event ) { event.Skip(); }
virtual void OnImport( wxCommandEvent& event ) { event.Skip(); }
diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp
index 9e38502aa..af7d759f8 100644
--- a/common/widgets/widget_hotkey_list.cpp
+++ b/common/widgets/widget_hotkey_list.cpp
@@ -226,6 +226,52 @@ public:
};
+/**
+ * Class HOTKEY_FILTER
+ *
+ * Class to manage logic for filtering hotkeys based on user input
+ */
+class HOTKEY_FILTER
+{
+public:
+ HOTKEY_FILTER( const wxString& aFilterStr )
+ {
+ m_normalised_filter_str = aFilterStr.Upper();
+ m_valid = m_normalised_filter_str.size() > 0;
+ }
+
+
+ /**
+ * Method FilterMatches
+ *
+ * Checks if the filter matches the given hotkey
+ *
+ * @return true on match (or if filter is disabled)
+ */
+ bool FilterMatches( const EDA_HOTKEY& aHotkey ) const
+ {
+ if( !m_valid )
+ return true;
+
+ // Match in the (translated) filter string
+ const auto normedInfo = wxGetTranslation( aHotkey.m_InfoMsg ).Upper();
+ if( normedInfo.Contains( m_normalised_filter_str ) )
+ return true;
+
+ const wxString keyName = KeyNameFromKeyCode( aHotkey.m_KeyCode );
+ if( keyName.Upper().Contains( m_normalised_filter_str ) )
+ return true;
+
+ return false;
+ }
+
+private:
+
+ bool m_valid;
+ wxString m_normalised_filter_str;
+};
+
+
WIDGET_HOTKEY_CLIENT_DATA* WIDGET_HOTKEY_LIST::GetHKClientData( wxTreeListItem aItem )
{
if( aItem.IsOk() )
@@ -449,6 +495,12 @@ void WIDGET_HOTKEY_LIST::InstallOnPanel( wxPanel* aPanel )
}
+void WIDGET_HOTKEY_LIST::ApplyFilterString( const wxString& aFilterStr )
+{
+ updateShownItems( aFilterStr );
+}
+
+
void WIDGET_HOTKEY_LIST::ResetAllHotkeys( bool aResetToDefault )
{
Freeze();
@@ -470,13 +522,20 @@ void WIDGET_HOTKEY_LIST::ResetAllHotkeys( bool aResetToDefault )
}
+bool WIDGET_HOTKEY_LIST::TransferDataToControl()
+{
+ updateShownItems( "" );
+ return true;
+}
-bool WIDGET_HOTKEY_LIST::TransferDataToControl()
+void WIDGET_HOTKEY_LIST::updateShownItems( const wxString& aFilterStr )
{
Freeze();
DeleteAllItems();
+ HOTKEY_FILTER filter( aFilterStr );
+
for( auto& section: m_hk_store.GetSections() )
{
// Create parent tree item
@@ -484,8 +543,11 @@ bool WIDGET_HOTKEY_LIST::TransferDataToControl()
for( auto& hotkey: section.m_hotkeys )
{
- wxTreeListItem item = AppendItem( parent, wxEmptyString );
- SetItemData( item, new WIDGET_HOTKEY_CLIENT_DATA( hotkey ) );
+ if( filter.FilterMatches( hotkey.GetCurrentValue() ) )
+ {
+ wxTreeListItem item = AppendItem( parent, wxEmptyString );
+ SetItemData( item, new WIDGET_HOTKEY_CLIENT_DATA( hotkey ) );
+ }
}
Expand( parent );
@@ -493,8 +555,6 @@ bool WIDGET_HOTKEY_LIST::TransferDataToControl()
UpdateFromClientData();
Thaw();
-
- return true;
}
diff --git a/include/panel_hotkeys_editor.h b/include/panel_hotkeys_editor.h
index 576277f4e..fd4776c49 100644
--- a/include/panel_hotkeys_editor.h
+++ b/include/panel_hotkeys_editor.h
@@ -72,6 +72,14 @@ private:
void OnExport( wxCommandEvent& aEvent ) override;
void OnImport( wxCommandEvent& aEvent ) override;
+
+ /**
+ * Function OnFilterSearch
+ * Handle a change in the hoteky filter text
+ *
+ * @param aEvent: the search event, used to get the search query
+ */
+ void OnFilterSearch( wxCommandEvent& aEvent ) override;
};
diff --git a/include/widgets/widget_hotkey_list.h b/include/widgets/widget_hotkey_list.h
index 36f2bb84e..78a6216b1 100644
--- a/include/widgets/widget_hotkey_list.h
+++ b/include/widgets/widget_hotkey_list.h
@@ -67,6 +67,15 @@ class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST
*/
void UpdateFromClientData();
+ /**
+ * Method updateShownItems
+ *
+ * Update the items shown in the widget based on a given filter string.
+ *
+ * @param aFilterStr the string to filter with. Empty means no filter.
+ */
+ void updateShownItems( const wxString& aFilterStr );
+
protected:
/**
@@ -147,6 +156,15 @@ public:
*/
void InstallOnPanel( wxPanel* aPanel );
+ /**
+ * Method ApplyFilterString
+ * Apply a filter string to the hotkey list, selecting which hotkeys
+ * to show.
+ *
+ * @param aFilterStr the string to filter by
+ */
+ void ApplyFilterString( const wxString& aFilterStr );
+
/**
* Set hotkeys in the control to default or original values.
* @param aResetToDefault if true,.reset to the defaults inherent to the
--
2.19.0
From aabce7ad3b74eab22f2d8967eadc31efa15d2fbc Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 27 Sep 2018 12:30:43 +0100
Subject: [PATCH 4/6] Mark modified hotkeys in the hotkey editor
THis uses a simple " *" suffix, as the wxTreeListCtrl-derivative
widget doesn't allow easy access to things like font style.
---
common/widgets/widget_hotkey_list.cpp | 11 +++++++++--
include/hotkey_store.h | 9 +++++++++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp
index af7d759f8..55cd903e1 100644
--- a/common/widgets/widget_hotkey_list.cpp
+++ b/common/widgets/widget_hotkey_list.cpp
@@ -308,10 +308,17 @@ void WIDGET_HOTKEY_LIST::UpdateFromClientData()
if( hkdata )
{
- const EDA_HOTKEY& hk = hkdata->GetChangedHotkey().GetCurrentValue();
+ const auto& changed_hk = hkdata->GetChangedHotkey();
+ const EDA_HOTKEY& hk = changed_hk.GetCurrentValue();
+
+ wxString key_text = KeyNameFromKeyCode( hk.m_KeyCode );
+
+ // mark unsaved changes
+ if( changed_hk.HasUnsavedChange() )
+ key_text += " *";
SetItemText( i, 0, wxGetTranslation( hk.m_InfoMsg ) );
- SetItemText( i, 1, KeyNameFromKeyCode( hk.m_KeyCode ) );
+ SetItemText( i, 1, key_text);
}
}
}
diff --git a/include/hotkey_store.h b/include/hotkey_store.h
index cbeb58d34..b025ce891 100644
--- a/include/hotkey_store.h
+++ b/include/hotkey_store.h
@@ -71,6 +71,15 @@ public:
m_orig = m_changed;
}
+ /**
+ * @brief Return true if the hotkey doesn't match the original (i.e. it
+ * has been changed)
+ */
+ bool HasUnsavedChange() const
+ {
+ return m_orig.m_KeyCode != m_changed.m_KeyCode;
+ }
+
const wxString& GetSectionTag() const
{
return m_tag;
--
2.19.0
From 1a12c5306a91672badb960ede17555ca8baa328d Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 27 Sep 2018 13:31:24 +0100
Subject: [PATCH 5/6] 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 55cd903e1..62f78f75f 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 b025ce891..fd5b5ec90 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 afccc55d7..21e0dc5a8 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 78a6216b1..a0f93a861 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:
/**
--
2.19.0
From 1700b314525599e1c65199f924447a8b3c588138 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Wed, 26 Sep 2018 15:26:26 +0100
Subject: [PATCH 1/6] Separate storage at iteration of hotkeys from the HK list
widget
This separates the "ground truth" store of hotkeys from what is shown
in the dialog. This will allow us to filter the displayed hotkeys
while keeping the same underlying data structures.
Now, the UI data items interact with an intermediate set of data, which
represents the "original" hotkey data, and the "changed" data. The
ultimate aim here is to allow UI elements to come and go, but the
hotkeys that are "in-edit" are preserved.
This also allows us to abstract some bookkeeping complexity
out of the WIDGET_HOTKEY_LIST class into a separate non-GUI
class.
---
common/CMakeLists.txt | 1 +
common/dialogs/panel_hotkeys_editor.cpp | 10 +-
common/hotkey_store.cpp | 129 ++++++++++++++
common/widgets/widget_hotkey_list.cpp | 221 +++++-------------------
include/hotkey_store.h | 168 ++++++++++++++++++
include/panel_hotkeys_editor.h | 3 +
include/widgets/widget_hotkey_list.h | 60 ++-----
7 files changed, 363 insertions(+), 229 deletions(-)
create mode 100644 common/hotkey_store.cpp
create mode 100644 include/hotkey_store.h
diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
index 0be12d5f5..eccf7302f 100644
--- a/common/CMakeLists.txt
+++ b/common/CMakeLists.txt
@@ -283,6 +283,7 @@ set( COMMON_SRCS
gr_basic.cpp
grid_tricks.cpp
hash_eda.cpp
+ hotkey_store.cpp
hotkeys_basic.cpp
html_messagebox.cpp
incremental_text_ctrl.cpp
diff --git a/common/dialogs/panel_hotkeys_editor.cpp b/common/dialogs/panel_hotkeys_editor.cpp
index 662633ea3..6059d5465 100644
--- a/common/dialogs/panel_hotkeys_editor.cpp
+++ b/common/dialogs/panel_hotkeys_editor.cpp
@@ -32,10 +32,10 @@ PANEL_HOTKEYS_EDITOR::PANEL_HOTKEYS_EDITOR( EDA_BASE_FRAME* aFrame, wxWindow* aW
m_frame( aFrame ),
m_hotkeys( aHotkeys ),
m_showHotkeys( aShowHotkeys ),
- m_nickname( aNickname )
+ m_nickname( aNickname ),
+ m_hotkeyStore( aShowHotkeys )
{
- HOTKEY_SECTIONS sections = WIDGET_HOTKEY_LIST::GenSections( m_showHotkeys );
- m_hotkeyListCtrl = new WIDGET_HOTKEY_LIST( m_panelHotkeys, sections );
+ m_hotkeyListCtrl = new WIDGET_HOTKEY_LIST( m_panelHotkeys, m_hotkeyStore );
m_hotkeyListCtrl->InstallOnPanel( m_panelHotkeys );
}
@@ -60,12 +60,12 @@ bool PANEL_HOTKEYS_EDITOR::TransferDataFromWindow()
void PANEL_HOTKEYS_EDITOR::ResetClicked( wxCommandEvent& aEvent )
{
- m_hotkeyListCtrl->TransferDataToControl();
+ m_hotkeyListCtrl->ResetAllHotkeys( false );
}
void PANEL_HOTKEYS_EDITOR::DefaultsClicked( wxCommandEvent& aEvent )
{
- m_hotkeyListCtrl->TransferDefaultsToControl();
+ m_hotkeyListCtrl->ResetAllHotkeys( true );
}
diff --git a/common/hotkey_store.cpp b/common/hotkey_store.cpp
new file mode 100644
index 000000000..62e9fc424
--- /dev/null
+++ b/common/hotkey_store.cpp
@@ -0,0 +1,129 @@
+/*
+ * This program source code file is part of KICAD, a free EDA CAD application.
+ *
+ * Copyright (C) 1992-2018 Kicad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#include <hotkey_store.h>
+
+HOTKEY_STORE::HOTKEY_STORE( EDA_HOTKEY_CONFIG* aHotkeys )
+{
+ for( EDA_HOTKEY_CONFIG* section = aHotkeys; section->m_HK_InfoList; ++section )
+ {
+ m_hk_sections.push_back( genSection( *section ) );
+ }
+}
+
+
+HOTKEY_SECTION HOTKEY_STORE::genSection( EDA_HOTKEY_CONFIG& aSection )
+{
+ HOTKEY_SECTION generated_section { {}, {}, aSection };
+
+ generated_section.m_name = wxGetTranslation( *aSection.m_Title );
+
+ for( EDA_HOTKEY** info_ptr = aSection.m_HK_InfoList; *info_ptr; ++info_ptr )
+ {
+ generated_section.m_hotkeys.push_back( { **info_ptr, *aSection.m_SectionTag } );
+ }
+
+ return generated_section;
+}
+
+
+std::vector<HOTKEY_SECTION>& HOTKEY_STORE::GetSections()
+{
+ return m_hk_sections;
+}
+
+
+void HOTKEY_STORE::SaveAllHotkeys()
+{
+ for( auto& section: m_hk_sections )
+ {
+ for( auto& hotkey: section.m_hotkeys )
+ {
+ hotkey.SaveHotkey();
+ }
+ }
+}
+
+
+void HOTKEY_STORE::ResetAllHotkeysToDefault()
+{
+ for( auto& section: m_hk_sections )
+ {
+ for( auto& hotkey: section.m_hotkeys )
+ {
+ hotkey.GetCurrentValue().ResetKeyCodeToDefault();
+ }
+ }
+}
+
+
+void HOTKEY_STORE::ResetAllHotkeysToOriginal()
+{
+ for( auto& section: m_hk_sections )
+ {
+ for( auto& hotkey: section.m_hotkeys )
+ {
+ hotkey.GetCurrentValue().m_KeyCode = hotkey.GetOriginalValue().m_KeyCode;
+ }
+ }
+}
+
+
+bool HOTKEY_STORE::CheckKeyConflicts( long aKey, const wxString& aSectionTag,
+ EDA_HOTKEY** aConfKey, EDA_HOTKEY_CONFIG** aConfSect )
+{
+ EDA_HOTKEY* conflicting_key = nullptr;
+ EDA_HOTKEY_CONFIG* conflicting_section = nullptr;
+
+ for( auto& section: m_hk_sections )
+ {
+ const auto& sectionTag = *section.m_section.m_SectionTag;
+ if( aSectionTag != g_CommonSectionTag
+ && sectionTag != g_CommonSectionTag
+ && sectionTag != aSectionTag )
+ {
+ // This key and its conflict candidate are in orthogonal sections, so skip.
+ continue;
+ }
+
+ // See if any *current* hotkeys are in conflict
+ for( auto& hotkey: section.m_hotkeys )
+ {
+ auto& curr_hk = hotkey.GetCurrentValue();
+ if( aKey == curr_hk.m_KeyCode )
+ {
+ conflicting_key = &curr_hk;
+ conflicting_section = §ion.m_section;
+ }
+ }
+ }
+
+ // Write the outparams
+ if( aConfKey )
+ *aConfKey = conflicting_key;
+
+ if( aConfSect )
+ *aConfSect = conflicting_section;
+
+ return conflicting_key == nullptr;
+}
\ No newline at end of file
diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp
index 3c3871f70..9e38502aa 100644
--- a/common/widgets/widget_hotkey_list.cpp
+++ b/common/widgets/widget_hotkey_list.cpp
@@ -51,24 +51,25 @@ enum ID_WHKL_MENU_IDS
};
+
+
+
/**
* Class WIDGET_HOTKEY_CLIENT_DATA
- * Stores the hotkey and section tag associated with each row. To change a
- * hotkey, edit it in the row's client data, then call WIDGET_HOTKEY_LIST::UpdateFromClientData().
+ * Stores the hotkey change data associated with each row. To change a
+ * hotkey, edit it via GetCurrentValue() in the row's client data, then call
+ * WIDGET_HOTKEY_LIST::UpdateFromClientData().
*/
class WIDGET_HOTKEY_CLIENT_DATA : public wxClientData
{
- EDA_HOTKEY m_hotkey;
- wxString m_section_tag;
+ CHANGED_HOTKEY& m_changed_hotkey;
public:
- WIDGET_HOTKEY_CLIENT_DATA( const EDA_HOTKEY& aHotkey, const wxString& aSectionTag )
- : m_hotkey( aHotkey ), m_section_tag( aSectionTag )
+ WIDGET_HOTKEY_CLIENT_DATA( CHANGED_HOTKEY& aChangedHotkey )
+ : m_changed_hotkey( aChangedHotkey )
{}
-
- EDA_HOTKEY& GetHotkey() { return m_hotkey; }
- const wxString& GetSectionTag() const { return m_section_tag; }
+ CHANGED_HOTKEY& GetChangedHotkey() { return m_changed_hotkey; }
};
@@ -261,7 +262,7 @@ void WIDGET_HOTKEY_LIST::UpdateFromClientData()
if( hkdata )
{
- EDA_HOTKEY& hk = hkdata->GetHotkey();
+ const EDA_HOTKEY& hk = hkdata->GetChangedHotkey().GetCurrentValue();
SetItemText( i, 0, wxGetTranslation( hk.m_InfoMsg ) );
SetItemText( i, 1, KeyNameFromKeyCode( hk.m_KeyCode ) );
@@ -270,19 +271,6 @@ void WIDGET_HOTKEY_LIST::UpdateFromClientData()
}
-void WIDGET_HOTKEY_LIST::LoadSection( EDA_HOTKEY_CONFIG* aSection )
-{
- HOTKEY_LIST list;
-
- for( EDA_HOTKEY** info_ptr = aSection->m_HK_InfoList; *info_ptr; ++info_ptr )
- {
- list.push_back( **info_ptr );
- }
-
- m_hotkeys.push_back( list );
-}
-
-
void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
{
WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
@@ -305,14 +293,17 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
bool exists;
KeyNameFromKeyCode( key, &exists );
- if( exists && hkdata->GetHotkey().m_KeyCode != key )
+ auto& changed_hk = hkdata->GetChangedHotkey();
+ auto& curr_hk = changed_hk.GetCurrentValue();
+
+ if( exists && curr_hk.m_KeyCode != key )
{
- wxString tag = hkdata->GetSectionTag();
+ wxString tag = changed_hk.GetSectionTag();
bool canUpdate = ResolveKeyConflicts( key, tag );
if( canUpdate )
{
- hkdata->GetHotkey().m_KeyCode = key;
+ curr_hk.m_KeyCode = key;
}
}
@@ -328,27 +319,7 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
{
WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
- EDA_HOTKEY* hk = &hkdata->GetHotkey();
-
- for( size_t sec_index = 0; sec_index < m_sections.size(); ++sec_index )
- {
- wxString& section_tag = *( m_sections[sec_index].m_section->m_SectionTag );
-
- if( section_tag != hkdata->GetSectionTag() )
- continue;
-
- HOTKEY_LIST& each_list = m_hotkeys[sec_index];
- HOTKEY_LIST::iterator hk_it;
-
- for( hk_it = each_list.begin(); hk_it != each_list.end(); ++hk_it )
- {
- if( hk_it->m_Idcommand == hk->m_Idcommand )
- {
- hk->m_KeyCode = hk_it->m_KeyCode;
- break;
- }
- }
- }
+ hkdata->GetChangedHotkey().ResetHotkey();
UpdateFromClientData();
}
@@ -357,8 +328,8 @@ void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
void WIDGET_HOTKEY_LIST::ResetItemToDefault( wxTreeListItem aItem )
{
WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
- EDA_HOTKEY* hk = &hkdata->GetHotkey();
- hk->ResetKeyCodeToDefault();
+ hkdata->GetChangedHotkey().GetCurrentValue().ResetKeyCodeToDefault();
+
UpdateFromClientData();
}
@@ -404,11 +375,11 @@ void WIDGET_HOTKEY_LIST::OnMenu( wxCommandEvent& aEvent )
break;
case ID_RESET_ALL:
- TransferDataToControl();
+ ResetAllHotkeys( false );
break;
case ID_DEFAULT_ALL:
- TransferDefaultsToControl();
+ ResetAllHotkeys( true );
break;
default:
@@ -417,67 +388,14 @@ void WIDGET_HOTKEY_LIST::OnMenu( wxCommandEvent& aEvent )
}
-bool WIDGET_HOTKEY_LIST::CheckKeyConflicts( long aKey, const wxString& aSectionTag,
- EDA_HOTKEY** aConfKey, EDA_HOTKEY_CONFIG** aConfSect )
-{
- EDA_HOTKEY* conflicting_key = NULL;
- struct EDA_HOTKEY_CONFIG* conflicting_section = NULL;
-
- for( wxTreeListItem item = GetFirstItem(); item.IsOk(); item = GetNextItem( item ) )
- {
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( item );
-
- if( !hkdata )
- continue;
-
- EDA_HOTKEY& hk = hkdata->GetHotkey();
- wxString tag = hkdata->GetSectionTag();
-
- if( aSectionTag != g_CommonSectionTag
- && tag != g_CommonSectionTag
- && tag != aSectionTag )
- {
- // This key and its conflict candidate are in orthogonal sections, so skip.
- continue;
- }
-
- if( aKey == hk.m_KeyCode )
- {
- conflicting_key = &hk;
-
- // Find the section
- HOTKEY_SECTIONS::iterator it;
-
- for( it = m_sections.begin(); it != m_sections.end(); ++it )
- {
- if( *it->m_section->m_SectionTag == tag )
- {
- conflicting_section = it->m_section;
- break;
- }
- }
- }
- }
-
- // Write the outparams
- if( aConfKey )
- *aConfKey = conflicting_key;
-
- if( aConfSect )
- *aConfSect = conflicting_section;
-
- return conflicting_key == NULL;
-}
-
-
bool WIDGET_HOTKEY_LIST::ResolveKeyConflicts( long aKey, const wxString& aSectionTag )
{
- EDA_HOTKEY* conflicting_key = NULL;
- EDA_HOTKEY_CONFIG* conflicting_section = NULL;
+ EDA_HOTKEY* conflicting_key = nullptr;
+ EDA_HOTKEY_CONFIG* conflicting_section = nullptr;
- CheckKeyConflicts( aKey, aSectionTag, &conflicting_key, &conflicting_section );
+ m_hk_store.CheckKeyConflicts( aKey, aSectionTag, &conflicting_key, &conflicting_section );
- if( conflicting_key != NULL )
+ if( conflicting_key != nullptr )
{
wxString info = wxGetTranslation( conflicting_key->m_InfoMsg );
wxString msg = wxString::Format(
@@ -490,6 +408,7 @@ bool WIDGET_HOTKEY_LIST::ResolveKeyConflicts( long aKey, const wxString& aSectio
if( dlg.ShowModal() == wxID_YES )
{
+ // Reset the other hotkey
conflicting_key->m_KeyCode = 0;
UpdateFromClientData();
return true;
@@ -506,9 +425,9 @@ bool WIDGET_HOTKEY_LIST::ResolveKeyConflicts( long aKey, const wxString& aSectio
}
-WIDGET_HOTKEY_LIST::WIDGET_HOTKEY_LIST( wxWindow* aParent, const HOTKEY_SECTIONS& aSections )
+WIDGET_HOTKEY_LIST::WIDGET_HOTKEY_LIST( wxWindow* aParent, HOTKEY_STORE& aHotkeyStore )
: TWO_COLUMN_TREE_LIST( aParent, wxID_ANY, wxDefaultPosition, wxDefaultSize, wxTL_SINGLE ),
- m_sections( aSections )
+ m_hk_store( aHotkeyStore )
{
AppendColumn( _( "Command (double-click to edit)" ) );
AppendColumn( _( "Hotkey" ) );
@@ -521,22 +440,6 @@ WIDGET_HOTKEY_LIST::WIDGET_HOTKEY_LIST( wxWindow* aParent, const HOTKEY_SECTIONS
}
-HOTKEY_SECTIONS WIDGET_HOTKEY_LIST::GenSections( EDA_HOTKEY_CONFIG* aHotkeys )
-{
- HOTKEY_SECTIONS sections;
-
- for( EDA_HOTKEY_CONFIG* section = aHotkeys; section->m_HK_InfoList; ++section )
- {
- HOTKEY_SECTION sec;
- sec.m_name = wxGetTranslation( *section->m_Title );
- sec.m_section = section;
- sections.push_back( sec );
- }
-
- return sections;
-}
-
-
void WIDGET_HOTKEY_LIST::InstallOnPanel( wxPanel* aPanel )
{
wxBoxSizer* sizer = new wxBoxSizer( wxVERTICAL );
@@ -546,50 +449,43 @@ void WIDGET_HOTKEY_LIST::InstallOnPanel( wxPanel* aPanel )
}
-bool WIDGET_HOTKEY_LIST::TransferDefaultsToControl()
+void WIDGET_HOTKEY_LIST::ResetAllHotkeys( bool aResetToDefault )
{
Freeze();
- for( wxTreeListItem item = GetFirstItem(); item.IsOk(); item = GetNextItem( item ) )
+ // Reset all the hotkeys, not just the ones shown
+ // Should not need to check conflicts, as the state we're about
+ // to set to a should be consistent
+ if( aResetToDefault )
{
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( item );
- if( hkdata == NULL)
- continue;
-
- hkdata->GetHotkey().ResetKeyCodeToDefault();
+ m_hk_store.ResetAllHotkeysToDefault();
+ }
+ else
+ {
+ m_hk_store.ResetAllHotkeysToOriginal();
}
UpdateFromClientData();
Thaw();
-
- return true;
}
+
+
bool WIDGET_HOTKEY_LIST::TransferDataToControl()
{
Freeze();
DeleteAllItems();
- m_hotkeys.clear();
- for( size_t sec_index = 0; sec_index < m_sections.size(); ++sec_index )
+ for( auto& section: m_hk_store.GetSections() )
{
- // LoadSection pushes into m_hotkeys
- LoadSection( m_sections[sec_index].m_section );
- wxASSERT( m_hotkeys.size() == sec_index + 1 );
-
- wxString section_tag = *( m_sections[sec_index].m_section->m_SectionTag );
-
// Create parent tree item
- wxTreeListItem parent = AppendItem( GetRootItem(), m_sections[sec_index].m_name );
-
- HOTKEY_LIST& each_list = m_hotkeys[sec_index];
- HOTKEY_LIST::iterator hk_it;
+ wxTreeListItem parent = AppendItem( GetRootItem(), section.m_name );
- for( hk_it = each_list.begin(); hk_it != each_list.end(); ++hk_it )
+ for( auto& hotkey: section.m_hotkeys )
{
wxTreeListItem item = AppendItem( parent, wxEmptyString );
- SetItemData( item, new WIDGET_HOTKEY_CLIENT_DATA( &*hk_it, section_tag ) );
+ SetItemData( item, new WIDGET_HOTKEY_CLIENT_DATA( hotkey ) );
}
Expand( parent );
@@ -604,32 +500,7 @@ bool WIDGET_HOTKEY_LIST::TransferDataToControl()
bool WIDGET_HOTKEY_LIST::TransferDataFromControl()
{
- for( size_t sec_index = 0; sec_index < m_sections.size(); ++sec_index )
- {
- EDA_HOTKEY_CONFIG* section = m_sections[sec_index].m_section;
-
- for( EDA_HOTKEY** info_ptr = section->m_HK_InfoList; *info_ptr; ++info_ptr )
- {
- EDA_HOTKEY* info = *info_ptr;
-
- for( wxTreeListItem item = GetFirstItem(); item.IsOk(); item = GetNextItem( item ) )
- {
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( item );
-
- if( !hkdata )
- continue;
-
- EDA_HOTKEY& hk = hkdata->GetHotkey();
-
- if( hk.m_Idcommand == info->m_Idcommand )
- {
- info->m_KeyCode = hk.m_KeyCode;
- break;
- }
- }
- }
- }
-
+ m_hk_store.SaveAllHotkeys();
return true;
}
diff --git a/include/hotkey_store.h b/include/hotkey_store.h
new file mode 100644
index 000000000..13b985b36
--- /dev/null
+++ b/include/hotkey_store.h
@@ -0,0 +1,168 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 2016-2018 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 3
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+/**
+ * @file hotkey_store.h
+ */
+
+#ifndef HOTKEY_STORE__H
+#define HOTKEY_STORE__H
+
+
+#include <hotkeys_basic.h>
+
+
+/**
+ * Class that manages a hotkey that can be changed, reset to its
+ * old value, a default or saved.
+ */
+class CHANGED_HOTKEY
+{
+public:
+ CHANGED_HOTKEY( EDA_HOTKEY& aHotkey, const wxString& aTag ):
+ m_orig( aHotkey ),
+ m_changed( aHotkey ),
+ m_tag( aTag )
+ {}
+
+ EDA_HOTKEY& GetCurrentValue()
+ {
+ return m_changed;
+ }
+
+ const EDA_HOTKEY& GetCurrentValue() const
+ {
+ return m_changed;
+ }
+
+ /**
+ * Reset the changed hotkey back to the original value.
+ */
+ void ResetHotkey()
+ {
+ m_changed = m_orig;
+ }
+
+ /**
+ * Save changed hotkey to the original location.
+ */
+ void SaveHotkey()
+ {
+ m_orig = m_changed;
+ }
+
+ const wxString& GetSectionTag() const
+ {
+ return m_tag;
+ }
+
+private:
+ // Reference to an "original" hotkey config
+ EDA_HOTKEY& m_orig;
+
+ // A separate changeable config
+ EDA_HOTKEY m_changed;
+
+ // The hotkey section tag, used to spot conflicts
+ const wxString& m_tag;
+
+};
+
+/**
+ * Associates a set of hotkeys (a section) with a display name and the hotkeys
+ */
+struct HOTKEY_SECTION
+{
+ // The displayed, translated, name of the section
+ wxString m_name;
+
+ // List of update-able hotkey data for this section
+ std::vector<CHANGED_HOTKEY> m_hotkeys;
+
+ // Back reference to the underlying hotkey data of this section
+ EDA_HOTKEY_CONFIG& m_section;
+};
+
+
+/**
+ * A class that contains a set of hotkeys, arranged into "sections"
+ * and provides some book-keeping functions for them.
+ */
+class HOTKEY_STORE
+{
+public:
+
+ using SECTION_LIST = std::vector<HOTKEY_SECTION>;
+
+ /**
+ * Construct a HOTKEY_STORE from a list of hotkey sections
+ *
+ * @param aHotkeys the hotkey configs that will be managed by this store.
+ */
+ HOTKEY_STORE( EDA_HOTKEY_CONFIG* aHotkeys );
+
+ /**
+ * Get the list of sections managed by this store
+ */
+ SECTION_LIST& GetSections();
+
+ /**
+ * Persist all changes to hotkeys in the store to the underlying
+ * data structures.
+ */
+ void SaveAllHotkeys();
+
+ /**
+ * Reset every hotkey in the store to the default values
+ */
+ void ResetAllHotkeysToDefault();
+
+ /**
+ * Resets every hotkey to the original values.
+ */
+ void ResetAllHotkeysToOriginal();
+
+ /**
+ * Check whether the given key conflicts with anything in this store.
+ *
+ * @param aKey - key to check
+ * @param aSectionTag - section tag into which the key is proposed to be installed
+ * @param aConfKey - if not NULL, outparam getting the key this one conflicts with
+ * @param aConfSect - if not NULL, outparam getting the section this one conflicts with
+ */
+ bool CheckKeyConflicts( long aKey, const wxString& aSectionTag,
+ EDA_HOTKEY** aConfKey, EDA_HOTKEY_CONFIG** aConfSect );
+
+private:
+
+ /**
+ * Generate a HOTKEY_SECTION for a single section
+ * described by an EDA_HOTKEY_CONFIG
+ */
+ HOTKEY_SECTION genSection( EDA_HOTKEY_CONFIG& aSection );
+
+ // Internal data for every hotkey passed in
+ SECTION_LIST m_hk_sections;
+};
+
+#endif // HOTKEY_STORE__H
\ No newline at end of file
diff --git a/include/panel_hotkeys_editor.h b/include/panel_hotkeys_editor.h
index 5087b8666..576277f4e 100644
--- a/include/panel_hotkeys_editor.h
+++ b/include/panel_hotkeys_editor.h
@@ -25,6 +25,8 @@
#define PANEL_HOTKEYS_EDITOR_H
#include <hotkeys_basic.h>
+#include <hotkey_store.h>
+
#include "../common/dialogs/panel_hotkeys_editor_base.h"
#include <widgets/widget_hotkey_list.h>
@@ -37,6 +39,7 @@ protected:
struct EDA_HOTKEY_CONFIG* m_showHotkeys;
wxString m_nickname;
+ HOTKEY_STORE m_hotkeyStore;
WIDGET_HOTKEY_LIST* m_hotkeyListCtrl;
bool TransferDataToWindow() override;
diff --git a/include/widgets/widget_hotkey_list.h b/include/widgets/widget_hotkey_list.h
index dff73f03e..36f2bb84e 100644
--- a/include/widgets/widget_hotkey_list.h
+++ b/include/widgets/widget_hotkey_list.h
@@ -36,26 +36,15 @@
#include <widgets/two_column_tree_list.h>
#include <hotkeys_basic.h>
+#include <hotkey_store.h>
-/**
- * struct HOTKEY_SECTION
- * Associates a hotkey configuration with a name.
- */
-struct HOTKEY_SECTION
-{
- wxString m_name;
- EDA_HOTKEY_CONFIG* m_section;
-};
-
-typedef std::vector<HOTKEY_SECTION> HOTKEY_SECTIONS;
-typedef std::vector<EDA_HOTKEY> HOTKEY_LIST;
class WIDGET_HOTKEY_CLIENT_DATA;
class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST
{
- HOTKEY_SECTIONS m_sections;
- std::vector<HOTKEY_LIST> m_hotkeys;
+ HOTKEY_STORE& m_hk_store;
+
wxTreeListItem m_context_menu_item;
/**
@@ -79,12 +68,6 @@ class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST
void UpdateFromClientData();
protected:
- /**
- * Method LoadSection
- * Generates a HOTKEY_LIST from the given hotkey configuration array and pushes
- * it to m_hotkeys.
- */
- void LoadSection( EDA_HOTKEY_CONFIG* aSection );
/**
* Method EditItem
@@ -128,18 +111,6 @@ protected:
*/
void OnSize( wxSizeEvent& aEvent );
- /**
- * Method CheckKeyConflicts
- * Check whether the given key conflicts with anything in this WIDGET_HOTKEY_LIST.
- *
- * @param aKey - key to check
- * @param aSectionTag - section tag into which the key is proposed to be installed
- * @param aConfKey - if not NULL, outparam getting the key this one conflicts with
- * @param aConfSect - if not NULL, outparam getting the section this one conflicts with
- */
- bool CheckKeyConflicts( long aKey, const wxString& aSectionTag,
- EDA_HOTKEY** aConfKey, EDA_HOTKEY_CONFIG** aConfSect );
-
/**
* Method ResolveKeyConflicts
* Check if we can set a hotkey, and prompt the user if there is a conflict between
@@ -163,18 +134,10 @@ public:
* Create a WIDGET_HOTKEY_LIST.
*
* @param aParent - parent widget
- * @param aSections - list of the hotkey sections to display and their names.
- * See WIDGET_HOTKEY_LIST::GenSections for a way to generate these easily
- * from an EDA_HOTKEY_CONFIG*.
+ * @param aHotkeys - EDA_HOTKEY_CONFIG data - a hotkey store is constructed
+ * from this.
*/
- WIDGET_HOTKEY_LIST( wxWindow* aParent, const HOTKEY_SECTIONS& aSections );
-
- /**
- * Static method GenSections
- * Generate a list of sections and names from an EDA_HOTKEY_CONFIG*. Titles
- * will be looked up from translations.
- */
- static HOTKEY_SECTIONS GenSections( EDA_HOTKEY_CONFIG* aHotkeys );
+ WIDGET_HOTKEY_LIST( wxWindow* aParent, HOTKEY_STORE& aHotkeyStore );
/**
* Method InstallOnPanel
@@ -185,16 +148,15 @@ public:
void InstallOnPanel( wxPanel* aPanel );
/**
- * Method TransferDefaultsToControl
- * Set hotkeys in the control to default values.
- * @return true iff the operation was successful
+ * Set hotkeys in the control to default or original values.
+ * @param aResetToDefault if true,.reset to the defaults inherent to the
+ * hotkeym, else reset to the value they had when the dialog was invoked.
*/
- bool TransferDefaultsToControl();
+ void ResetAllHotkeys( bool aResetToDefault );
/**
* Method TransferDataToControl
- * Load the hotkey data into the control. It is safe to call this multiple times,
- * for example to reset the control.
+ * Load the hotkey data from the store into the control.
* @return true iff the operation was successful
*/
bool TransferDataToControl();
--
2.19.0
From 3eba9e2a53799173e3d424d7f40169f98b0155b8 Mon Sep 17 00:00:00 2001
From: John Beard <john.j.beard@xxxxxxxxx>
Date: Thu, 27 Sep 2018 14:44:03 +0100
Subject: [PATCH 6/6] Prevent segfault when undoing or resetting non-hotkey
rows
This is caused by:
* Not checking the hotkey data is not null when performing a
hotkey action
* Allowing hotkey actions on non-hotkey rows.
This fixes both of these, and adds an assert to warn if someone
does manage to fire a hotkey action on a non-hotkey row (but it
won't crash).
Fixes: lp:1794756
* https://bugs.launchpad.net/kicad/+bug/1794756
---
common/widgets/widget_hotkey_list.cpp | 44 ++++++++++++++++++++-------
include/widgets/widget_hotkey_list.h | 7 +++++
2 files changed, 40 insertions(+), 11 deletions(-)
diff --git a/common/widgets/widget_hotkey_list.cpp b/common/widgets/widget_hotkey_list.cpp
index 62f78f75f..cb7f31def 100644
--- a/common/widgets/widget_hotkey_list.cpp
+++ b/common/widgets/widget_hotkey_list.cpp
@@ -300,6 +300,18 @@ WIDGET_HOTKEY_CLIENT_DATA* WIDGET_HOTKEY_LIST::GetSelHKClientData()
}
+WIDGET_HOTKEY_CLIENT_DATA* WIDGET_HOTKEY_LIST::getExpectedHkClientData( wxTreeListItem aItem )
+{
+ const auto hkdata = GetHKClientData( aItem );
+
+ // This probably means a hotkey-only action is being attempted on
+ // a row that is not a hotkey (like a section heading)
+ wxASSERT_MSG( hkdata != nullptr, "No hotkey data found for list item" );
+
+ return hkdata;
+}
+
+
void WIDGET_HOTKEY_LIST::UpdateFromClientData()
{
for( wxTreeListItem i = GetFirstItem(); i.IsOk(); i = GetNextItem( i ) )
@@ -351,13 +363,10 @@ void WIDGET_HOTKEY_LIST::changeHotkey( CHANGED_HOTKEY& aHotkey, long aKey )
void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
{
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
+ WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem );
if( !hkdata )
- {
- // Activated item was not a hotkey row
return;
- }
wxString name = GetItemText( aItem, 0 );
wxString current_key = GetItemText( aItem, 1 );
@@ -365,7 +374,7 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
wxKeyEvent key_event = HK_PROMPT_DIALOG::PromptForKey( GetParent(), name, current_key );
long key = MapKeypressToKeycode( key_event );
- if( hkdata && key )
+ if( key )
{
changeHotkey( hkdata->GetChangedHotkey(), key );
UpdateFromClientData();
@@ -375,7 +384,10 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
{
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
+ WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem );
+
+ if( !hkdata )
+ return;
auto& changed_hk = hkdata->GetChangedHotkey();
const auto& orig_hk = changed_hk.GetOriginalValue();
@@ -387,7 +399,10 @@ void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
void WIDGET_HOTKEY_LIST::ResetItemToDefault( wxTreeListItem aItem )
{
- WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( aItem );
+ WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem );
+
+ if( !hkdata )
+ return;
auto& changed_hk = hkdata->GetChangedHotkey();
@@ -409,10 +424,17 @@ void WIDGET_HOTKEY_LIST::OnContextMenu( wxTreeListEvent& aEvent )
wxMenu menu;
- menu.Append( ID_EDIT, _( "Edit..." ) );
- menu.Append( ID_RESET, _( "Undo Changes" ) );
- menu.Append( ID_DEFAULT, _( "Restore Default" ) );
- menu.Append( wxID_SEPARATOR );
+ WIDGET_HOTKEY_CLIENT_DATA* hkdata = GetHKClientData( m_context_menu_item );
+
+ // Some actions only apply if the row is hotkey data
+ if( hkdata )
+ {
+ menu.Append( ID_EDIT, _( "Edit..." ) );
+ menu.Append( ID_RESET, _( "Undo Changes" ) );
+ menu.Append( ID_DEFAULT, _( "Restore Default" ) );
+ menu.Append( wxID_SEPARATOR );
+ }
+
menu.Append( ID_RESET_ALL, _( "Undo All Changes" ) );
menu.Append( ID_DEFAULT_ALL, _( "Restore All to Default" ) );
diff --git a/include/widgets/widget_hotkey_list.h b/include/widgets/widget_hotkey_list.h
index a0f93a861..2e728d3c8 100644
--- a/include/widgets/widget_hotkey_list.h
+++ b/include/widgets/widget_hotkey_list.h
@@ -61,6 +61,13 @@ class WIDGET_HOTKEY_LIST : public TWO_COLUMN_TREE_LIST
*/
WIDGET_HOTKEY_CLIENT_DATA* GetSelHKClientData();
+ /**
+ * Get the WIDGET_HOTKEY_CLIENT_DATA form an item and assert if it isn't
+ * found. This is for use when the data not being present indicates an
+ * error.
+ */
+ WIDGET_HOTKEY_CLIENT_DATA* getExpectedHkClientData( wxTreeListItem aItem );
+
/**
* Method UpdateFromClientData
* Refresh the visible text on the widget from the rows' client data objects.
--
2.19.0
Follow ups
