New plugin type setting...

jm · 2008-06-20 23:56:41

ruud wrote:

[…] one would assume they’re intelligent enough to understand numbers ;)

It’s not understanding numbers; it’s having to memorize arbitrary meanings. Given most of TXP’s code isn’t documented¹, adding more magic numbers is not ideal. How clear is the following?

switch ($type)
{
    case 0:
        //...
        break;
    case 1:
        //...
        break;
    case 2:
        //...
        break;
    default:
        //...
}

Logical strings like “admin”, “lib”, “public,” or “all” make more sense from both TXP- and plugin-development perspectives.

Also, why would we need to wait till vNever for this? If it’s backwards-compatible, it could be implemented rather easily (do the old-boolean processing in an else).

¹ //-----------... AKA “Insert documentation here.”

Last edited by jm (2008-06-20 23:59:23)

ruud · 2008-06-21 08:46:21

The plugin type is documented in zem_tpl, which is the de-facto standard for plugins.

Changing numbers to words doesn’t necessarily make it clearer (not with the suggested words, IMHO) and it just makes the TXP code more crufty.

jm · 2008-06-21 11:38:16

ruud wrote:

[…] not with the suggested words, IMHO

ruud wrote:

type 0 = not loaded (= lib, I presume)
type 1 = public
type 2 = admin
type 3 = admin + public (=1+2)

The only difference is all/admin+public. Why would you want to rely on documentation elsewhere?

The redundant code in load_plugin|s and install_plugin is crufty. Documenting idiotic magic numbers in the code, a plugin template, or a wiki adds cruft. Well named variables simplify the code, so new developers can pick it up easily. If you don’t see that, then there’s really no hope for txplib_misc.php.

ruud · 2008-06-21 13:14:57

Sorry, but it seems to me this is change for the sake of changing stuff. Why solve a problem that doesn’t exist. I don’t see regular postings asking about the meaning of various plugin types.

Can we get back on-topic, which was having an option for admin-only plugins?

jm · 2008-06-21 13:23:25

It’s a change for improving TXP’s code and functionality. The update_to_4.0.X.php file can include an alter table query, so it’s not a very difficult modification.

Last edited by jm (2008-06-21 13:24:21)

net-carver · 2008-07-23 15:32:18

Devs

Just to stop this dropping off the radar. from my pov, even if we don’t get a brand-new string-based plugin type, could we please…

…get a type value that makes a plugin load only in the admin interface?
…consider not loading library plugins by default on the public interface; instead have libraries loaded by the public plugins that explicitly load/require them?

ruud · 2008-07-23 16:41:23

0 = public
1 = public+admin
2 = lib
3 (new) = admin

I wouldn’t object to that.

Are library plugins loaded by default on the public side? If so, that makes no sense, does it. What’s the difference with type 1?

net-carver · 2008-07-23 16:49:44

ruud wrote:

Are library plugins loaded by default on the public side?

As fas as my code inspection goes, every single enabled plugin is loaded by default on the public side…

	if ($use_plugins) load_plugins();

…taken from publish.php. Then load_plugins gets them all.

ruud · 2008-07-23 17:08:15

Ouch… I think the library thing is a bug which should be fixed anyway.

Admin-side only plugins (type 3). I’ll write a patch. Unless there are objections against this (Robert or anyone else?) I think this is worth adding.

ruud · 2008-07-23 17:19:19

Something like this perhaps:

--- textpattern/lib/txplib_misc.php	(revision 2946)
+++ textpattern/lib/txplib_misc.php	(working copy)
@@ -537,7 +537,7 @@
 	}

 // -------------------------------------------------------------
-	function load_plugins($type=NULL)
+	function load_plugins($type=0)
 	{
 		global $prefs,$plugins, $plugins_ver;

@@ -557,9 +557,7 @@
 			}
 		}

-		$where = 'status = 1';
-		if ($type !== NULL)
-			$where .= (" and type='".doSlash($type)."'");
+		$where = 'status = 1 and type IN ('.($type ? '1,3' : '0,1').')';

 		$rs = safe_rows("name, code, version", "txp_plugin", $where. ' order by load_order');
 		if ($rs) {

--- textpattern/include/txp_plugin.php	(revision 2946)
+++ textpattern/include/txp_plugin.php	(working copy)
@@ -295,7 +295,7 @@

 					extract($plugin);

-					$type  = empty($type)  ? 0 : min(max(intval($type), 0), 2);
+					$type  = empty($type)  ? 0 : min(max(intval($type), 0), 3);
 					$order = empty($order) ? 5 : min(max(intval($order), 1), 9);

 					$exists = fetch('name','txp_plugin','name',$name);

net-carver · 2008-07-23 17:24:20

Ruud

fantastic, thank you for looking at this. Don’t forget the documentation in Zem’s example plugin type ‘3’ would need adding and it’s not exactly accurate describing type 1 as ‘admin’. Should be ‘Common’ or ‘Public & Admin’ or some-such.

wet · 2008-07-23 17:28:58

ruud wrote:

Unless there are objections against this

Fine for me.

Textpattern CMS

Textpattern CMS support forum

#13 2008-06-20 23:56:41

Re: New plugin type setting...

#14 2008-06-21 08:46:21

Re: New plugin type setting...

#15 2008-06-21 11:38:16

Re: New plugin type setting...

#16 2008-06-21 13:14:57

Re: New plugin type setting...

#17 2008-06-21 13:23:25

Re: New plugin type setting...

#18 2008-07-23 15:32:18

Re: New plugin type setting...

#19 2008-07-23 16:41:23

Re: New plugin type setting...

#20 2008-07-23 16:49:44

Re: New plugin type setting...

#21 2008-07-23 17:08:15

Re: New plugin type setting...

#22 2008-07-23 17:19:19

Re: New plugin type setting...

#23 2008-07-23 17:24:20

Re: New plugin type setting...

#24 2008-07-23 17:28:58

Re: New plugin type setting...

Board footer