A few tweaks to settings migration infrastructure

common/settings/json_settings.cpp

@@ -40,7 +40,7 @@ JSON_SETTINGS::JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocati
                               nlohmann::json aDefault ) :
         nlohmann::json( std::move( aDefault ) ), m_filename( aFilename ), m_legacy_filename( "" ),
         m_location( aLocation ), m_createIfMissing( aCreateIfMissing ), m_writeFile( aWriteFile ),
-        m_schemaVersion( aSchemaVersion )
+        m_schemaVersion( aSchemaVersion ), m_manager( nullptr )
 {
     m_params.emplace_back(
             new PARAM<std::string>( "meta.filename", &m_filename, m_filename, true ) );
@@ -73,6 +73,7 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
     Load();
 
     bool      migrated        = false;
+    bool      legacy_migrated = false;
     LOCALE_IO locale;
 
     auto migrateFromLegacy = [&] ( wxFileName& aPath ) {
@@ -92,7 +93,7 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
         }
 
         // Either way, we want to clean up the old file afterwards
-        migrated = true;
+        legacy_migrated = true;
     };
 
     wxFileName path( aDirectory, m_filename, "json" );
@@ -123,16 +124,27 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
             in >> *this;
 
             // If parse succeeds, check if schema migration is required
+            int filever = -1;
+
             try
             {
-                int filever = at( PointerFromString( "meta.version" ) ).get<int>();
+                filever = at( PointerFromString( "meta.version" ) ).get<int>();
+            }
+            catch( ... )
+            {
+                wxLogTrace( traceSettings, "%s: file version could not be read!", m_filename );
+            }
 
-                if( filever < m_schemaVersion )
+            if( filever >= 0 && filever < m_schemaVersion )
             {
                 wxLogTrace( traceSettings, "%s: attempting migration from version %d to %d",
                         m_filename, filever, m_schemaVersion );
 
-                    if( !Migrate() )
+                if( Migrate() )
+                {
+                    migrated = true;
+                }
+                else
                 {
                     wxLogTrace( traceSettings, "%s: migration failed!", m_filename );
                 }
@@ -144,11 +156,6 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
                         filever, m_schemaVersion );
             }
         }
-            catch( ... )
-            {
-                wxLogTrace( traceSettings, "%s: file version could not be read!", m_filename );
-            }
-        }
         catch( nlohmann::json::parse_error& error )
         {
             wxLogTrace(
@@ -168,9 +175,9 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
     wxLogTrace( traceSettings, "Loaded %s with schema %d", GetFilename(), m_schemaVersion );
 
     // If we migrated, clean up the legacy file (with no extension)
-    if( migrated )
+    if( legacy_migrated || migrated )
     {
-        if( !wxRemoveFile( path.GetFullPath() ) )
+        if( legacy_migrated && !wxRemoveFile( path.GetFullPath() ) )
         {
             wxLogTrace(
                     traceSettings, "Warning: could not remove legacy file %s", path.GetFullPath() );

common/settings/settings_manager.cpp

@@ -73,6 +73,8 @@ JSON_SETTINGS* SETTINGS_MANAGER::RegisterSettings( JSON_SETTINGS* aSettings, boo
 {
     std::unique_ptr<JSON_SETTINGS> ptr( aSettings );
 
+    ptr->SetManager( this );
+
     wxLogTrace( traceSettings, "Registered new settings object %s", ptr->GetFilename() );
 
     if( aLoadNow )
@@ -231,6 +233,9 @@ public:
 
 void SETTINGS_MANAGER::registerColorSettings( const wxString& aFilename )
 {
+    if( m_color_settings.count( aFilename ) )
+        return;
+
     m_color_settings[aFilename] = static_cast<COLOR_SETTINGS*>(
             RegisterSettings( new COLOR_SETTINGS( aFilename.ToStdString() ) ) );
 }

include/settings/json_settings.h

@@ -30,6 +30,8 @@
 class wxConfigBase;
 class NESTED_SETTINGS;
 class PARAM_BASE;
+class SETTINGS_MANAGER;
+
 
 enum class SETTINGS_LOC {
     USER,       ///< The main config directory (e.g. ~/.config/kicad/)
@@ -166,6 +168,11 @@ public:
      */
     void ReleaseNestedSettings( NESTED_SETTINGS* aSettings );
 
+    void SetManager( SETTINGS_MANAGER* aManager )
+    {
+        m_manager = aManager;
+    }
+
     /**
     * Builds a JSON pointer based on a given string
     * @param aPath is the path in the form "key1.key2.key3"
@@ -227,6 +234,9 @@ protected:
     /// Version of this settings schema.
     int m_schemaVersion;
 
+    /// A pointer to the settings manager managing this file (may be null)
+    SETTINGS_MANAGER* m_manager;
+
     /// A list of JSON pointers that are preserved during a read-update-write to disk
     std::vector<nlohmann::json::json_pointer> m_preserved_paths;
 };