Skip to content

Conversation

@lhecker
Copy link
Member

@lhecker lhecker commented Aug 5, 2021

⚠️ crazy 4am idea ⚠️
👉 Open for discussion

Stop recreating settings.json if it's removed.
This improves UX, because users don't accidentally loose their settings anymore
and removes a source of crashes in the SUI. Our WinUI NavigationView crashes
if its MenuItems array is modified while a transition is ongoing.
The crash in particular occurs reliably if the settings are deleted.

PR Checklist

  • I work here
  • Tests added/passed

Validation Steps Performed

  • Removing settings.json doesn't cause it to be created ✔️

@lhecker lhecker force-pushed the dev/lhecker/ignore-settings-remove branch from ebf7512 to 0d946fe Compare August 5, 2021 00:34
@github-actions

This comment has been minimized.

@lhecker lhecker added Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Aug 5, 2021
@WSLUser
Copy link
Contributor

WSLUser commented Aug 5, 2021

I don't think this is the right answer. What if it was removed purposely for the intention of getting a clean new settings.json? A PR was specifically added and merged to ensure that even while WT is running, a new settings.json could be generated. This PR doesn't support the end user. There must be other ways to address crashes than removing an option that users like myself rely on.

@lhecker
Copy link
Member Author

lhecker commented Aug 5, 2021

What if it was removed purposely for the intention of getting a clean new settings.json?

I imagined that you could just restart the app.

@zadjii-msft zadjii-msft added Needs-Discussion Something that requires a team discussion before we can proceed and removed Needs-Discussion Something that requires a team discussion before we can proceed labels Aug 16, 2021
@miniksa
Copy link
Member

miniksa commented Jan 14, 2022

I think after a few months of idling here, we should just close this PR and revisit in the future if necessary.

@DHowett
Copy link
Member

DHowett commented Jan 14, 2022

We can reopen/revaluate later (:

@DHowett DHowett closed this Jan 14, 2022
@lhecker lhecker deleted the dev/lhecker/ignore-settings-remove branch June 10, 2022 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants