r110285 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110284‎ | r110285 | r110286 >
Date:12:52, 30 January 2012
Author:jeroendedauw
Status:reverted (Comments)
Tags:
Comment:
add hook that allows changing the check to see if a page exists or not
Modified paths:
  • /trunk/phase3/includes/Title.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Title.php
——@@ -28,7 +28,8 @@
2929 *
3030 * @internal documentation reviewed 15 Mar 2010
3131 */
32 -class Title {
 32+class
 33+Title {
3334 /** @name Static cache variables */
3435 // @{
3536 static private $titleCache = array();
——@@ -4164,7 +4165,21 @@
41654166 * @return Bool
41664167 */
41674168 public function isKnown() {
4168 - return $this->isAlwaysKnown() || $this->exists();
 4169+ $isKnown = null;
 4170+
 4171+ /**
 4172+ * Allows overriding default behaviour for determining if a page exists.
 4173+ * If $isKnown is kept as null, regular checks happen. If it's
 4174+ * a boolean, this value is returned by the isKnown method.
 4175+ *
 4176+ * @since 1.19
 4177+ *
 4178+ * @param Title $title
 4179+ * @param boolean|null $isKnown
 4180+ */
 4181+ wfRunHooks( 'TitleIsKnown', array( $this, &$isKnown ) );
 4182+
 4183+ return is_null( $isKnown ) ? ( $this->isAlwaysKnown() || $this->exists() ) : $isKnown;
41694184 }
41704185
41714186 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r110286follow up to r110285, fixed accidental newlinejeroendedauw12:54, 30 January 2012
r110289Revert r110285, r110286. No new features in core during slush....siebrand13:54, 30 January 2012
r111017put in r110285 again now that 1.19 branchedjeroendedauw01:34, 9 February 2012

Comments

#Comment by Dantman (talk | contribs)   13:26, 30 January 2012

Hmmm... I'm a wary of this change.

IIRC we use $title->exists() internally in low-level code in cases where we NEVER want the underlying software to have a false result to this test.

Also I could have sworn there was already a hook to influence things like redlinks which probably takes care of most of the cases where you'd want a hook like this.

#Comment by Jeroen De Dauw (talk | contribs)   13:48, 30 January 2012

This does not even allow influencing the behavior of $title->exists(), so that's a concern I do not get.

I looked for a hook that already allows for this, but there did not appear to be one that got applied at all places where applicable. Still, if there is a related hook, please point me to it.

#Comment by MaxSem (talk | contribs)   13:30, 30 January 2012

Please update docs/hooks.txt when adding or modifying hooks.

#Comment by Jeroen De Dauw (talk | contribs)   13:48, 30 January 2012

I know, but I figured it likely people would go mad due to slush, so why bother at this point? :)

#Comment by Siebrand (talk | contribs)   13:50, 30 January 2012

If you were aware this was bound to happen, you should have discussed on wikitech-l or #mediawiki first.

#Comment by Jeroen De Dauw (talk | contribs)   13:53, 30 January 2012

So then I'd have to post this code so people know what I was talking about. Seems not very different then having it here. After all, if it's reverted, no damage is done even if found not suited for 1.19.

Btw, are there any docs on the exact policy for this slush thing?

#Comment by 😂 (talk | contribs)   13:55, 30 January 2012

See the wikitech-l discussion from the beginning of the month, subject: "Rolling towards 1.19... scheduling a code freeze"

#Comment by Siebrand (talk | contribs)   13:36, 30 January 2012

Please revert, and consider applying after 1.19 was branched.

#Comment by 😂 (talk | contribs)   13:43, 30 January 2012

+1

Status & tagging log