Clear out state.json when we find and empty settings.json#11448
Merged
2 commits merged intomainfrom Oct 11, 2021
Merged
Conversation
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`
DHowett
reviewed
Oct 7, 2021
|
|
||
| #define MTSM_APPLICATION_STATE_GEN(type, name, key, ...) \ | ||
| { \ | ||
| auto state = _state.lock(); \ |
Member
There was a problem hiding this comment.
we may like to refactor this to do the whole thing under one lock over _state; @lhecker?
Member
There was a problem hiding this comment.
This will also, i think, try to enqueue the throttler once per setting. It won't fire, but it's not necessary.
Member
There was a problem hiding this comment.
To be honest I don't quite get why we have to mark all fields as changed in the first place...
When we re-read the file it will not exist, so it shouldn't be possible for the old data to come back.
In that case I'd just write _state.lock() = {};.
Member
Author
There was a problem hiding this comment.
_state.lock() = {};
now that's some witchcraft if I've ever seen it
I'll try that. I was just marking them as changed because I was trying to avoid the DeleteFile() but (maybe accidentally) left that in
lhecker
approved these changes
Oct 7, 2021
DHowett
approved these changes
Oct 8, 2021
|
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
PankajBhojwani
pushed a commit
that referenced
this pull request
Oct 13, 2021
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`
PankajBhojwani
pushed a commit
that referenced
this pull request
Oct 15, 2021
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` (cherry picked from commit 8dd3173)
This pull request was closed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
ApplicationStateobject and reset it tothe 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.
state.jsonand an empty
settings.jsonsettings.json