b4n requested changes on this pull request.
I fail to see what value these changes give, given that the "old" API is fully
functional (we spent some effort to make sure it'd still work flawlessly), and
the new one, while nicer, doesn't provide any actual new feature this plugin
would need. This looks like a change for the sake of change, which I don't
really fancy (one reason being a bug likeliness to improvement ration *very*
low). Also, *if* this gets done, it should fully embrace the new API
philosophy and try and get rid of the global variables (unless actually
problematic).
And there's at least one unrelated change (help location), which should at
least be a separate commit.
> @@ -32,18 +32,8 @@
/*#define DISPLAY_SCORE 1*/
-GeanyPlugin *geany_plugin;
-GeanyData *geany_data;
-
-PLUGIN_VERSION_CHECK(226)
-
-PLUGIN_SET_TRANSLATABLE_INFO (
- LOCALEDIR, GETTEXT_PACKAGE,
- _("Commander"),
- _("Provides a command panel for quick access to actions, files and more"),
- VERSION,
- "Colomban Wendling <b...@herbesfolles.org>"
-)
+static GeanyPlugin *geany_plugin = NULL;
+static GeanyData *geany_data = NULL;
These globals should be removed then, unless it's *highly* impractical
> @@ -762,11 +752,15 @@ on_plugin_idle_init (gpointer dummy)
return FALSE;
}
-void
-plugin_init (GeanyData *data)
+
+static gboolean
+plugin_commander_init (GeanyPlugin *plugin, G_GNUC_UNUSED gpointer pdata)
style: each argument on its own line
> @@ -783,10 +777,12 @@ plugin_init (GeanyData *data)
/* delay for other plugins to have a chance to load before, so we will
* include their items */
plugin_idle_add (geany_plugin, on_plugin_idle_init, NULL);
+ return TRUE;
style: blank line above return
> }
-void
-plugin_cleanup (void)
+
+static void
+plugin_commander_cleanup (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UNUSED
gpointer pdata)
style: each argument on its own line
> @@ -796,8 +792,32 @@ plugin_cleanup (void)
}
}
-void
-plugin_help (void)
+
+static void
+plugin_commander_help (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UNUSED
gpointer pdat)
style: each argument on its own line
> @@ -796,8 +792,32 @@ plugin_cleanup (void)
}
}
-void
-plugin_help (void)
+
+static void
+plugin_commander_help (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UNUSED
gpointer pdat)
+{
+ utils_open_browser("https://plugins.geany.org/commander.html";);
style: space before the open parenthesis
> @@ -796,8 +792,32 @@ plugin_cleanup (void)
}
}
-void
-plugin_help (void)
+
+static void
+plugin_commander_help (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UNUSED
gpointer pdat)
+{
+ utils_open_browser("https://plugins.geany.org/commander.html";);
Why changing the target URI? Why open a website when the same documentation is
available locally?
> @@ -762,11 +752,15 @@ on_plugin_idle_init (gpointer dummy)
return FALSE;
}
-void
-plugin_init (GeanyData *data)
+
style: only one blank line
> {
- utils_open_browser (DOCDIR "/" PLUGIN "/README");
+ /* Setup translation */
style: indentation should be 2 spaces
> {
- utils_open_browser (DOCDIR "/" PLUGIN "/README");
+ /* Setup translation */
+ main_locale_init(LOCALEDIR, GETTEXT_PACKAGE);
style: space before open parenthesis
> - utils_open_browser (DOCDIR "/" PLUGIN "/README");
+ /* Setup translation */
+ main_locale_init(LOCALEDIR, GETTEXT_PACKAGE);
+
+ /* Set metadata */
+ plugin->info->name = _("Commander");
+ plugin->info->description = _("Provides a command panel for quick
access to actions, files and more");
+ plugin->info->version = VERSION;
+ plugin->info->author = "Colomban Wendling <b...@herbesfolles.org>";
+
+ /* Set functions */
+ plugin->funcs->init = plugin_commander_init;
+ plugin->funcs->cleanup = plugin_commander_cleanup;
+ plugin->funcs->help = plugin_commander_help;
+
+ /* Register! */
this comment should be removed, as it adds no value and is not very formal
> + /* Setup translation */
+ main_locale_init(LOCALEDIR, GETTEXT_PACKAGE);
+
+ /* Set metadata */
+ plugin->info->name = _("Commander");
+ plugin->info->description = _("Provides a command panel for quick
access to actions, files and more");
+ plugin->info->version = VERSION;
+ plugin->info->author = "Colomban Wendling <b...@herbesfolles.org>";
+
+ /* Set functions */
+ plugin->funcs->init = plugin_commander_init;
+ plugin->funcs->cleanup = plugin_commander_cleanup;
+ plugin->funcs->help = plugin_commander_help;
+
+ /* Register! */
+ GEANY_PLUGIN_REGISTER(plugin, 226);
style: space before open parenthesis (OK, it was wrong for
`PLUGIN_VERSOIN_CHECK()` before, but well)
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/geany/geany-plugins/pull/703#pullrequestreview-99132369
