Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add plugins overview #194

Merged
merged 11 commits into from
Jul 1, 2021
Merged

Add plugins overview #194

merged 11 commits into from
Jul 1, 2021

Conversation

clausnagel
Copy link
Member

This PR adds a "Plugins" section to the preferences tab of the Importer/Exporter GUI, which lists all installed and available plugins. Each plugin is displayed with additional information such as its version, its vendor, a description and the implemented extensions of the Plugin API. In addition, the plugins can be enabled/disabled using this dialog.

The Plugin API has been changed in a non-backwards compatible way to make this new dialog possible. Every plugin must now provide a plugin.xml file that is stored in the same package as the plugin class itself and that provides the additional metadata. In addition, an optional pluginLogo.svg (and a pluginLogo_dark.svg for the dark theme) can be provided.

The new "Plugins" section can be tested with the ADE Manager plugin and the Spreadsheet Generator plugin. Simply use the feature-new-plugin-api branch for both plugins.

# Conflicts:
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/common/DefaultPreferencesEntry.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/database/DatabasePlugin.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/exporter/CityGMLExportPlugin.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/importer/CityGMLImportPlugin.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/preferences/PreferencesPlugin.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/preferences/view/PreferencesPanel.java
#	impexp-client-gui/src/main/java/org/citydb/gui/operation/visExporter/VisExportPlugin.java
@yaozhihang
Copy link
Member

yaozhihang commented Jun 30, 2021

Thanks @clausnagel. Very cool new feature.

I have two remarks after my review:

  1. How about adding an additional CLI option like --enabledPlugins? This could be useful when users want to enable/disable certain FeatureExportExtensions
  2. It might be a Nice to Have to allow users to change the order of plugins listed in the preference panel. A benefit is that the plugins of FeatureExportExtensions can be run by following an user-defined order to build a process chain.

All other things work well in my tests. 👍

@clausnagel
Copy link
Member Author

Thanks, @yaozhihang.

re 1: Should --enabledPlugins only list all enabled/disabled plugins, or would you like to be able to actively enable/disable a plugin on the command line? In the latter case, what would be the input to the paramter? The class name of the plugin?

re 2: In general, you cannot define dependencies for plugins (for instance, you cannot say that plugin B should only be executed after plugin A). The plugins overview simply lists the plugins in alphabetical order.

I agree that it would be nice to be able to define the execution order for separate FeatureExportExtensions. For other extensions like ConfigExtension or ViewExtension, an execution order does not make sense though. We could add a <depends-on> element to the plugin.xml configuration to express a dependency - this would be static then (and again would not make sense for all plugins). But what if the plugin referenced by <depends-on> is not installed or disabled? If we want to have it dynamic, I would rather not use the general plugins overview for this purpose but would recommend a specific dialog. This dialog should only list those plugins for which defining an execution order makes sense (could be a separate "Execution order" node beneath the "Plugins" node in the preferences tree). Hm, sounds doable but requires more effort.

@yaozhihang
Copy link
Member

  1. The --enabledPlugins should allow to actively enable a list of plugins. I think we could use the full class name of plugin e.g. org.citydb.plugins.spreadsheet_gen.SPSHGPlugin
  2. I prefer your latter idea. Yes, you are right. it sounds doable and need more coding. We could move this to a separate PR.

@clausnagel
Copy link
Member Author

clausnagel commented Jul 1, 2021

@yaozhihang, I added a --use-plugin option with 189de12 that allows to enable or disable plugins on the command line. You have to provide the fully qualified class name of the plugin and, optionally, you can set true or false to enable or disable it (true is the default value). For instance, the following command uses the plugin a.b.c.PluginA but disables d.e.f.PluginB:

    $ impexp gui --use-plugin a.b.c.PluginA --use-plugin d.e.f.PluginB=false

I agree to move your second proposal to another PR once we have a requirement to support the execution order of specific plugins.

@yaozhihang
Copy link
Member

@clausnagel Looks very good.

@clausnagel clausnagel merged commit 5d17674 into master Jul 1, 2021
@clausnagel clausnagel deleted the feature-plugins-settings branch July 1, 2021 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants