search

Flow-Launcher / Flow.Launcher

:mag: Quick file search & app launcher for Windows with community-made plugins
https://flowlauncher.com
MIT License
7.82k stars 307 forks source link

Accessing Flow.Launcher.Infrastructure? #337

Closed taooceros closed 3 years ago

taooceros commented 3 years ago

Question Should we allow plugins to access Infrastructure namespace? It seems quite hard to port all utility method to Plugin namespace, and the plugin namespace seems originally supposed to only store models.

jjw24 commented 3 years ago

Yeah I think that should be fine

jjw24 commented 3 years ago

Maybe we should move share SharedCommands into Core so Core is where the shared commands will also live, and Plugin is just interface and models.

taooceros commented 3 years ago

Maybe we should move share SharedCommands into Core so Core is where the shared commands will also live, and Plugin is just interface and models.

I think you mean infrastructure because core should be the place that we don't want external plugin to access. I agree with you that we should do this if we decide to let infrastructure available to Plugins.

jjw24 commented 3 years ago

Well from my perspective Core should contain additionally the core functionalities to Flow. Whether they need to be exposed is entirely controlled via the plugin interface right. Infrastructure project is the logging, settings, user location data management side of things.

What do you mean by let infrastructure available to Plugins? I thought you would do it the same way you have done for string matcher.

taooceros commented 3 years ago

Well I think the Core contains some core functionalities that we don't want plugins to access, like PluginManager, direct access to internationalization, etc. So moving the shared command to Core may be weird because we do want plugins to have access to them.

Well, PT run is allowing Infrastructure to Plugin as a separate dependency. I am not sure whether we shall do that, or it would be better to move everything to IPublicAPI (or create a new UtilityAPI seperately). The issue leading me to think about that is some class, like JsonStorage and PluginJsonStorage actually should be available to plugins, but it may be a bit harder to do that because the Logging and some setting, which lies in Infrasturature namespace. I think it is also possible by adding log to IPublicAPI, and pass the api to JsonStorage in constructor, and move that to Plugin Interface. That may also make Plugin namespace not a clearly model interface. Or maybe we may not need to expose the whole JsonStorage to plugins, but only Save and Load, and let plugins to access them by IPluginAPI.

jjw24 commented 3 years ago

I would be leaning towards the last option, let plugin access them by IPluginAPI, Save and Load. Any models can go in shared model folder, like how you have done for StringMatcher. Would you think there will be potential issues this way?

taooceros commented 3 years ago

Well one issue is that the IPublicAPI may grow quite large, but that shouldn't be a really large issue. I think we need to try it, and then we will find some issues or no issues.

jjw24 commented 3 years ago

As long as they are just models and keep the actual logic in infra, I think it will be fine.