plugin system is flawed
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Pluck CMS |
Fix Committed
|
Medium
|
Unassigned |
Bug Description
The plugin system, specifically the run_hook() function, is flawed.
First, it should be clarified in the functions description, that the parameter "par" is an array of parameters, not just an array of strings and that their meaning and index depend on the specific hook. It should also be made clear, that the parameters are passed by reference and that the return value is irrelevant, at least in effect.
And that's where the actual problem is, imho. If a function called via the hook returns a value, the processing of other possible hooks stops.
It is a design decision, either the return value of a hook determines if the "event" was handled finally, or it has some meaning and processing should not stop.
From the way the run_hook() is implemented , I suspect the return values of the individual hooks should have been collected in an array without stopping the process and returned after all hooks have been called.
But the return values of run_hook() are never used anywhere so far. Especially with the "theme_content" hook, there is unnecessary memory usage involved with the current method. The seo module for example returns the $content as well as altering it via the parameter, thus having two copies of the content in memory, at least until it is clear that the return value is not needed.
Also, I think plugin hooks should indicate the by reference passing in their declaration:
e.g. seo_theme_
The current way is sort of a "work around" for the deprecated runtime calling by reference. Maybe it's just a matter of time until this too issues a warning.
So I propose to have the hooks' return value ignored and adjust all current hook function's declarations to indicate the passing by reference.
If the return value was to determine the stopping of processing, modules should have a way to prioritize themselves. But I think that is not needed here.
Related branches
- Anders G. Jørgensen: Approve
-
Diff: 72 lines (+14/-10)3 files modifieddata/inc/editpage.php (+8/-7)
data/inc/functions.admin.php (+4/-1)
data/inc/functions.all.php (+2/-2)
Changed in pluck-cms: | |
importance: | Undecided → Medium |
fixed in lp:~uranium235/pluck-cms/pluck-cms