Plugin manager improve #249
Conversation
2. Return HotKeys list when query is like "pm *"
taooceros
force-pushed
the
PluginManagerImprove
branch
from
eb5b3e1 to
5a03587
Compare
| public class Main : ISettingProvider, IPlugin, ISavable, IContextMenu, IPluginI18n | ||
| { | ||
| internal PluginInitContext Context { get; set; } | ||
| internal static PluginInitContext Context { get; set; } |
There was a problem hiding this comment.
does this need to be static, since we are passing it through constructors already
There was a problem hiding this comment.
Oh we shouldn't make it static, though I think it is singleton. That has been changed to because I want to pass Context.API to the delegate to the default hotkey list.
There was a problem hiding this comment.
Let me check if there is a better way of passing variable.
|
thanks for making this change, really appreciate it. I might make a couple tweaks just more around formatting and push to this PR if ok |
| using var _ = File.CreateText(Path.Combine(plugin.PluginDirectory, "NeedDelete.txt")); | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
how come some times your changes contain extra space?
There was a problem hiding this comment.
hmmm, I don't know…… Maybe due to auto formatting?
Sure! |
|
A quick question, do we want the hotkeys here to be able to be modified? If not, why not make the strings to be const? @jjw24 |
….Json since our plugins.json can be large, so no need to create an extra string to store it.
Remove extra whitespace add await using for stream
add async await to download manifest task remove unused encoding name for new GetStreamAsync in Http fix unintened text error
yeah that is something i thought would be nice later down the track to allow it to be modifiable by users, although it just wasnt a priority to impment for now |
| from existingPlugin in Context.API.GetAllPlugins() | ||
| join pluginFromManifest in pluginsManifest.UserPlugins | ||
| on existingPlugin.Metadata.ID equals pluginFromManifest.ID | ||
| where existingPlugin.Metadata.Version.CompareTo(pluginFromManifest.Version) > 0 |
There was a problem hiding this comment.
just going to make a change here, dont use compareto, use equals or ==
use compareto for sorting
There was a problem hiding this comment.
Shouldn't we only update plugin if the version is higher on manifest instead of they are not equal?
Sorry, the comparison should be less than 0.😖
There was a problem hiding this comment.
my apologies, you are right actually. I was coming from the perception that we dont bother actually compare the version strings, since new updates should have version greater than the previous, but using compareto is better code.
I have changed it to less than 0
There was a problem hiding this comment.
Haha thank you. I found that due to my computer has an unpublished window walker, which has version higher than the one in our manifest. 😝
|
looks good. Will merge and lets get the release rolling 👍 |
2 participants
Uh oh!
There was an error while loading. Please reload this page.