From 9cc5b4b3f915e13b9beb113776e65d4ed9b177d4 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Thu, 27 Feb 2020 22:53:00 -0500 Subject: [PATCH] Fix management of nested settings for PNS --- common/eda_draw_frame.cpp | 2 -- common/settings/json_settings.cpp | 17 +++++++++++++++++ common/settings/nested_settings.cpp | 6 ++++++ include/settings/json_settings.h | 6 ++++++ include/settings/nested_settings.h | 2 +- pcbnew/dialogs/dialog_pns_settings.cpp | 1 - pcbnew/pcbnew_settings.cpp | 3 +++ pcbnew/pcbnew_settings.h | 4 ++-- pcbnew/router/pns_tool_base.cpp | 10 ++++++---- 9 files changed, 41 insertions(+), 10 deletions(-) diff --git a/common/eda_draw_frame.cpp b/common/eda_draw_frame.cpp index bc3224f334..b0c6250502 100644 --- a/common/eda_draw_frame.cpp +++ b/common/eda_draw_frame.cpp @@ -642,8 +642,6 @@ bool EDA_DRAW_FRAME::saveCanvasTypeSetting( EDA_DRAW_PANEL_GAL::GAL_TYPE aCanvas if( cfg ) cfg->m_Graphics.canvas_type = static_cast( aCanvasType ); - Pgm().GetSettingsManager().Save( cfg ); - return false; } diff --git a/common/settings/json_settings.cpp b/common/settings/json_settings.cpp index 95335fa71c..691a877b37 100644 --- a/common/settings/json_settings.cpp +++ b/common/settings/json_settings.cpp @@ -381,10 +381,27 @@ bool JSON_SETTINGS::fromLegacyColor( wxConfigBase* aConfig, const std::string& a void JSON_SETTINGS::AddNestedSettings( NESTED_SETTINGS* aSettings ) { + wxLogTrace( traceSettings, "AddNestedSettings %s", aSettings->GetFilename() ); m_nested_settings.push_back( aSettings ); } +void JSON_SETTINGS::ReleaseNestedSettings( NESTED_SETTINGS* aSettings ) +{ + auto it = std::find_if( m_nested_settings.begin(), m_nested_settings.end(), + [&aSettings]( const JSON_SETTINGS* aPtr ) { + return aPtr == aSettings; + } ); + + if( it != m_nested_settings.end() ) + { + wxLogTrace( traceSettings, "Flush and release %s", ( *it )->GetFilename() ); + ( *it )->SaveToFile(); + m_nested_settings.erase( it ); + } +} + + // Specializations to allow conversion between wxString and std::string via JSON_SETTINGS API template<> wxString JSON_SETTINGS::Get( std::string aPath ) const diff --git a/common/settings/nested_settings.cpp b/common/settings/nested_settings.cpp index d181c9ea8d..d2a382b36f 100644 --- a/common/settings/nested_settings.cpp +++ b/common/settings/nested_settings.cpp @@ -38,6 +38,12 @@ NESTED_SETTINGS::NESTED_SETTINGS( const std::string& aName, int aVersion, JSON_S } +NESTED_SETTINGS::~NESTED_SETTINGS() +{ + m_parent->ReleaseNestedSettings( this ); +} + + void NESTED_SETTINGS::LoadFromFile( const std::string& aDirectory ) { clear(); diff --git a/include/settings/json_settings.h b/include/settings/json_settings.h index c676ee1b25..bf0f78c54c 100644 --- a/include/settings/json_settings.h +++ b/include/settings/json_settings.h @@ -147,6 +147,12 @@ public: */ void AddNestedSettings( NESTED_SETTINGS* aSettings ); + /** + * Saves and frees a nested settings object, if it exists within this one + * @param aSettings is a pointer to a NESTED_SETTINGS that has already been added to this one + */ + void ReleaseNestedSettings( NESTED_SETTINGS* aSettings ); + /** * Builds a JSON pointer based on a given string * @param aPath is the path in the form "key1.key2.key3" diff --git a/include/settings/nested_settings.h b/include/settings/nested_settings.h index 56aa8e1e39..717d586929 100644 --- a/include/settings/nested_settings.h +++ b/include/settings/nested_settings.h @@ -34,7 +34,7 @@ public: NESTED_SETTINGS( const std::string& aName, int aSchemaVersion, JSON_SETTINGS* aParent, const std::string& aPath, nlohmann::json aDefault = nlohmann::json( {} ) ); - virtual ~NESTED_SETTINGS() {} + virtual ~NESTED_SETTINGS(); /** * Loads the JSON document from the parent and then calls Load() diff --git a/pcbnew/dialogs/dialog_pns_settings.cpp b/pcbnew/dialogs/dialog_pns_settings.cpp index 15ceeed38e..3fbb23d433 100644 --- a/pcbnew/dialogs/dialog_pns_settings.cpp +++ b/pcbnew/dialogs/dialog_pns_settings.cpp @@ -77,7 +77,6 @@ void DIALOG_PNS_SETTINGS::OnOkClick( wxCommandEvent& aEvent ) m_settings.SetFreeAngleMode( m_freeAngleMode->GetValue() ); m_settings.SetInlineDragEnabled( m_dragToolMode->GetSelection () ? true : false ); m_settings.SetOptimizeDraggedTrack( m_optimizeDraggedTrack->GetValue() ); - m_settings.SaveToFile(); aEvent.Skip(); // ends returning wxID_OK (default behavior) } diff --git a/pcbnew/pcbnew_settings.cpp b/pcbnew/pcbnew_settings.cpp index fdbbcdf451..8d77565478 100644 --- a/pcbnew/pcbnew_settings.cpp +++ b/pcbnew/pcbnew_settings.cpp @@ -387,6 +387,9 @@ PCBNEW_SETTINGS::PCBNEW_SETTINGS() : APP_SETTINGS_BASE( "pcbnew", pcbnewSchemaVe } +PCBNEW_SETTINGS::~PCBNEW_SETTINGS() = default; + + bool PCBNEW_SETTINGS::MigrateFromLegacy( wxConfigBase* aCfg ) { bool ret = APP_SETTINGS_BASE::MigrateFromLegacy( aCfg ); diff --git a/pcbnew/pcbnew_settings.h b/pcbnew/pcbnew_settings.h index 12d0a9698a..0bab45c4fc 100644 --- a/pcbnew/pcbnew_settings.h +++ b/pcbnew/pcbnew_settings.h @@ -191,7 +191,7 @@ public: PCBNEW_SETTINGS(); - virtual ~PCBNEW_SETTINGS() {} + virtual ~PCBNEW_SETTINGS(); virtual bool MigrateFromLegacy( wxConfigBase* aLegacyConfig ) override; @@ -258,7 +258,7 @@ public: bool m_MagneticGraphics; - PNS::ROUTING_SETTINGS* m_PnsSettings; + std::unique_ptr m_PnsSettings; #if defined(KICAD_SCRIPTING) && defined(KICAD_SCRIPTING_ACTION_MENU) ACTION_PLUGIN_SETTINGS_LIST m_VisibleActionPlugins; diff --git a/pcbnew/router/pns_tool_base.cpp b/pcbnew/router/pns_tool_base.cpp index 0d1a1b5b20..e1846a1a26 100644 --- a/pcbnew/router/pns_tool_base.cpp +++ b/pcbnew/router/pns_tool_base.cpp @@ -89,7 +89,6 @@ TOOL_BASE::~TOOL_BASE() } - void TOOL_BASE::Reset( RESET_REASON aReason ) { delete m_gridHelper; @@ -109,9 +108,12 @@ void TOOL_BASE::Reset( RESET_REASON aReason ) m_router->UpdateSizes( m_savedSizes ); - auto settings = new PNS::ROUTING_SETTINGS( frame()->GetSettings(), "tools.pns" ); - frame()->GetSettings()->m_PnsSettings = settings; - m_router->LoadSettings( frame()->GetSettings()->m_PnsSettings ); + PCBNEW_SETTINGS* settings = frame()->GetSettings(); + + if( !settings->m_PnsSettings ) + settings->m_PnsSettings = std::make_unique( settings, "tools.pns" ); + + m_router->LoadSettings( settings->m_PnsSettings.get() ); m_gridHelper = new GRID_HELPER( frame() ); }