Allow Desktop Shortcut Deletion

[mrixner]

• I have read the contributing guidelines, and I agree with the Code of Conduct.
• Have you checked that there aren't other open pull requests for the same changes?
• Have you tested that the committed code can be executed without errors?
• This PR is not composed of garbage changes used to farm GitHub activity to enter potential Crypto AirDrops.
  Any user suspected of farming GitHub activity with crypto purposes will get banned. Submitting broken code wastes the contributors' time, who have to spend their free time reviewing, fixing, and testing code that does not even compile breaks other features, or does not introduce any useful changes. I appreciate your understanding.

---

This pull request allows users to have UniGetUI delete shortcuts automatically added to their desktop. This will be off by default, behind a setting, and require explicit user confirmation before UniGetUI deletes anything.

• Change desktop shortcut storage to use the new complex settings
• Copy the detection code to the InstallPackageOperation as well and have a layer setting (marticliment)

---

Closes #2942
Closes #2080
Closes #624
Relates to #2160

[mrixner]

@marticliment - The base functionality for this PR is complete, however it felt odd to me to effectively have a copy paste of very similar code for the storage of the ignored updates and desktop shortcuts databases. I was thinking about maybe modifying both of them to use some sort of arbitrary mass configurator, like a settings system but allowing for more complicated storage; in these cases, dictionaries. This would also be useful for some other feature requests I was thinking about adding (unless you don't want me to; I wouldn't want to be a burden on your repository).

I haven't done a whole lot of thinking; I'll do much more if you're OK with me adding it but I'm thinking the rough (very rough!) behavior would be something like an ArbitraryMassConfigurator with a Get function that would return some implementation of a Configurator<T> that can store different types of data (with basic functions like Add(name, T), Change(name, T), and Remove(name)); maybe a KeyValueConfigurator<K, V> to store maps (like the IgnoredUpdates and DesktopShortcuts databases do) [Add(K, V)], a ListConfigurator<V> to store a list of values (useful for an operation history) [AddValue(T)], a KeyMapConfigurator<K, Map<K, V>> to store key-map pairs (for package installation options) [Add(K, Map<K, V>)], etc. Each configurator would then take inputs and convert them to JSON, handing it back to the ArbitraryMassConfigurator which would write it all to one file. That way, all settings that are non-trivial are handled together consistently.

Does that sound like something that would be helpful? If you're not interested I'll mark this PR as ready for review, but if you are I'll work on it here (or I suppose make a new one; up to you).

[mrixner]

Also, when you test this, will you make sure the setting is off by default? I thought it was, but I'm having second thoughts now (but without a clean installation to test it).

[marticliment]

I will be busy this weeks, but I should be able to get this reviewed and merged in a week or two.

I haven't done a whole lot of thinking; I'll do much more if you're OK with me adding it but I'm thinking the rough (very rough!) behavior would be something like an ArbitraryMassConfigurator with a Get function that would return some implementation of a Configurator<T> that can store different types of data (with basic functions like Add(name, T), Change(name, T), and Remove(name)); maybe a KeyValueConfigurator<K, V> to store maps (like the IgnoredUpdates and DesktopShortcuts databases do) [Add(K, V)], a ListConfigurator<V> to store a list of values (useful for an operation history) [AddValue(T)], a KeyMapConfigurator<K, Map<K, V>> to store key-map pairs (for package installation options) [Add(K, Map<K, V>)], etc. Each configurator would then take inputs and convert them to JSON, handing it back to the ArbitraryMassConfigurator which would write it all to one file. That way, all settings that are non-trivial are handled together consistently.

Perhaps the Settings class could be extended to load and store dictionaries and/or lists (perhaps with custom type parameters that can be serializable to JSON via ISerializable?). Feel free to experiment with it, but please open a new pr. Also, if possible, i'd like to discuss the implementation of it, so we can make sure it properly integrates with UniGetUI and the existing codebase.

This would also be useful for some other feature requests I was thinking about adding (unless you don't want me to; I wouldn't want to be a burden on your repository).

Again, feel free, but before implementation it'd be cool if we could discuss the implementation (a general sketch on how it'd work, etc.). Feel free to use the existing relevant issue, if any, or if not perhaps you can create the empty PR.

[mrixner]

I will be busy this weeks, but I should be able to get this reviewed and merged in a week or two.

Of course. Take your time :)

Perhaps the Settings class could be extended to load and store dictionaries and/or lists (perhaps with custom type parameters that can be serializable to JSON via ISerializable?). Feel free to experiment with it, but please open a new pr. Also, if possible, i'd like to discuss the implementation of it, so we can make sure it properly integrates with UniGetUI and the existing codebase.

GitHub won't let me create an empty draft PR...

That's a much better idea, hence why I decided to run it past you first :) My thoughts now would be extending the settings object to support lists and dictionaries as well as just booleans on strings.

For lists, maybe Settings could have: (where T is ISerializable)

• List<T> Settings.GetList<T>(string ListName)
• void Settings.SetList<T>(string ListName, List<T> List)
• T Settings.GetListItem<T>(string ListName, int Index)
• void Settings.AddToList<T>(string ListName, T Item)
• bool Settings.RemoveFromList<T>(string ListName, T Item)
• bool Settings.RemoveFromList(string ListName, int Index)
• void Settings.ClearList(string ListName)
• void Settings.ListContains<T>(string ListName, T Item)
• int Settings.ListCount(string ListName)

For dictionaries: (where K and V are ISerializable)

• Dictionary<K, V> Settings.GetDictionary<K, V>(string DictName)
• void Settings.SetDictionary<K, V>(string DictName, Dictionary<K, V> Dict)
• V Settings.GetDictionaryItem<K, V>(string DictName, K Key)
• void Settings.SetDictionaryItem<K, V>(string DictName, K Key, V Value) (also works as Add)
• void Settings.RemoveDictionaryKey<K>(string DictName, K Key)
• void Settings.ClearDictionary(string DictName)
• void Settings.DictionaryContainsKey<K>(string DictName, K Key)
• void Settings.DictionaryContainsValue<V>(string DictName, V Value)
• int Settings.DictionaryCount(string DictName)

Although I suppose at the base level all it really needs is Set/Get if you don't want to add that many functions to the Settings class.

Obviously we could just serialize lists and dictionaries into strings for SetValue/GetValue, which I think would work for the ignored updates manager and desktop shortcuts manager, since they have their own database to abstract the complexity. But for things such as a recently updated list or package installation options it's much safer (less error-prone) and more thread-safe to have all of that abstracted since it just needs to call a settings function to get or add a list item rather than repeatedly serializing and deserializing the whole list, which is a lot of repetitive code. I think the above functions should cover everything including nested dictionaries, even excessively as you could technically just use Get().Count instead of .Count(). What do you think?

[marticliment]

Obviously we could just serialize lists and dictionaries into strings for SetValue/GetValue, which I think would work for the ignored updates manager and desktop shortcuts manager, since they have their own database to abstract the complexity. But for things such as a recently updated list or package installation options it's much safer (less error-prone) and more thread-safe to have all of that abstracted since it just needs to call a settings function to get or add a list item rather than repeatedly serializing and deserializing the whole list, which is a lot of repetitive code. I think the above functions should cover everything including nested dictionaries, even excessively as you could technically just use Get().Count instead of .Count(). What do you think?

The interface you are proposing is amazing. The only thing I would say is that perhaps some methods could be considered redundant (remove(int index), Count(), ...), because while the idea is exposing a list that automatically saves and loads to/from disk, I can't really think of an usecase where you'd need the length without actually having the need to use the entire list.

Also, since modifying the returned lists or values does not update the values on the settings, I'd set the return values for lists and dicts to be IReadOnlyList and IReadOnlyDictionary, (which are inherited by List and Dictionary and hence only require a cast), so the action of modifying a returned list can not be misunderstood as updating the settings. What do you think about this?

[mrixner]

The only thing I would say is that perhaps some methods could be considered redundant (remove(int index)

If you don't want a redundant remove item is it better to keep remove by index or remove by value? It's possible my C++ background and it's inability to remove something by value from a list without writing some ridiculous 95 character line is showing through here but I just wasn't sure about only leaving a remove by value method.

I can't really think of an usecase where you'd need the length without actually having the need to use the entire list.

That's a good point. Unless you want some sort of Tooltip on the operation history button to say how many there are? But I think that's a stretch.

Also, since modifying the returned lists or values does not update the values on the settings, I'd set the return values for lists and dicts to be IReadOnlyList and IReadOnlyDictionary, (which are inherited by List and Dictionary and hence only require a cast), so the action of modifying a returned list can not be misunderstood as updating the settings. What do you think about this?

Yes, that's a good idea. I didn't know about those.

The interface you are proposing is amazing.

Alright, I'll go work on this when it's not, you know, almost 2am.

[marticliment]

If you don't want a redundant remove item is it better to keep remove by index or remove by value? It's possible my C++ background and it's inability to remove something by value from a list without writing some ridiculous 95 character line is showing through here but I just wasn't sure about only leaving a remove by value method.

Taking into account the usage the settings will have I would maintain the remove by value, as I don't see as useful remove by index in the usecases settings will be more needed for.

That's a good point. Unless you want some sort of Tooltip on the operation history button to say how many there are? But I think that's a stretch.

No, it is not a stretch, but I reckon this is the task of the operation history (or whichever class fetches said issue) to fetch the data efficiently and reuse the data read (which at the end it will need to read the entire list) for the tooltips and other info.

[marticliment]

You should be able to pull main with the new complex settings

[marticliment]

I have simplified the ContentDialog and moved some methods to the ShortcutManagerDatabase.
Please confirm that the changes I have made still work on your machine.

[mrixner]

Everything works fine on my machine, and it all looks good aside from the comments I made.