Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Profiles missing after resetting config #11119

Closed
ktremain opened this issue Sep 2, 2021 · 4 comments · Fixed by #11448
Closed

Profiles missing after resetting config #11119

ktremain opened this issue Sep 2, 2021 · 4 comments · Fixed by #11448
Assignees
@zadjii-msft
Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Milestone
Terminal v1.12

Comments

@ktremain
Copy link

ktremain commented Sep 2, 2021

Windows Terminal version (or Windows build number)

1.10.2383.0

Other Software

No response

Steps to reproduce

  1. have terminal ver 1.9 with default profiles including powershell/WSL/Azure shell
  2. upgrade to v1.10
  3. open settings.json
  4. delete contents of file and save
  5. reload terminal

as per notes at top of config file, i wanted to ensure that i had any new defaults now that v1.10 has been released
"..It should still be usable in newer versions, but newer versions might have additional
settings, help text, or changes that you will not see unless you clear this file
and let us generate a new one for you."

Expected Behavior

expected that all default profiles get regenerated

Actual Behavior

only "windows powershell" and "Command prompt" appear.

Also note that terminal did infact launch "powershell" (v7.1.4) and had an empty dropdown in the "default profile" box in settings as what was launched did not have any profile.

I had to reset the app to restore all my profiles
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Sep 2, 2021
@zadjii-msft zadjii-msft mentioned this issue Sep 2, 2021
Merged
@zadjii-msft
Copy link
Member

HMMMMMM This is a good point. In 1.11 (and backported to 1.10), we added the ability to just delete dynamic profiles from the settings, and have them not auto-regenerate. But if you totally blow away the settings file, that's kinda a different case now.

@DHowett @lhecker for discussion: if settings.json is totally empty, or missing, should we clear out state.json too? Or is that just a bad heuristic? Is there a reason we would want to keep state.json even if they blow away their settings?

@zadjii-msft zadjii-msft added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Discussion Something that requires a team discussion before we can proceed Product-Terminal The new Windows Terminal. labels Sep 2, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 2, 2021
@zadjii-msft zadjii-msft added this to the Terminal v1.12 milestone Sep 2, 2021
@lhecker
Copy link
Member

lhecker commented Sep 2, 2021

@DHowett @lhecker for discussion: if settings.json is totally empty, or missing, should we clear out state.json too?

Sounds good to me actually.

@zadjii-msft zadjii-msft removed the Needs-Discussion Something that requires a team discussion before we can proceed label Sep 8, 2021
@carlos-zamora
Copy link
Member

@zadjii-msft Is there anything left to do here after merging that docs change?

@zadjii-msft
Copy link
Member

Oh we were going to change the Terminal so that when we encounter a missing or empty settings.json, we'll also make sure to manually clear out state.json. That way any state from the previous settings instance isn't used accidentally.

@zadjii-msft zadjii-msft self-assigned this Oct 7, 2021
zadjii-msft added a commit that referenced this issue Oct 7, 2021
@zadjii-msft
Clear out state.json when we find and empty settings.json
4be258c 
  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`
@zadjii-msft zadjii-msft mentioned this issue Oct 7, 2021
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Oct 7, 2021
@ghost ghost closed this as completed in #11448 Oct 11, 2021
@ghost ghost removed the In-PR This issue has a related PR label Oct 11, 2021
ghost pushed a commit that referenced this issue Oct 11, 2021
@zadjii-msft
Clear out state.json when we find and empty settings.json (#11448)
8dd3173 
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`
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Oct 11, 2021
PankajBhojwani pushed a commit that referenced this issue Oct 13, 2021
@zadjii-msft @PankajBhojwani
Clear out state.json when we find and empty settings.json (#11448)
1ddceba 
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 issue Oct 15, 2021
@zadjii-msft @PankajBhojwani
Clear out state.json when we find and empty settings.json (#11448)
fc2751e 
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 issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zadjii-msft zadjii-msft

Labels
Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
Terminal v1.12
Development

Successfully merging a pull request may close this issue.

Clear out state.json when we find and empty settings.json microsoft/terminal
4 participants
@lhecker @carlos-zamora @zadjii-msft @ktremain