plugin_admin_common

[cmb]

Hello Developers,

not long ago I found out, that the pluginloader could be used to easily implement editing of files in the back-end:

Code: Select all

        $pth['file'][$admin] = $file_name;
        $o .= plugin_admin_common($action, $admin, $plugin);
        unset($pth['file'][$admin]);
 

IMO that's pretty nice way of not reinventing the wheel. But just now I wanted to edit the file on admin=start_points instead of admin=plugin_main (note: be careful to avoid values for admin that are already used by the plugin loader) , so I came up with the following solution:

Code: Select all

        $pth['file']['start_points'] = $file_name;
        $o .= plugin_admin_common($action, 'start_points', $plugin);
        unset($pth['file']['start_points']);
 

But that didn't work. So I had a look on plugin_admin_common() and found:

Code: Select all

function plugin_admin_common($action, $admin, $plugin, $hint=ARRAY()) {

    global $sn, $action, $admin, $plugin, $pth, $tx;
 

So the three mandatory parameters are obviously superfluous! Is it necessary to global those variables? Does it have historical reasons? Can't we get rid of the global for $action, $admin, $plugin?

Christoph

[cmb]

Hello Community,

I want to bring this old thread to your attention again. I strongly suggest to get rid of this nonsense in one way or another. Note, that the $hint parameter of plugin_admin_common() is not used anymore since CMSimple_XH 1.6, and that editing of custom files in the back-end can be done easily done by other means at least since CMSimple_XH 1.6.

I see several possibilities:

1. Don't use the global variables inside plugin_admin_common()
2. Temporarily override the global variables according to the passed parameters during plugin_admin_common()
3. Make the parameters optional
4. Remove all parameters from the function signature

(1) is probably a bad idea, as plugin_admin_common() calls other functions/methods, which wouldn't cater to the given parameters. That's likely to cause a mess. (2) would solve this issue, but I have some doubts that it's really useful to pass custom parameters. Furthermore that would break plugins which pass wrong parameters inadvertently (it's all to easy to get the order of $admin and $action wrong, for example).

Actually, I tend to prefer (4), as that seems to result in the cleanest API. PHP is forgiving about passing additional parameters to a function, which could be used by func_get_args(), so there's no BC break. However, there is an RFC draft about triggering a warning on superfluous arguments, which might be accepted sometime in the future (I don't think that'll make it in the near future, though). Therefore option (3) might be preferable.

If we choose option (3) or (4), we should consider whether superfluous arguments should be silently accepted, or whether they shall trigger a deprecated warning or maybe a notice or normal warning. The latter would be the cleaner solution, in my opinion, and could be easily accomplished (if (func_num_args() > 0) trigger_error(...);), but I'm afraid that many plugins won't be updated/fixed in this regard, so silently accepting the arguments might be the more practical option. Furthermore plugin_admin_common() may be superseded by another API in the future, so why bother plugin developers with two deprecations.

Opinions? Maybe there is an even better option (5)?

[manu]

I would go for option (4).
Passing parameters has no effect, only irritation.
In future php7 will possibly throw a warning, which is visible in admin and debug mode only.
regards manu

[cmb]

I would go for option (4).
Passing parameters has no effect, only irritation.
In future php7 will possibly throw a warning, which is visible in admin and debug mode only.

ACK. Here's a respective patch (a feature branch would be overkill):

Code: Select all

Index: cmsimple/adminfuncs.php
===================================================================
--- cmsimple/adminfuncs.php	(revision 1580)
+++ cmsimple/adminfuncs.php	(working copy)
@@ -760,20 +760,14 @@
  * Handles reading and writing of plugin files
  * (e.g. en.php, config.php, stylesheet.css).
  *
- * @param bool  $action Unused.
- * @param array $admin  Unused.
- * @param bool  $plugin Unused.
- *
  * @global string The requested action.
  * @global string The requested admin-action.
  * @global string The name of the currently loading plugin.
  *
  * @return string Returns the created form or the result of saving the data.
- *
- * @todo Deprecated unused parameters.
  */
 // @codingStandardsIgnoreStart
-function plugin_admin_common($action, $admin, $plugin)
+function plugin_admin_common()
 {
 // @codingStandardsIgnoreEnd
     global $action, $admin, $plugin;

[cmb]

Done (r1616).