<p><b>@b4n</b> requested changes on this pull request.</p>
<p>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 <em>very</em> low). Also, <em>if</em> this gets done, it should fully embrace the new API philosophy and try and get rid of the global variables (unless actually problematic).</p>
<p>And there's at least one unrelated change (help location), which should at least be a separate commit.</p><hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442059">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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;
</pre>
<p>These globals should be removed then, unless it's <em>highly</em> impractical</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442069">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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)
</pre>
<p>style: each argument on its own line</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442073">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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;
</pre>
<p>style: blank line above return</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442075">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> }
-void
-plugin_cleanup (void)
+
+static void
+plugin_commander_cleanup (G_GNUC_UNUSED GeanyPlugin *plugin, G_GNUC_UNUSED gpointer pdata)
</pre>
<p>style: each argument on its own line</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442079">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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)
</pre>
<p>style: each argument on its own line</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442083">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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");
</pre>
<p>style: space before the open parenthesis</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442105">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -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");
</pre>
<p>Why changing the target URI? Why open a website when the same documentation is available locally?</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442128">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> @@ -762,11 +752,15 @@ on_plugin_idle_init (gpointer dummy)
return FALSE;
}
-void
-plugin_init (GeanyData *data)
+
</pre>
<p>style: only one blank line</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442132">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> {
- utils_open_browser (DOCDIR "/" PLUGIN "/README");
+ /* Setup translation */
</pre>
<p>style: indentation should be 2 spaces</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442137">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> {
- utils_open_browser (DOCDIR "/" PLUGIN "/README");
+ /* Setup translation */
+ main_locale_init(LOCALEDIR, GETTEXT_PACKAGE);
</pre>
<p>style: space before open parenthesis</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442157">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> - 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! */
</pre>
<p>this comment should be removed, as it adds no value and is not very formal</p>
<hr>
<p>In <a href="https://github.com/geany/geany-plugins/pull/703#discussion_r170442161">commander/src/commander-plugin.c</a>:</p>
<pre style='color:#555'>> + /* 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);
</pre>
<p>style: space before open parenthesis (OK, it was wrong for <code>PLUGIN_VERSOIN_CHECK()</code> before, but well)</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/geany/geany-plugins/pull/703#pullrequestreview-99132369">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABDrJ-bhUoM75mtijO_DGBn1vigYLb7hks5tYPQMgaJpZM4SR5Z_">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABDrJ9knOSbfsVs0d06_E9J_ene7uT7Zks5tYPQMgaJpZM4SR5Z_.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
<link itemprop="url" href="https://github.com/geany/geany-plugins/pull/703#pullrequestreview-99132369"></link>
<meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/geany/geany-plugins","title":"geany/geany-plugins","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/geany/geany-plugins"}},"updates":{"snippets":[{"icon":"PERSON","message":"@b4n requested changes on #703"}],"action":{"name":"View Pull Request","url":"https://github.com/geany/geany-plugins/pull/703#pullrequestreview-99132369"}}}</script>