Thanks for chiming in everyone. Hopefully this new behavior will save some aggravating extension-conflict headaches, and it seems the consensus is that not-returning a value should always be considered a bug in the extension.
Determining whether it was intended to return true or false or a String would have to be based on what the extension was trying to achieved, but in most cases it was probably just a forgotten "return true".
Brion wrote:
A hook function (and I mean specifically those run via wfRunHooks()) can return three possible values:
Since NULL is not an acceptable value, throwing a full-stop seems appropriate (to me). This certainly would have saved me some debugging time in the past :)
Brion wrote:
My one concern about 3) is that it may encourage continuing breakage in extensions that try to run across multiple versions, since they'd have to remember to return true for older versions.
I agree, and I'd add that coaxing NULL values into true may also cause subtle breakages where one hooking method "expects" another hooking method (in another extension) to have killed the wfRunHooks() stack due to the original behavior. Granted this is an edge case, and most likely indicates a bug in one or both extensions.
As a writer of multi-version compatible extensions, I appreciate steps taken to facilitate BC.
-- Jim
On 6/21/07, Brion Vibber brion@wikimedia.org wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Jim Wilson wrote:
Per r23133, any function that is called by a hook and does not return a value will through a full stop error.
Since not returning anything in PHP is equivalent (more or less) to returning null, is there any concern that the extension writer's intent
was
to do so? Or is this simply a massive case of forgotten "return true" statements?
A hook function (and I mean specifically those run via wfRunHooks()) can return three possible values:
true - continue running hooks, and return 'true' from wfRunHooks() false - cancel all remaining hooks, and return 'false' from wfRunHooks() a string - display the string as an error message, and return 'false' from wfRunHooks()
NULL is not a valid return value, but it's not a string and it evaluates to false when checked in boolean context. Since true is what is intended 99% of the time, the NULL default return breaks things.
There's three ways to handle that:
- Ignore it -- what we did before ;)
- Toss a big error message so it's obvious it needs to be fixed -- what
I just added to try it out 3) Change it to treat as if you returned true instead of false -- what Tim suggests as the most convenient behavior
My one concern about 3) is that it may encourage continuing breakage in extensions that try to run across multiple versions, since they'd have to remember to return true for older versions.
- -- brion vibber (brion @ wikimedia.org)
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2.2 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFGeoFlwRnhpk1wk44RAribAJ411AwRj2LqgfUct253pKBzhF5tGQCfT6xF BIF13UvzRxxvvoMz8kS0fb0= =wU7j -----END PGP SIGNATURE-----
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org http://lists.wikimedia.org/mailman/listinfo/wikitech-l