Talk:Hooks spec

Tim

A. The proposed implementation sounds exactly like how the events system works.

B. To comment on your proposed use cases, which I numbered for convenience: 3. The correct solution to this would be to make the whole front page display from blocks (like the My Moodle page.) 4. mime types should be handed exactly the same way as the list of countries and time zones are currently handled. 5. List of licenses should be a configuration thing. 6. What advantage does hooks give over events? 7. 7 8. seem to be the same thing. Let us redesign formslib first. Implementing hooks now would just make it harder to redesign formslib later. To solve your specific example today, I would make a "This class is in room ..." block.

So, overall, I am not convinced this add much other than complexity at the moment.--Tim Hunt 16:26, 19 April 2013 (WST)

Andrew

I feel that a hook system must be simple. That is to say that it should be as simple as calling:

   \core\hook::fire('pre_page_header_output');

If arguments are to be passed then they must all be in a single object:

   \core\hook::fire('pre_course_delete', (object) ['course' => $course]);

There should be no class to define, and it should only support autoloading. Sorry, but let's forget that legacy stuff - that was never the point of hooks. Also, the current terminology is wrong - these are not callbacks. We've said this many times. They are callables and they are called during the invocation of the hook.

Discoverability of hooks should be through other mechanisms than class phpdocs. Sorry, but they must be simple to implement otherwise we won't put them in at the right places and they will be useless. I also feel that we should look to others: 1. Wordpress 2. Drupal 3. Symfony / Cake / Laravel / etc.

Both Wordpress and Drupal go for the simple route (as my example above), and Symfony has a similar system whereby their 'Events' are simple by default but more complex versions can be passed if required.

Wordpress discoverability is via grep; Drupal is via grep and by sample (but unused) implementations of the hooks in core being documented.

I feel that we should be looking to these kinds of examples. I also think that looking at the JavaScript Event system can give us some clues. JS Events are essentially Hooks, and the EventFacade normalises the packet delivered to the EventHandler and adds additional items to it (e.g. functions to halt, prevent bubbling, prevent default action, etc.). Many of these are not relevant to us, but the halt() one could be.

I've put a feature-complete proof of concept up at https://github.com/andrewnicols/moodle/commits/MDL-44078-master which implements the above with a simple firing mechanism which passes data in a hook facade. It also adds two extra functions: 1. halt() - as discussed, the ability to stop executing further callables on the hook may be useful in some situations 2. deprecate_key() - thinking to the future if a hook originally passed an argument but wants to remove it, I think it is useful to consider this in some fashion.

It can be called in the following ways:

   // Basic hook invocation without any arguments.
   \core\hook::fire('pre_page_header_output');

   // Basic hook invocation with a single argument.
   \core\hook::fire('pre_course_delete', (object) ['course' => $course]);

   // Basic hook invocation with a simple argument (simple arguments are not passed-by-reference as standard in PHP).
   \core\hook::fire('pre_user_message_send', (object) ['messagetext' => &$message, 'author' => $user]);

   // Expanded hook invocation with an argument marked as deprecated since 3.1 and targetted for removal in 3.5.
   $hook = \core\hook::create('pre_user_delete', (object) ['user' => $user, 'userid' => $userid]);
   $hook->deprecate_key('userid', '3.1', '3.5');
   $hook->fire();

--Andrew Nicols (talk) 08:39, 16 March 2016 (AWST)

Andrew, the concept you are suggesting does not allow to have any return values. It is not suitable for the most of the existing hooks - course reset form, recent activity, etc. --Marina