r71342 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r71341‎ | r71342 | r71343 >
Date:10:25, 20 August 2010
Author:nikerabbit
Status:ok (Comments)
Tags:
Comment:
Trying to clean up the mess with $wgCanonicalNamespaceNames and $wgExtraNamespaces.

Now those two together define the set of namespaces used in the wiki.
$wgExtraNamespaces is for user configuration and overrides, while
$wgCanonicalNamespaceNames is for extension adding their own namespaces.
No code should access those two directly for reading. Instead they should use
MWNamespace::getCanonicalNamespaces.

Also fixed indentation with spaces to tabs in Language.php
Modified paths:
  • /trunk/phase3/includes/DefaultSettings.php (modified) (history)
  • /trunk/phase3/includes/Namespace.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Namespace.php
@@ -28,7 +28,8 @@
2929 NS_CATEGORY_TALK => 'Category_talk',
3030 );
3131
32 -if( isset( $wgExtraNamespaces ) && is_array( $wgExtraNamespaces ) ) {
 32+/// @todo UGLY UGLY
 33+if( is_array( $wgExtraNamespaces ) ) {
3334 $wgCanonicalNamespaceNames = $wgCanonicalNamespaceNames + $wgExtraNamespaces;
3435 }
3536
@@ -112,21 +113,41 @@
113114 * Returns whether the specified namespace exists
114115 */
115116 public static function exists( $index ) {
116 - global $wgCanonicalNamespaceNames;
117 - return isset( $wgCanonicalNamespaceNames[$index] );
 117+ $nslist = self::getCanonicalNamespaces();
 118+ return isset( $nslist[$index] );
118119 }
119120
120121
121122 /**
122 - * Returns the canonical (English Wikipedia) name for a given index
 123+ * Returns array of all defined namespaces with their canonical
 124+ * (English) names.
123125 *
 126+ * @return \array
 127+ * @since 1.17
 128+ */
 129+ public static function getCanonicalNamespaces() {
 130+ static $namespaces = null;
 131+ if ( $namespaces === null ) {
 132+ global $wgExtraNamespaces, $wgCanonicalNamespaceNames;
 133+ if ( is_array( $wgExtraNamespaces ) ) {
 134+ $namespaces = $wgCanonicalNamespaceNames + $wgExtraNamespaces;
 135+ }
 136+ $namespaces[NS_MAIN] = '';
 137+ var_dump( $namespaces );
 138+ }
 139+ return $namespaces;
 140+ }
 141+
 142+ /**
 143+ * Returns the canonical (English) name for a given index
 144+ *
124145 * @param $index Int: namespace index
125146 * @return string or false if no canonical definition.
126147 */
127148 public static function getCanonicalName( $index ) {
128 - global $wgCanonicalNamespaceNames;
129 - if( isset( $wgCanonicalNamespaceNames[$index] ) ) {
130 - return $wgCanonicalNamespaceNames[$index];
 149+ $nslist = self::getCanonicalNamespaces();
 150+ if( isset( $nslist[$index] ) ) {
 151+ return $nslist[$index];
131152 } else {
132153 return false;
133154 }
@@ -140,11 +161,10 @@
141162 * @return int
142163 */
143164 public static function getCanonicalIndex( $name ) {
144 - global $wgCanonicalNamespaceNames;
145165 static $xNamespaces = false;
146166 if ( $xNamespaces === false ) {
147167 $xNamespaces = array();
148 - foreach ( $wgCanonicalNamespaceNames as $i => $text ) {
 168+ foreach ( self::getCanonicalNamespaces() as $i => $text ) {
149169 $xNamespaces[strtolower($text)] = $i;
150170 }
151171 }
@@ -164,9 +184,7 @@
165185 static $mValidNamespaces = null;
166186
167187 if ( is_null( $mValidNamespaces ) ) {
168 - global $wgCanonicalNamespaceNames;
169 - $mValidNamespaces = array( NS_MAIN ); // Doesn't appear in $wgCanonicalNamespaceNames for some reason
170 - foreach ( array_keys( $wgCanonicalNamespaceNames ) as $ns ) {
 188+ foreach ( array_keys( self::getCanonicalNamespaces() ) as $ns ) {
171189 if ( $ns > 0 ) {
172190 $mValidNamespaces[] = $ns;
173191 }
Index: trunk/phase3/includes/DefaultSettings.php
@@ -2295,7 +2295,10 @@
22962296 /**
22972297 * Additional namespaces. If the namespaces defined in Language.php and
22982298 * Namespace.php are insufficient, you can create new ones here, for example,
2299 - * to import Help files in other languages.
 2299+ * to import Help files in other languages. You can also override the namespace
 2300+ * names of existing namespaces. Extensions developers should use
 2301+ * $wgCanonicalNamespaceNames.
 2302+ *
23002303 * PLEASE NOTE: Once you delete a namespace, the pages in that namespace will
23012304 * no longer be accessible. If you rename it, then you can access them through
23022305 * the new namespace name.
Index: trunk/phase3/languages/Language.php
@@ -241,13 +241,13 @@
242242 */
243243 function getNamespaces() {
244244 if ( is_null( $this->namespaceNames ) ) {
245 - global $wgExtraNamespaces, $wgMetaNamespace, $wgMetaNamespaceTalk;
 245+ global $wgMetaNamespace, $wgMetaNamespaceTalk;
246246
247247 $this->namespaceNames = self::$dataCache->getItem( $this->mCode, 'namespaceNames' );
248 - if ( $wgExtraNamespaces ) {
249 - $this->namespaceNames = $wgExtraNamespaces + $this->namespaceNames;
250 - }
 248+ $validNamespaces = MWNamespace::getCanonicalNamespaces();
251249
 250+ $this->namespaceNames = $validNamespaces + $this->namespaceNames;
 251+
252252 $this->namespaceNames[NS_PROJECT] = $wgMetaNamespace;
253253 if ( $wgMetaNamespaceTalk ) {
254254 $this->namespaceNames[NS_PROJECT_TALK] = $wgMetaNamespaceTalk;
@@ -258,13 +258,10 @@
259259 }
260260
261261 # Sometimes a language will be localised but not actually exist on this wiki.
262 - global $wgCanonicalNamespaceNames;
263 - $validNamespaces = array_keys($wgCanonicalNamespaceNames);
264 - $validNamespaces[] = NS_MAIN;
265262 foreach( $this->namespaceNames as $key => $text ) {
266 - if ( ! in_array( $key, $validNamespaces ) ) {
267 - unset( $this->namespaceNames[$key] );
268 - }
 263+ if ( !isset( $validNamespaces[$key] ) ) {
 264+ unset( $this->namespaceNames[$key] );
 265+ }
269266 }
270267
271268 # The above mixing may leave namespaces out of canonical order.

Follow-up revisions

RevisionCommit summaryAuthorDate
r71366Missing else condition was breaking namespaces for everyone else but me. Foll...nikerabbit16:28, 20 August 2010
r71388Fix translated core namespaces broken in r71342....nikerabbit08:05, 21 August 2010
r71391One more fix to namespace stuff. Follow-up r71342....nikerabbit08:20, 21 August 2010
r71467Fix yet another regression in r71342: NS_MAIN was dropped from valid namespac...nikerabbit08:46, 23 August 2010

Comments

#Comment by Nikerabbit (talk | contribs)   10:31, 20 August 2010

var_dump removed in r71343.

#Comment by Platonides (talk | contribs)   19:49, 20 August 2010

This breaks namespace translations. See (after 71366) the tests "Namespace (lang=de) Benutzer User", "Namespace (lang=de) Benutzer Diskussion User talk" and "Simple category in language variants",

#Comment by Nikerabbit (talk | contribs)   19:54, 20 August 2010

Does it work if you change?

-			$this->namespaceNames = $validNamespaces + $this->namespaceNames;
+			$this->namespaceNames = $this->namespaceNames + $validNamespaces;

If so, probably has to restore $wgExtraNamespaces and do $wgExtraNamespaces + $this->namespaces + $validNamespaces.

#Comment by Platonides (talk | contribs)   20:32, 20 August 2010

It does.

Regarding the $wgCanonicalNamespaceNames for extensions, I don't think extensions should be fiddling with $wgCanonicalNamespaceNames. It should be moved inside the class and extensions should call some method for adding a namespace with all the options it needs. That could also return a namespace index which was free, based on some hash of the name.

#Comment by Nikerabbit (talk | contribs)   20:46, 20 August 2010

Maybe, but in the short them we just want working extension namespace translations.

#Comment by Nikerabbit (talk | contribs)   17:56, 24 August 2010

On a second thought, the current way is prone to bugs caused by race conditions (seen on twn). Looks like we need to add a hook.

Status & tagging log