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`
This commit is contained in:
Mike Griese 2021-10-11 10:51:47 -05:00 committed by GitHub
parent 479ef264b2
commit 8dd317313b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 0 deletions

View File

@ -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:
// - <none>
// Return Value:
// - <none>
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;

View File

@ -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;

View File

@ -28,6 +28,7 @@ namespace Microsoft.Terminal.Settings.Model
static ApplicationState SharedInstance();
void Reload();
void Reset();
String FilePath { get; };

View File

@ -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;