Add ability to skip JSON writes if the params aren't modified

common/settings/json_settings.cpp

@@ -36,13 +36,14 @@ extern const char* traceSettings;
 
 
 JSON_SETTINGS::JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation,
-                              int aSchemaVersion, bool aCreateIfMissing, bool aWriteFile,
+                              int aSchemaVersion, bool aCreateIfMissing, bool aCreateIfDefault,
-                              nlohmann::json aDefault ) :
+                              bool aWriteFile ) :
-        nlohmann::json( std::move( aDefault ) ),
+        nlohmann::json(),
         m_filename( aFilename ),
         m_legacy_filename( "" ),
         m_location( aLocation ),
         m_createIfMissing( aCreateIfMissing ),
+        m_createIfDefault( aCreateIfDefault ),
         m_writeFile( aWriteFile ),
         m_schemaVersion( aSchemaVersion ),
         m_manager( nullptr )
@@ -206,10 +207,17 @@ void JSON_SETTINGS::LoadFromFile( const std::string& aDirectory )
 }
 
 
-void JSON_SETTINGS::Store()
+bool JSON_SETTINGS::Store()
 {
+    bool modified = false;
+
     for( auto param : m_params )
+    {
+        modified |= !param->MatchesFile( this );
         param->Store( this );
+    }
+
+    return modified;
 }
 
 
@@ -220,29 +228,49 @@ void JSON_SETTINGS::ResetToDefaults()
 }
 
 
-void JSON_SETTINGS::SaveToFile( const std::string& aDirectory )
+bool JSON_SETTINGS::SaveToFile( const std::string& aDirectory, bool aForce )
 {
     if( !m_writeFile )
-        return;
+        return false;
-
-    wxLogTrace( traceSettings, "Saving %s", m_filename );
 
     wxFileName path( aDirectory, m_filename, "json" );
 
     if( !m_createIfMissing && !path.FileExists() )
-        return;
+    {
+        wxLogTrace( traceSettings,
+                "File for %s doesn't exist and m_createIfMissing == false; not saving",
+                m_filename );
+        return false;
+    }
+
+    bool modified = false;
+
+    for( auto settings : m_nested_settings )
+        modified |= settings->SaveToFile();
+
+    modified |= Store();
+
+    if( !modified && !aForce && path.FileExists() )
+    {
+        wxLogTrace( traceSettings, "%s contents not modified, skipping save", m_filename );
+        return false;
+    }
+    else if( !modified && !aForce && !m_createIfDefault )
+    {
+        wxLogTrace( traceSettings,
+                "%s contents still default and m_createIfDefault == false; not saving",
+                m_filename );
+        return false;
+    }
 
     if( !path.DirExists() && !path.Mkdir() )
     {
         wxLogTrace( traceSettings, "Warning: could not create path %s, can't save %s",
                     path.GetPath(), m_filename );
-        return;
+        return false;
     }
 
-    for( auto settings : m_nested_settings )
+    wxLogTrace( traceSettings, "Saving %s", m_filename );
-        settings->SaveToFile();
-
-    Store();
 
     LOCALE_IO dummy;
 
@@ -258,6 +286,8 @@ void JSON_SETTINGS::SaveToFile( const std::string& aDirectory )
     catch( ... )
     {
     }
+
+    return true;
 }
 
 

common/settings/nested_settings.cpp

@@ -26,8 +26,8 @@ extern const char* traceSettings;
 
 
 NESTED_SETTINGS::NESTED_SETTINGS( const std::string& aName, int aVersion, JSON_SETTINGS* aParent,
-                                  const std::string& aPath, nlohmann::json aDefault ) :
+                                  const std::string& aPath ) :
-        JSON_SETTINGS( aName, SETTINGS_LOC::NESTED, aVersion, std::move( aDefault ) ),
+        JSON_SETTINGS( aName, SETTINGS_LOC::NESTED, aVersion ),
         m_parent( aParent ), m_path( aPath )
 {
     if( m_parent )
@@ -70,9 +70,23 @@ void NESTED_SETTINGS::LoadFromFile( const std::string& aDirectory )
 }
 
 
-void NESTED_SETTINGS::SaveToFile( const std::string& aDirectory )
+bool NESTED_SETTINGS::SaveToFile( const std::string& aDirectory, bool aForce )
 {
-    Store();
+    bool modified = Store();
+
+    try
+    {
+        nlohmann::json patch =
+                nlohmann::json::diff( *this, ( *m_parent )[PointerFromString( m_path )] );
+        modified |= !patch.empty();
+    }
+    catch( ... )
+    {
+        modified = true;
+    }
+
+    if( !modified && !aForce )
+        return false;
 
     try
     {
@@ -86,4 +100,6 @@ void NESTED_SETTINGS::SaveToFile( const std::string& aDirectory )
         wxLogTrace( traceSettings, "NESTED_SETTINGS %s: Could not store to %s at %s",
                     m_filename, m_parent->GetFilename(), m_path );
     }
+
+    return modified;
 }

common/settings/settings_manager.cpp

@@ -290,7 +290,12 @@ void SETTINGS_MANAGER::SaveColorSettings( COLOR_SETTINGS* aSettings, const std::
 
     nlohmann::json::json_pointer ptr = JSON_SETTINGS::PointerFromString( aNamespace );
 
-    aSettings->Store();
+    if( !aSettings->Store() )
+    {
+        wxLogTrace( traceSettings, "Color scheme %s not modified; skipping save",
+                aSettings->GetFilename(), aNamespace );
+        return;
+    }
 
     wxASSERT( aSettings->contains( ptr ) );
 
@@ -305,7 +310,7 @@ void SETTINGS_MANAGER::SaveColorSettings( COLOR_SETTINGS* aSettings, const std::
     ( *aSettings )[ptr].update( backup );
     aSettings->Load();
 
-    aSettings->SaveToFile( path );
+    aSettings->SaveToFile( path, true );
 }
 
 

include/settings/color_settings.h

@@ -133,14 +133,30 @@ public:
         return m_default;
     }
 
-    virtual void SetDefault() override
+    void SetDefault() override
     {
         ( *m_map )[ m_key ] = m_default;
     }
 
+    bool IsDefault() const override
+    {
+        return ( *m_map )[ m_key ] == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( OPT<COLOR4D> optval = aSettings->Get<COLOR4D>( m_path ) )
+            return m_map->count( m_key ) && ( *optval == m_map->at( m_key ) );
+
+        // If the JSON doesn't exist, the map shouldn't exist either
+        return !m_map->count( m_key );
+    }
+
 private:
     int m_key;
+
     COLOR4D m_default;
+
     std::unordered_map<int, COLOR4D>* m_map;
 };
 

include/settings/json_settings.h

@@ -44,13 +44,11 @@ enum class SETTINGS_LOC {
 class JSON_SETTINGS : public nlohmann::json
 {
 public:
-    JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion,
+    JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion ) :
-                   nlohmann::json aDefaultJson = nlohmann::json( {} ) ) :
+            JSON_SETTINGS( aFilename, aLocation, aSchemaVersion, true, true, true ) {}
-            JSON_SETTINGS( aFilename, aLocation, aSchemaVersion, true, true, aDefaultJson ) {}
 
     JSON_SETTINGS( const std::string& aFilename, SETTINGS_LOC aLocation, int aSchemaVersion,
-                   bool aCreateIfMissing, bool aWriteFile,
+                   bool aCreateIfMissing, bool aCreateIfDefault, bool aWriteFile );
-                   nlohmann::json aDefaultJson = nlohmann::json( {} ) );
 
     virtual ~JSON_SETTINGS();
 
@@ -68,8 +66,9 @@ public:
     /**
      * Stores the current parameters into the JSON document represented by this object
      * Note: this doesn't do any writing to disk; that's handled by SETTINGS_MANAGER
+     * @return true if any part of the JSON document was updated
      */
-    virtual void Store();
+    virtual bool Store();
 
     /**
      * Loads the backing file from disk and then calls Load()
@@ -80,8 +79,10 @@ public:
     /**
      * Calls Store() and then writes the contents of the JSON document to a file
      * @param aDirectory is the directory to save to, including trailing separator
+     * @param aForce if true will always save, even if contents are not modified
+     * @return true if the file was saved
      */
-    virtual void SaveToFile( const std::string& aDirectory );
+    virtual bool SaveToFile( const std::string& aDirectory, bool aForce = false );
 
     /**
      * Resets all parameters to default values.  Does NOT write to file or update underlying JSON.
@@ -228,6 +229,12 @@ protected:
     /// Whether or not the backing store file should be created it if doesn't exist
     bool m_createIfMissing;
 
+    /**
+     * Whether or not the  backing store file should be created if all parameters are still
+     * at their default values.  Ignored if m_createIfMissing is false or m_writeFile is false.
+     */
+    bool m_createIfDefault;
+
     /// Whether or not the backing store file should be written
     bool m_writeFile;
 

include/settings/nested_settings.h

@@ -32,7 +32,7 @@ class NESTED_SETTINGS : public JSON_SETTINGS
 {
 public:
     NESTED_SETTINGS( const std::string& aName, int aSchemaVersion, JSON_SETTINGS* aParent,
-                     const std::string& aPath, nlohmann::json aDefault = nlohmann::json( {} ) );
+                     const std::string& aPath );
 
     virtual ~NESTED_SETTINGS();
 
@@ -40,13 +40,14 @@ public:
      * Loads the JSON document from the parent and then calls Load()
      * @param aDirectory
      */
-    virtual void LoadFromFile( const std::string& aDirectory = "" ) override;
+    void LoadFromFile( const std::string& aDirectory = "" ) override;
 
     /**
      * Calls Store() and then saves the JSON document contents into the parent JSON_SETTINGS
      * @param aDirectory is ignored
+     * @return true if the document contents were updated
      */
-    virtual void SaveToFile( const std::string& aDirectory = "" ) override;
+    bool SaveToFile( const std::string& aDirectory = "", bool aForce = false ) override;
 
 private:
 

include/settings/parameters.h

@@ -53,6 +53,19 @@ public:
 
     virtual void SetDefault() = 0;
 
+    /**
+     * Checks whether or not this param has been changed from its default value
+     * @return true if the parameter in memory matches its default value
+     */
+    virtual bool IsDefault() const = 0;
+
+    /**
+     * Checks whether the parameter in memory matches the one in a given JSON file
+     * @param aSettings is a JSON_SETTINGS to check the JSON file contents of
+     * @return true if the parameter in memory matches its value in the file
+     */
+    virtual bool MatchesFile( JSON_SETTINGS* aSettings ) const = 0;
+
     /**
      * @return the path name of the parameter used to store it in the json file
      * mainly usefull in error messages
@@ -115,7 +128,7 @@ public:
         *m_ptr = val;
     }
 
-    void Store( JSON_SETTINGS* aSettings) const override
+    void Store( JSON_SETTINGS* aSettings ) const override
     {
         aSettings->Set<ValueType>( m_path, *m_ptr );
     }
@@ -125,11 +138,24 @@ public:
         return m_default;
     }
 
-    virtual void SetDefault() override
+    void SetDefault() override
     {
         *m_ptr = m_default;
     }
 
+    bool IsDefault() const override
+    {
+        return *m_ptr == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( OPT<ValueType> optval = aSettings->Get<ValueType>( m_path ) )
+            return *optval == *m_ptr;
+
+        return false;
+    }
+
 private:
     ValueType* m_ptr;
     ValueType m_default;
@@ -193,11 +219,33 @@ public:
         return m_default;
     }
 
-    virtual void SetDefault() override
+    void SetDefault() override
     {
         m_setter( m_default );
     }
 
+    bool IsDefault() const override
+    {
+        return m_getter() == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( std::is_same<ValueType, nlohmann::json>::value )
+        {
+            if( OPT<nlohmann::json> optval = aSettings->GetJson( m_path ) )
+                return *optval == m_default;
+        }
+        else
+        {
+            if( OPT<ValueType> optval = aSettings->Get<ValueType>( m_path ) )
+                return *optval == m_default;
+        }
+
+        // Not in file
+        return false;
+    }
+
 private:
     ValueType m_default;
 
@@ -275,6 +323,19 @@ public:
         *m_ptr = m_default;
     }
 
+    bool IsDefault() const override
+    {
+        return *m_ptr == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( OPT<double> optval = aSettings->Get<double>( m_path ) )
+            return *optval == ( *m_ptr * m_scale );
+
+        return false;
+    }
+
 private:
     ValueType* m_ptr;
     ValueType m_default;
@@ -333,11 +394,34 @@ public:
         aSettings->Set<nlohmann::json>( m_path, js );
     }
 
-    virtual void SetDefault() override
+    void SetDefault() override
     {
         *m_ptr = m_default;
     }
 
+    bool IsDefault() const override
+    {
+        return *m_ptr == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( OPT<nlohmann::json> js = aSettings->GetJson( m_path ) )
+        {
+            if( js->is_array() )
+            {
+                std::vector<Type> val;
+
+                for( const auto& el : js->items() )
+                    val.emplace_back( el.value().get<Type>() );
+
+                return val == *m_ptr;
+            }
+        }
+
+        return false;
+    }
+
 private:
     std::vector<Type>* m_ptr;
 
@@ -405,6 +489,29 @@ public:
         *m_ptr = m_default;
     }
 
+    bool IsDefault() const override
+    {
+        return *m_ptr == m_default;
+    }
+
+    bool MatchesFile( JSON_SETTINGS* aSettings ) const override
+    {
+        if( OPT<nlohmann::json> js = aSettings->GetJson( m_path ) )
+        {
+            if( js->is_object() )
+            {
+                std::map<std::string, Value> val;
+
+                for( const auto& el : js->items() )
+                    val[ el.key() ] = el.value().get<Value>();
+
+                return val == *m_ptr;
+            }
+        }
+
+        return false;
+    }
+
 private:
     std::map<std::string, Value>* m_ptr;