Don't overwrite the settings file (#2475)

This is more trouble than it's worth. We had code before to re-serialize
  settings when they changed, to try and gracefully migrate settings from old
  schemas to new ones. This is good in theory, but with #754 coming soon, this
  is going to become a minefield. In the future we'll just always be providing a
  base schema that's reasonable, so this won't matter so much. Keys that users
  have that aren't understood will just be ignored, and that's _fine_.

src/cascadia/TerminalApp/App.cpp

@@ -680,19 +680,15 @@ namespace winrt::TerminalApp::implementation
 
     // Method Description:
     // - Attempt to load the settings. If we fail for any reason, returns an error.
-    // Arguments:
-    // - saveOnLoad: If true, after loading the settings, we should re-write
-    //   them to the file, to make sure the schema is updated. See
-    //   `CascadiaSettings::LoadAll` for details.
     // Return Value:
     // - S_OK if we successfully parsed the settings, otherwise an appropriate HRESULT.
-    [[nodiscard]] HRESULT App::_TryLoadSettings(const bool saveOnLoad) noexcept
+    [[nodiscard]] HRESULT App::_TryLoadSettings() noexcept
     {
         HRESULT hr = E_FAIL;
 
         try
         {
-            auto newSettings = CascadiaSettings::LoadAll(saveOnLoad);
+            auto newSettings = CascadiaSettings::LoadAll();
             _settings = std::move(newSettings);
             const auto& warnings = _settings->GetWarnings();
             hr = warnings.size() == 0 ? S_OK : S_FALSE;
@@ -733,7 +729,7 @@ namespace winrt::TerminalApp::implementation
         //    we should display the loading error.
         //    * We can't display the error now, because we might not have a
         //      UI yet. We'll display the error in _OnLoaded.
-        _settingsLoadedResult = _TryLoadSettings(true);
+        _settingsLoadedResult = _TryLoadSettings();
 
         if (FAILED(_settingsLoadedResult))
         {
@@ -813,7 +809,7 @@ namespace winrt::TerminalApp::implementation
         //  - don't change the settings (and don't actually apply the new settings)
         //  - don't persist them.
         //  - display a loading error
-        _settingsLoadedResult = _TryLoadSettings(false);
+        _settingsLoadedResult = _TryLoadSettings();
 
         if (FAILED(_settingsLoadedResult))
         {

src/cascadia/TerminalApp/App.h

@@ -85,7 +85,7 @@ namespace winrt::TerminalApp::implementation
         void _ShowLoadWarningsDialog();
         void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey);
 
-        [[nodiscard]] HRESULT _TryLoadSettings(const bool saveOnLoad) noexcept;
+        [[nodiscard]] HRESULT _TryLoadSettings() noexcept;
         void _LoadSettings();
         void _OpenSettings();
 

src/cascadia/TerminalApp/CascadiaSettings.h

@@ -40,7 +40,7 @@ class TerminalApp::CascadiaSettings final
     CascadiaSettings();
     ~CascadiaSettings();
 
-    static std::unique_ptr<CascadiaSettings> LoadAll(const bool saveOnLoad = true);
+    static std::unique_ptr<CascadiaSettings> LoadAll();
     void SaveAll() const;
 
     winrt::Microsoft::Terminal::Settings::TerminalSettings MakeSettings(std::optional<GUID> profileGuid) const;

src/cascadia/TerminalApp/CascadiaSettingsSerialization.cpp

@@ -30,12 +30,9 @@ static constexpr std::string_view Utf8Bom{ u8"\uFEFF" };
 //      it will load the settings from our packaged localappdata. If we're
 //      running as an unpackaged application, it will read it from the path
 //      we've set under localappdata.
-// Arguments:
-// - saveOnLoad: If true, we'll write the settings back out after we load them,
-//   to make sure the schema is updated.
 // Return Value:
 // - a unique_ptr containing a new CascadiaSettings object.
-std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoad)
+std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll()
 {
     std::unique_ptr<CascadiaSettings> resultPtr;
     std::optional<std::string> fileData = _ReadSettings();
@@ -70,22 +67,6 @@ std::unique_ptr<CascadiaSettings> CascadiaSettings::LoadAll(const bool saveOnLoa
 
         // If this throws, the app will catch it and use the default settings (temporarily)
         resultPtr->_ValidateSettings();
-
-        const bool foundWarnings = resultPtr->_warnings.size() > 0;
-
-        // Don't save on load if there were warnings - we tried to gracefully
-        // handle them.
-        if (saveOnLoad && !foundWarnings)
-        {
-            // Logically compare the json we've parsed from the file to what
-            // we'd serialize at runtime. If the values are different, then
-            // write the updated schema back out.
-            const Json::Value reserialized = resultPtr->ToJson();
-            if (reserialized != root)
-            {
-                resultPtr->SaveAll();
-            }
-        }
     }
     else
     {