lin-ycv / EverythingPowerToys

Everything search plugin for PowerToys Run
Eclipse Public License 2.0
2.21k stars 58 forks source link

[BUG] Setting.Options could be null #41

Closed daniel-richter closed 1 year ago

daniel-richter commented 1 year ago

Describe the bug If _setting.Options is null (what it is by default), ContextMenuLoader.LoadContextMenus will throw an exception.

To Reproduce & Version Run the plugin(?) with version ef6154b

Additional context When creating a new ContextMenuLoader, _setting.Options is null or at least could be null.

https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Main.cs#L80

The following code would fail in this case:

https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/ContextMenuLoader.cs#L39

lin-ycv commented 1 year ago

I'm not getting any exceptions or errors on my side?

On my computers, UpdateSettings is executed first on launch of PT, this is before Init so _setting is never null https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Main.cs#L135

are you running pre-release version of PT? if you're getting exceptions, can you provide log?

daniel-richter commented 1 year ago

To clarify: The Options property of the _settings object is (or can be) null.

https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Settings.cs#L9-L21

All of the other settings get initialized, but (only) Options does not.

lin-ycv commented 1 year ago

UpdateSettings is executed first at launch of PT, which initialized _setting at L135 https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Main.cs#L131-L135 In the constructor for Settings(), if there's custom settings in the settings.toml file, Options property is filled out using those settings https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Settings.cs#L42-L43 If there's no custom settings for Options (aka it's null), it's initialized to this hard-coded value https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Settings.cs#L52

So while Options should never be null? at least not when ContextMenuLoader() is being called.

daniel-richter commented 1 year ago

In the constructor for Settings(), if there's custom settings in the settings.toml file, Options property is filled out using those settings. If there's no custom settings for Options (aka it's null), it's initialized to this hard-coded value

No, not in every case. If there isn't any settings.toml found, the constructor of Settings will return immediately and won't initialize any property (see catch block in line 25). So all properties will have their default value from their property definition; Options does not have any (so it's null).

https://github.com/lin-ycv/EverythingPowerToys/blob/3ec0ddcf1e60a7259093c333cd9d21de70f4da7f/Settings.cs#L21-L25

daniel-richter commented 1 year ago

BTW: Hardcoding the path (resp. the folder) of settings.toml can fail in the following cases:

I would recommend to determine the path as follows:

string settingsPath = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "settings.toml");
lin-ycv commented 1 year ago

ah ok, I see what you mean. will make some changes.