@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.

In commander/src/commander-plugin.c:

> @@ -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 <ban@herbesfolles.org>"
-)
+static GeanyPlugin      *geany_plugin = NULL;
+static GeanyData        *geany_data = NULL;

These globals should be removed then, unless it's highly impractical

In commander/src/commander-plugin.c:

> @@ -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

In commander/src/commander-plugin.c:

> @@ -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

In commander/src/commander-plugin.c:

>  }
 
-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

In commander/src/commander-plugin.c:

> @@ -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

In commander/src/commander-plugin.c:

> @@ -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

In commander/src/commander-plugin.c:

> @@ -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?

In commander/src/commander-plugin.c:

> @@ -762,11 +752,15 @@ on_plugin_idle_init (gpointer dummy)
   return FALSE;
 }
 
-void
-plugin_init (GeanyData *data)
+

style: only one blank line

In commander/src/commander-plugin.c:

>  {
-  utils_open_browser (DOCDIR "/" PLUGIN "/README");
+	/* Setup translation */

style: indentation should be 2 spaces

In commander/src/commander-plugin.c:

>  {
-  utils_open_browser (DOCDIR "/" PLUGIN "/README");
+	/* Setup translation */
+	main_locale_init(LOCALEDIR, GETTEXT_PACKAGE);

style: space before open parenthesis

In commander/src/commander-plugin.c:

> -  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 <ban@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

In commander/src/commander-plugin.c:

> +	/* 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 <ban@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, view it on GitHub, or mute the thread.