zokugun / vscode-sync-settings

Easily synchronize your settings
MIT License
146 stars 12 forks source link

Extension can delete all user settings.json #58

Open TonyGravagno opened 8 months ago

TonyGravagno commented 8 months ago

On experimenting with this extension I think I created a condition that deletes the user settings.json file. I recovered it, others may not be so fortunate.

I think this happens when we set:

"syncSettings.resources": ["extensions"]

And not

"syncSettings.resources": ["extensions","user"]

Since user settings are not configured to sync outbound, when a sync is done and there are no user settings to pull in, the user settings are simply deleted - or replaced with the null settings that have been retrieved.

To reproduce this:

  1. Backup your settings.json and everything else you care about.
  2. Generate a new profile based on the current profile.
  3. Switch to that profile. Since the new profile has no user settings, the current profile is set to "no settings".
  4. OR ... perhaps it was on switching back to the original profile that does not specify that "user" is in "resources".

Preferred behaviour : If a resource isn't specified, don't process it at all.

maxwowpow commented 6 months ago

Yes it's a dreadful experience, thanks for opening the issue. Just doing a destructive operation without any backup/confirmation can do a lot of damage.

daiyam commented 6 months ago

@TonyGravagno I can't reproduce the bug...

Preferred behaviour : If a resource isn't specified, don't process it at all.

It's what it already does: https://github.com/zokugun/vscode-sync-settings/blob/7101534588921361bac2183d1f85263758834e7c/src/repositories/file.ts#L351

Anyway, an extended profile never synchronizes the settings.json(https://github.com/zokugun/vscode-sync-settings?tab=readme-ov-file#profile-inheritance) https://github.com/zokugun/vscode-sync-settings/blob/7101534588921361bac2183d1f85263758834e7c/src/repositories/file.ts#L440

daiyam commented 6 months ago

I was able to discover a bug with the same result as yours. But, in the extended profile, the "settings"resource need to be added the "syncSettings.resources" property (while not present the base profile).

I think an extended profile shouldn't be able to change the resources property. What do you think?

Are you still able the reproduce your bug?

TonyGravagno commented 6 months ago

Sincere thanks for looking into this. I've been intensely working on other projects and just came back here. I will look into this soon.

TonyGravagno commented 2 months ago

On creating a new syncSettings.resources value in the User settings.json, the attribute "user" isn't specified - "user" isn't a valid value for Resources:

https://github.com/zokugun/vscode-sync-settings/blob/7101534588921361bac2183d1f85263758834e7c/src/repository.ts#L26

I don't know why I had "user" there and I'm sorry that this might be an invalid report.
I can't reasonably ask for a fix to an issue that was caused by invalid data. 😢
However, when editing the settings file there is no intellisense that tells us a value is invalid:

  "syncSettings.resources": [
    "extensions",
    "keybindings",
    "settings",
    "snippets",
    "uiState",
    "user" // not valid but no indication from UI
  ]

Your code looks fine. If there is an invalid option, it looks like it is ignored. https://github.com/zokugun/vscode-sync-settings/blob/7101534588921361bac2183d1f85263758834e7c/src/repositories/file.ts#L315

So now I think there might be a different trigger for that error. I have no idea what to check at the moment. I'll come back to this ticket if I see another pattern.

Thanks.