From 8dd317313b917f016c90c68f10b78777c0579749 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 11 Oct 2021 10:51:47 -0500 Subject: [PATCH] Clear out `state.json` when we find and empty `settings.json` (#11448) If we find that the settings file doesn't exist, or is empty, then let's quick delete the state file as well. If the user does have a state file, and not a settings, then they probably tried to reset their settings. It might have data in it that was only relevant for a previous iteration of the settings file. If we don't, we'll load the old state and ignore all dynamic profiles (for example)! We'll remove all of the data in the `ApplicationState` object and reset it to the defaults. This will delete the state file! That's the sure-fire way to make sure the data doesn't come back. If we leave it untouched, then when we go to write the file back out, we'll first re-read it's contents and try to overlay our new state. However, nullopts won't remove keys from the JSON, so we'll end up with the original state in the file. * [x] Closes #11119 * [x] Tested on a cold launch of the Terminal with an existing `state.json` and an empty `settings.json` * [x] Tested a hot-reload of deleting the `settings.json` --- .../ApplicationState.cpp | 20 +++++++++++++++++++ .../TerminalSettingsModel/ApplicationState.h | 1 + .../ApplicationState.idl | 1 + .../CascadiaSettingsSerialization.cpp | 12 +++++++++++ 4 files changed, 34 insertions(+) diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp index acd876884..7e5bc1da3 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.cpp +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.cpp @@ -141,6 +141,26 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation MTSM_APPLICATION_STATE_FIELDS(MTSM_APPLICATION_STATE_GEN) #undef MTSM_APPLICATION_STATE_GEN + // Method Description: + // - See GH#11119. Removes all of the data in this ApplicationState object + // and resets it to the defaults. This will delete the state file! That's + // the sure-fire way to make sure the data doesn't come back. If we leave + // it untouched, then when we go to write the file back out, we'll first + // re-read it's contents and try to overlay our new state. However, + // nullopts won't remove keys from the JSON, so we'll end up with the + // original state in the file. + // Arguments: + // - + // Return Value: + // - + void ApplicationState::Reset() noexcept + try + { + LOG_LAST_ERROR_IF(!DeleteFile(_path.c_str())); + *_state.lock() = {}; + } + CATCH_LOG() + Json::Value ApplicationState::_getRoot(const locked_hfile& file) const noexcept { Json::Value root; diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.h b/src/cascadia/TerminalSettingsModel/ApplicationState.h index 9b4f40f28..d6d2d1749 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.h +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.h @@ -53,6 +53,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation // Methods void Reload() const noexcept; + void Reset() noexcept; // General getters/setters winrt::hstring FilePath() const noexcept; diff --git a/src/cascadia/TerminalSettingsModel/ApplicationState.idl b/src/cascadia/TerminalSettingsModel/ApplicationState.idl index 390782683..7890b58c6 100644 --- a/src/cascadia/TerminalSettingsModel/ApplicationState.idl +++ b/src/cascadia/TerminalSettingsModel/ApplicationState.idl @@ -28,6 +28,7 @@ namespace Microsoft.Terminal.Settings.Model static ApplicationState SharedInstance(); void Reload(); + void Reset(); String FilePath { get; }; diff --git a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp index c0e925c56..83f0952c2 100644 --- a/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp +++ b/src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp @@ -650,6 +650,18 @@ try { const auto settingsString = ReadUTF8FileIfExists(_settingsPath()).value_or(std::string{}); const auto firstTimeSetup = settingsString.empty(); + + // GH#11119: If we find that the settings file doesn't exist, or is empty, + // then let's quick delete the state file as well. If the user does have a + // state file, and not a settings, then they probably tried to reset their + // settings. It might have data in it that was only relevant for a previous + // iteration of the settings file. If we don't, we'll load the old state and + // ignore all dynamic profiles (for example)! + if (firstTimeSetup) + { + ApplicationState::SharedInstance().Reset(); + } + const auto settingsStringView = firstTimeSetup ? UserSettingsJson : settingsString; auto mustWriteToDisk = firstTimeSetup;