lin-ycv / EverythingPowerToys

Everything search plugin for PowerToys Run
Eclipse Public License 2.0
2.2k stars 57 forks source link

[BUG] PluginJsonStorage is meaningless #83

Closed KawaiiZapic closed 5 months ago

KawaiiZapic commented 7 months ago

Describe the bug This plugin use PluginJsonStorage with ISettingsProvider, which will save 2 copies of data: another copy of data is handled by PJStorage and saved in a JSON file, another copy of data is handled by PT Run itself, saved in %LocalAppDAta%\Microsoft\PowerToys\PowerToys Run\settings.json.
Everytime plugin load, it will load data by PJStorage first, and load again by PT Run itself from settings.json, so the data from PJStorage is completely meaningless and will be overwrite everytime when plugin load.

As reference, PT Run will delete all data in PJStorage when PT upgraded Powertoys#31269, but no one report problem like "My settings is losed after upgrade", because data in PJStorage is never used. To Reproduce Not provieded

Expected behavior Not provieded

Screenshots Not provieded

Logs (please upload or provide link to you log): Not provieded

Version (please provide the version of software you are using): Not provieded

Additional context Not provieded

lin-ycv commented 7 months ago

I haven't looked at what they're doing in the backend, but from testing, both ISettingsProvider and PluginJsonStorage(ISavable) are required. If one of them is missing, something breaks. Waiting for MS to reply on your other issue, then I'll take a look at it again accordingly.

lin-ycv commented 6 months ago

So my implementation basically followed the same implementation as the shell plugin. MS have made some changes to PT since I implemented non-boolean values in the settings (v0.66.0), which seems to make it possible to not use PluginJsonStorage, but this will require rewrites to a portion of the current logic.

I don't think I'll have time to "fix" this in the next few releases. If someone has time to do this, I'd welcome a PR.