Fix bugs with ACTIONs not being "honest" singletons.

Delete the copy ctor and assignment operator to start with, but
even then the separate apps each have their own statically allocated
copy of the common actions.  So we need to update all of them, which
also means having the kicad manager frame's set of actions on hand).

This changelist also adds a Clear Hotkey Assignment function since
the hotkeys set is now likely to be sparse with respect to the
actions.
This commit is contained in:
Jeff Young 2019-06-14 11:55:54 +01:00
parent 83ee51370c
commit b429dbfb88
14 changed files with 98 additions and 179 deletions

View File

@ -105,7 +105,7 @@ void PANEL_HOTKEYS_EDITOR::installButtons( wxSizer* aSizer )
const BUTTON_ROW_PANEL::BTN_DEF_LIST l_btn_defs = {
{
wxID_RESET,
_( "Reset Hotkeys" ),
_( "Undo All Changes" ),
_( "Undo all changes made so far in this dialog" ),
[this]( wxCommandEvent& ){
m_hotkeyListCtrl->ResetAllHotkeys( false );
@ -113,7 +113,7 @@ void PANEL_HOTKEYS_EDITOR::installButtons( wxSizer* aSizer )
},
{
wxID_ANY,
_( "Set to Defaults" ),
_( "Restore All to Defaults" ),
_( "Set all hotkeys to the built-in KiCad defaults" ),
[this]( wxCommandEvent& ){
m_hotkeyListCtrl->ResetAllHotkeys( true );
@ -184,8 +184,8 @@ void PANEL_HOTKEYS_EDITOR::ImportHotKeys()
{
for( HOTKEY& hotkey: section.m_HotKeys )
{
if( importedHotKeys.count( hotkey.m_Parent->GetName() ) )
hotkey.m_EditKeycode = importedHotKeys[ hotkey.m_Parent->GetName() ];
if( importedHotKeys.count( hotkey.m_Actions[ 0 ]->GetName() ) )
hotkey.m_EditKeycode = importedHotKeys[ hotkey.m_Actions[ 0 ]->GetName() ];
}
}

View File

@ -39,6 +39,7 @@
#include <tool/action_menu.h>
#include <tool/common_control.h>
#include <tool/tool_manager.h>
#include <tool/action_manager.h>
#include <menus_helpers.h>
#include <tool/actions.h>
@ -318,6 +319,8 @@ void EDA_BASE_FRAME::ShowChangedLanguage()
void EDA_BASE_FRAME::CommonSettingsChanged()
{
GetToolManager()->GetActionManager()->UpdateHotKeys( false );
if( GetMenuBar() )
{
// For icons in menus, icon scaling & hotkeys
@ -530,8 +533,10 @@ void EDA_BASE_FRAME::OnPreferences( wxCommandEvent& event )
}
// The Kicad manager frame is not a player so we have to add it by hand
if( IsType( KICAD_MAIN_FRAME_T ) )
InstallPreferences( &dlg, hotkeysPanel );
wxWindow* manager = wxFindWindowByName( KICAD_MANAGER_FRAME_NAME );
if( manager )
static_cast<EDA_BASE_FRAME*>( manager )->InstallPreferences( &dlg, hotkeysPanel );
if( dlg.ShowModal() == wxID_OK )
dlg.Kiway().CommonSettingsChanged();

View File

@ -22,6 +22,7 @@
*/
#include <hotkey_store.h>
#include <eda_base_frame.h>
#include <tool/tool_manager.h>
#include <tool/action_manager.h>
#include <tool/tool_event.h>
@ -38,16 +39,16 @@ public:
}
};
static GESTURE_PSEUDO_ACTION g_gesturePseudoActions[] = {
GESTURE_PSEUDO_ACTION( _( "Highlight Net" ), MD_CTRL + PSEUDO_WXK_LMB ),
GESTURE_PSEUDO_ACTION( _( "Clear Net Highlighting" ), MD_CTRL + PSEUDO_WXK_LMB ),
GESTURE_PSEUDO_ACTION( _( "Pan Left/Right" ), MD_CTRL + PSEUDO_WXK_WHEEL ),
GESTURE_PSEUDO_ACTION( _( "Pan Up/Down" ), MD_SHIFT + PSEUDO_WXK_WHEEL ),
GESTURE_PSEUDO_ACTION( _( "Finish Drawing" ), PSEUDO_WXK_DBLCLICK ),
GESTURE_PSEUDO_ACTION( _( "Add to Selection" ), MD_SHIFT + PSEUDO_WXK_LMB ),
GESTURE_PSEUDO_ACTION( _( "Remove from Selection" ), MD_CTRL + PSEUDO_WXK_LMB ),
GESTURE_PSEUDO_ACTION( _( "Ignore Grid Snaps" ), MD_ALT ),
GESTURE_PSEUDO_ACTION( _( "Ignore Other Snaps" ), MD_SHIFT ),
static GESTURE_PSEUDO_ACTION* g_gesturePseudoActions[] = {
new GESTURE_PSEUDO_ACTION( _( "Highlight Net" ), MD_CTRL + PSEUDO_WXK_LMB ),
new GESTURE_PSEUDO_ACTION( _( "Clear Net Highlighting" ), MD_CTRL + PSEUDO_WXK_LMB ),
new GESTURE_PSEUDO_ACTION( _( "Pan Left/Right" ), MD_CTRL + PSEUDO_WXK_WHEEL ),
new GESTURE_PSEUDO_ACTION( _( "Pan Up/Down" ), MD_SHIFT + PSEUDO_WXK_WHEEL ),
new GESTURE_PSEUDO_ACTION( _( "Finish Drawing" ), PSEUDO_WXK_DBLCLICK ),
new GESTURE_PSEUDO_ACTION( _( "Add to Selection" ), MD_SHIFT + PSEUDO_WXK_LMB ),
new GESTURE_PSEUDO_ACTION( _( "Remove from Selection" ), MD_CTRL + PSEUDO_WXK_LMB ),
new GESTURE_PSEUDO_ACTION( _( "Ignore Grid Snaps" ), MD_ALT ),
new GESTURE_PSEUDO_ACTION( _( "Ignore Other Snaps" ), MD_SHIFT ),
};
@ -88,8 +89,8 @@ void HOTKEY_STORE::Init( std::vector<TOOL_MANAGER*> aToolManagerList )
m_toolManagers = std::move( aToolManagerList );
// Collect all action maps into a single master map. This will re-group everything
// and elimate duplicates
std::map<std::string, TOOL_ACTION*> masterMap;
// and collect duplicates together
std::map<std::string, HOTKEY> masterMap;
for( TOOL_MANAGER* toolMgr : m_toolManagers )
{
@ -98,8 +99,10 @@ void HOTKEY_STORE::Init( std::vector<TOOL_MANAGER*> aToolManagerList )
// Internal actions probably shouldn't be allowed hotkeys
if( entry.second->GetLabel().IsEmpty() )
continue;
masterMap[ entry.first ] = entry.second;
HOTKEY& hotkey = masterMap[ entry.first ];
hotkey.m_Actions.push_back( entry.second );
hotkey.m_EditKeycode = entry.second->GetHotKey();
}
}
@ -108,14 +111,15 @@ void HOTKEY_STORE::Init( std::vector<TOOL_MANAGER*> aToolManagerList )
for( const auto& entry : masterMap )
{
wxString thisApp = GetAppName( entry.second );
TOOL_ACTION* entryAction = entry.second.m_Actions[ 0 ];
wxString entryApp = GetAppName( entryAction );
if( thisApp != currentApp )
if( entryApp != currentApp )
{
m_hk_sections.emplace_back( HOTKEY_SECTION() );
currentApp = thisApp;
currentApp = entryApp;
currentSection = &m_hk_sections.back();
currentSection->m_SectionName = GetSectionName( entry.second );
currentSection->m_SectionName = GetSectionName( entryAction );
}
currentSection->m_HotKeys.emplace_back( HOTKEY( entry.second ) );
@ -125,8 +129,8 @@ void HOTKEY_STORE::Init( std::vector<TOOL_MANAGER*> aToolManagerList )
currentSection = &m_hk_sections.back();
currentSection->m_SectionName = _( "Gestures" );
for( TOOL_ACTION& gesture : g_gesturePseudoActions )
currentSection->m_HotKeys.emplace_back( HOTKEY( &gesture ) );
for( TOOL_ACTION* gesture : g_gesturePseudoActions )
currentSection->m_HotKeys.emplace_back( HOTKEY( gesture ) );
}
@ -138,33 +142,33 @@ std::vector<HOTKEY_SECTION>& HOTKEY_STORE::GetSections()
void HOTKEY_STORE::SaveAllHotkeys()
{
for( HOTKEY_SECTION& section: m_hk_sections )
for( HOTKEY_SECTION& section : m_hk_sections )
{
for( HOTKEY& hotkey: section.m_HotKeys )
hotkey.m_Parent->SetHotKey( hotkey.m_EditKeycode );
for( HOTKEY& hotkey : section.m_HotKeys )
{
for( TOOL_ACTION* action : hotkey.m_Actions )
action->SetHotKey( hotkey.m_EditKeycode );
}
}
if( !m_toolManagers.empty() )
m_toolManagers[ 0 ]->GetActionManager()->UpdateHotKeys( false );
}
void HOTKEY_STORE::ResetAllHotkeysToDefault()
{
for( HOTKEY_SECTION& section: m_hk_sections )
for( HOTKEY_SECTION& section : m_hk_sections )
{
for( HOTKEY& hotkey: section.m_HotKeys )
hotkey.m_EditKeycode = hotkey.m_Parent->GetDefaultHotKey();
for( HOTKEY& hotkey : section.m_HotKeys )
hotkey.m_EditKeycode = hotkey.m_Actions[ 0 ]->GetDefaultHotKey();
}
}
void HOTKEY_STORE::ResetAllHotkeysToOriginal()
{
for( HOTKEY_SECTION& section: m_hk_sections )
for( HOTKEY_SECTION& section : m_hk_sections )
{
for( HOTKEY& hotkey: section.m_HotKeys )
hotkey.m_EditKeycode = hotkey.m_Parent->GetHotKey();
for( HOTKEY& hotkey : section.m_HotKeys )
hotkey.m_EditKeycode = hotkey.m_Actions[ 0 ]->GetHotKey();
}
}
@ -180,7 +184,7 @@ bool HOTKEY_STORE::CheckKeyConflicts( TOOL_ACTION* aAction, long aKey, HOTKEY**
for( HOTKEY& hotkey: section.m_HotKeys )
{
if( hotkey.m_Parent == aAction )
if( hotkey.m_Actions[ 0 ] == aAction )
continue;
if( hotkey.m_EditKeycode == aKey )

View File

@ -361,11 +361,8 @@ int WriteHotKeyConfig( const std::map<std::string, TOOL_ACTION*>& aActionMap )
// Overlay the current app's hotkey definitions onto the map
//
for( const auto& ii : aActionMap )
{
if( ii.second->GetHotKey() )
hotkeys[ ii.first ] = ii.second->GetHotKey();
}
hotkeys[ ii.first ] = ii.second->GetHotKey();
// Write entire hotkey set
//
wxFile file( fn.GetFullPath(), wxFile::OpenMode::write );

View File

@ -42,19 +42,13 @@ ACTION_MANAGER::ACTION_MANAGER( TOOL_MANAGER* aToolManager ) :
if( action->m_id == -1 )
action->m_id = MakeActionId( action->m_name );
RegisterAction( new TOOL_ACTION( *action ) );
RegisterAction( action );
}
}
ACTION_MANAGER::~ACTION_MANAGER()
{
while( !m_actionNameIndex.empty() )
{
TOOL_ACTION* action = m_actionNameIndex.begin()->second;
UnregisterAction( action );
delete action;
}
}
@ -71,22 +65,6 @@ void ACTION_MANAGER::RegisterAction( TOOL_ACTION* aAction )
}
void ACTION_MANAGER::UnregisterAction( TOOL_ACTION* aAction )
{
m_actionNameIndex.erase( aAction->m_name );
int hotkey = GetHotKey( *aAction );
if( hotkey )
{
std::list<TOOL_ACTION*>& actions = m_actionHotKeys[hotkey];
auto action = std::find( actions.begin(), actions.end(), aAction );
if( action != actions.end() )
actions.erase( action );
}
}
int ACTION_MANAGER::MakeActionId( const std::string& aActionName )
{
static int currentActionId = 1;
@ -207,9 +185,9 @@ void ACTION_MANAGER::UpdateHotKeys( bool aFullUpdate )
ReadHotKeyConfig( wxEmptyString, userHotKeyMap );
}
for( const auto& actionName : m_actionNameIndex )
for( const auto& ii : m_actionNameIndex )
{
TOOL_ACTION* action = actionName.second;
TOOL_ACTION* action = ii.second;
int hotkey = 0;
if( aFullUpdate )
@ -218,10 +196,9 @@ void ACTION_MANAGER::UpdateHotKeys( bool aFullUpdate )
hotkey = action->GetHotKey();
if( hotkey > 0 )
{
m_actionHotKeys[hotkey].push_back( action );
m_hotkeys[action->GetId()] = hotkey;
}
m_hotkeys[action->GetId()] = hotkey;
}
}
@ -232,10 +209,10 @@ int ACTION_MANAGER::processHotKey( TOOL_ACTION* aAction, std::map<std::string, i
aAction->m_hotKey = aAction->m_defaultHotKey;
if( !aAction->m_legacyName.empty() && aLegacyMap.count( aAction->m_legacyName ) )
aAction->m_hotKey = aLegacyMap[ aAction->m_legacyName ];
aAction->SetHotKey( aLegacyMap[ aAction->m_legacyName ] );
if( aHotKeyMap.count( aAction->m_name ) )
aAction->m_hotKey = aHotKeyMap[ aAction->m_name ];
aAction->SetHotKey( aHotKeyMap[ aAction->m_name ] );
return aAction->m_hotKey;
}

View File

@ -260,18 +260,6 @@ bool TOOL_MANAGER::InvokeTool( const std::string& aToolName )
}
void TOOL_MANAGER::RegisterAction( TOOL_ACTION* aAction )
{
m_actionMgr->RegisterAction( aAction );
}
void TOOL_MANAGER::UnregisterAction( TOOL_ACTION* aAction )
{
m_actionMgr->UnregisterAction( aAction );
}
bool TOOL_MANAGER::RunAction( const std::string& aActionName, bool aNow, void* aParam )
{
TOOL_ACTION* action = m_actionMgr->FindAction( aActionName );

View File

@ -44,8 +44,7 @@ enum ID_WHKL_MENU_IDS
ID_EDIT_HOTKEY = 2001,
ID_RESET,
ID_DEFAULT,
ID_RESET_ALL,
ID_DEFAULT_ALL,
ID_CLEAR
};
@ -159,9 +158,9 @@ public:
int key = aEvent.GetKeyCode();
for( size_t i = 0; i < sizeof( skipped_keys ) / sizeof( skipped_keys[0] ); ++i )
for( wxKeyCode skipped_key : skipped_keys )
{
if( key == skipped_keys[i] )
if( key == skipped_key )
return;
}
@ -209,14 +208,9 @@ public:
HK_PROMPT_DIALOG dialog( aParent, wxID_ANY, _( "Set Hotkey" ), aName, aCurrentKey );
if( dialog.ShowModal() == wxID_OK )
{
return dialog.m_event;
}
else
{
wxKeyEvent dummy;
return dummy;
}
return wxKeyEvent();
}
};
@ -235,7 +229,6 @@ public:
m_valid = m_normalised_filter_str.size() > 0;
}
/**
* Method FilterMatches
*
@ -249,7 +242,7 @@ public:
return true;
// Match in the (translated) filter string
const auto normedInfo = wxGetTranslation( aHotkey.m_Parent->GetLabel() ).Upper();
const auto normedInfo = wxGetTranslation( aHotkey.m_Actions[ 0 ]->GetLabel() ).Upper();
if( normedInfo.Contains( m_normalised_filter_str ) )
return true;
@ -302,14 +295,14 @@ void WIDGET_HOTKEY_LIST::UpdateFromClientData()
if( hkdata )
{
const auto& changed_hk = hkdata->GetChangedHotkey();
wxString label = changed_hk.m_Parent->GetLabel();
wxString label = changed_hk.m_Actions[ 0 ]->GetLabel();
wxString key_text = KeyNameFromKeyCode( changed_hk.m_EditKeycode );
if( label.IsEmpty() )
label = changed_hk.m_Parent->GetName();
label = changed_hk.m_Actions[ 0 ]->GetName();
// mark unsaved changes
if( changed_hk.m_EditKeycode != changed_hk.m_Parent->GetHotKey() )
if( changed_hk.m_EditKeycode != changed_hk.m_Actions[ 0 ]->GetHotKey() )
key_text += " *";
SetItemText( i, 0, label );
@ -331,7 +324,7 @@ void WIDGET_HOTKEY_LIST::changeHotkey( HOTKEY& aHotkey, long aKey )
if( exists && aHotkey.m_EditKeycode != aKey )
{
if( ResolveKeyConflicts( aHotkey.m_Parent, aKey ) )
if( aKey == 0 || ResolveKeyConflicts( aHotkey.m_Actions[ 0 ], aKey ) )
aHotkey.m_EditKeycode = aKey;
}
}
@ -358,7 +351,7 @@ void WIDGET_HOTKEY_LIST::EditItem( wxTreeListItem aItem )
}
void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem, int aResetId )
{
WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem );
@ -367,21 +360,13 @@ void WIDGET_HOTKEY_LIST::ResetItem( wxTreeListItem aItem )
auto& changed_hk = hkdata->GetChangedHotkey();
changeHotkey( changed_hk, changed_hk.m_Parent->GetHotKey() );
UpdateFromClientData();
}
if( aResetId == ID_RESET )
changeHotkey( changed_hk, changed_hk.m_Actions[ 0 ]->GetHotKey() );
else if( aResetId == ID_CLEAR )
changeHotkey( changed_hk, 0 );
else if( aResetId == ID_DEFAULT )
changeHotkey( changed_hk, changed_hk.m_Actions[ 0 ]->GetDefaultHotKey() );
void WIDGET_HOTKEY_LIST::ResetItemToDefault( wxTreeListItem aItem )
{
WIDGET_HOTKEY_CLIENT_DATA* hkdata = getExpectedHkClientData( aItem );
if( !hkdata )
return;
auto& changed_hk = hkdata->GetChangedHotkey();
changeHotkey( changed_hk, changed_hk.m_Parent->GetDefaultHotKey() );
UpdateFromClientData();
}
@ -406,14 +391,12 @@ void WIDGET_HOTKEY_LIST::OnContextMenu( wxTreeListEvent& aEvent )
{
menu.Append( ID_EDIT_HOTKEY, _( "Edit..." ) );
menu.Append( ID_RESET, _( "Undo Changes" ) );
menu.Append( ID_CLEAR, _( "Clear Assigned Hotkey" ) );
menu.Append( ID_DEFAULT, _( "Restore Default" ) );
menu.Append( wxID_SEPARATOR );
PopupMenu( &menu );
}
menu.Append( ID_RESET_ALL, _( "Undo All Changes" ) );
menu.Append( ID_DEFAULT_ALL, _( "Restore All to Default" ) );
PopupMenu( &menu );
}
@ -426,19 +409,9 @@ void WIDGET_HOTKEY_LIST::OnMenu( wxCommandEvent& aEvent )
break;
case ID_RESET:
ResetItem( m_context_menu_item );
break;
case ID_CLEAR:
case ID_DEFAULT:
ResetItemToDefault( m_context_menu_item );
break;
case ID_RESET_ALL:
ResetAllHotkeys( false );
break;
case ID_DEFAULT_ALL:
ResetAllHotkeys( true );
ResetItem( m_context_menu_item, aEvent.GetId() );
break;
default:
@ -456,7 +429,7 @@ bool WIDGET_HOTKEY_LIST::ResolveKeyConflicts( TOOL_ACTION* aAction, long aKey )
if( !conflictingHotKey )
return true;
TOOL_ACTION* conflictingAction = conflictingHotKey->m_Parent;
TOOL_ACTION* conflictingAction = conflictingHotKey->m_Actions[ 0 ];
wxString msg = wxString::Format( _( "\"%s\" is already assigned to \"%s\" in section \"%s\". "
"Are you sure you want to change its assignment?" ),
KeyNameFromKeyCode( aKey ),
@ -517,13 +490,9 @@ void WIDGET_HOTKEY_LIST::ResetAllHotkeys( bool aResetToDefault )
// Should not need to check conflicts, as the state we're about
// to set to a should be consistent
if( aResetToDefault )
{
m_hk_store.ResetAllHotkeysToDefault();
}
else
{
m_hk_store.ResetAllHotkeysToOriginal();
}
UpdateFromClientData();
Thaw();

View File

@ -52,6 +52,9 @@
#define KICAD_DEFAULT_DRAWFRAME_STYLE wxDEFAULT_FRAME_STYLE | wxWANTS_CHARS
#define KICAD_MANAGER_FRAME_NAME wxT( "KicadFrame" )
// Readability helper definitions for creating backup files.
#define CREATE_BACKUP_FILE true
#define NO_BACKUP_FILE false

View File

@ -32,13 +32,18 @@ class TOOL_MANAGER;
struct HOTKEY
{
TOOL_ACTION* m_Parent;
int m_EditKeycode;
std::vector<TOOL_ACTION*> m_Actions;
int m_EditKeycode;
HOTKEY( TOOL_ACTION* aParent ) :
m_Parent( aParent ),
m_EditKeycode( aParent->GetHotKey() )
HOTKEY() :
m_EditKeycode( 0 )
{ }
HOTKEY( TOOL_ACTION* aAction ) :
m_EditKeycode( aAction->GetHotKey() )
{
m_Actions.push_back( aAction );
}
};

View File

@ -62,13 +62,6 @@ public:
*/
void RegisterAction( TOOL_ACTION* aAction );
/**
* Function UnregisterAction()
* Removes a tool action from the manager and makes it unavailable for further usage.
* @param aAction: action to be removed.
*/
void UnregisterAction( TOOL_ACTION* aAction );
/**
* Generates an unique ID from for an action with given name.
*/

View File

@ -173,6 +173,10 @@ protected:
TOOL_ACTION_FLAGS m_flags;
void* m_param; // Generic parameter
private:
// TOOL_ACTIONS are singletons; don't be copying them around....
TOOL_ACTION( const TOOL_ACTION& ) = delete;
TOOL_ACTION& operator= ( const TOOL_ACTION& ) = delete;
};
#endif

View File

@ -94,23 +94,6 @@ public:
*/
bool InvokeTool( const std::string& aToolName );
/**
* Function RegisterAction()
* Registers an action that can be used to control tools (eg. invoke, trigger specific
* behaviours).
*
* @param aAction is the action to be registered.
*/
void RegisterAction( TOOL_ACTION* aAction );
/**
* Function UnregisterAction()
* Unregisters an action, so it is no longer active.
*
* @param aAction is the action to be unregistered.
*/
void UnregisterAction( TOOL_ACTION* aAction );
/**
* Function RunAction()
* Runs the specified action. The common format for action names is "application.ToolName.Action".

View File

@ -98,15 +98,9 @@ protected:
/**
* Method ResetItem
* Reset the item to the original from the dialog was created.
* Reset the item to either the default, the value when the dialog was opened, or none.
*/
void ResetItem( wxTreeListItem aItem );
/**
* Method ResetItemToDefault
* Reset the item to the default value.
*/
void ResetItemToDefault( wxTreeListItem aItem );
void ResetItem( wxTreeListItem aItem, int aResetId );
/**
* Method OnActivated

View File

@ -30,9 +30,6 @@
#include <eda_base_frame.h>
#include <kiway_player.h>
#define KICAD_MANAGER_FRAME_NAME wxT( "KicadFrame" )
class LAUNCHER_PANEL;
class TREEPROJECTFILES;
class TREE_PROJECT_FRAME;
class ACTION_TOOLBAR;