Skip to content

Commit

Permalink
Clear out state.json when we find and empty settings.json (#11448)
Browse files Browse the repository at this point in the history
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`
  • Loading branch information
zadjii-msft authored and PankajBhojwani committed Oct 13, 2021
1 parent 8a68956 commit 1ddceba
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 0 deletions.
20 changes: 20 additions & 0 deletions src/cascadia/TerminalSettingsModel/ApplicationState.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/ApplicationState.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalSettingsModel/ApplicationState.idl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ namespace Microsoft.Terminal.Settings.Model
static ApplicationState SharedInstance();

void Reload();
void Reset();

String FilePath { get; };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down

0 comments on commit 1ddceba

Please sign in to comment.