r75040 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r75039‎ | r75040 | r75041 >
Date:21:05, 19 October 2010
Author:tparscal
Status:ok (Comments)
Tags:
Comment:
* Added extendable fallback closure to configuration prototype.
* Renamed configuration to map, since it's being used more generally than just for configurations.
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -56,9 +56,10 @@
5757 *
5858 * @param {boolean} global whether to get/set/exists values on the window object or a private object
5959 * @param {function} parser function to perform extra processing; in the form of function( value, options )
 60+ * @param {function} fallback function to format default fallback; in the form of function( key )
6061 * where value is the data to be parsed and options is additional data passed through to the parser
6162 */
62 - 'configuration': function( global, parser ) {
 63+ 'map': function( global, parser, fallback ) {
6364
6465 /* Private Members */
6566
@@ -98,8 +99,13 @@
99100 return results;
100101 } else if ( typeof selection === 'string' ) {
101102 if ( typeof values[selection] === 'undefined' ) {
102 - return typeof options === 'object' && 'fallback' in options ?
103 - options.fallback : '<' + selection + '>';
 103+ if ( typeof options === 'object' && 'fallback' in options ) {
 104+ return options.fallback;
 105+ } else if ( typeof fallback === 'function' ) {
 106+ return fallback( selection );
 107+ } else {
 108+ return null;
 109+ }
104110 } else {
105111 if ( typeof parser === 'function' ) {
106112 return parser( values[selection], options );
@@ -158,7 +164,7 @@
159165 *
160166 * In legacy mode the values this object wraps will be in the global space
161167 */
162 - this.config = new this.prototypes.configuration( LEGACY_GLOBALS );
 168+ this.config = new this.prototypes.map( LEGACY_GLOBALS );
163169
164170 /*
165171 * Information about the current user
@@ -167,7 +173,7 @@
168174
169175 /* Public Members */
170176
171 - this.options = new that.prototypes.configuration();
 177+ this.options = new that.prototypes.map();
172178 } )();
173179
174180 /*
@@ -186,7 +192,7 @@
187193 /*
188194 * Localization system
189195 */
190 - this.msg = new that.prototypes.configuration( false, this.parser );
 196+ this.msg = new that.prototypes.map( false, this.parser, function( key ) { return '<' + key + '>'; } );
191197
192198 /*
193199 * Client-side module loader which integrates with the MediaWiki ResourceLoader

Comments

#Comment by Catrope (talk | contribs)   15:08, 20 October 2010
+		 * @param {function} fallback function to format default fallback; in the form of function( key )

You should document the fact that it's overridden by options.fallback. IMO this should be done more flexibly such that both the "globally" specified (i.e. in the constructor call) fallback and the override in the options param can be strings or functions.

#Comment by Trevor Parscal (WMF) (talk | contribs)   20:59, 20 October 2010

That won't work, what if you are using the map prototype to store key/function pairs? A default value being a function would then make sense, and executing it inside the function would not. The idea for this being in the constructor is simply to extend the implementation on the fly, allowing configurations to return null when no fallback is given, and messages to return <msg-key>.

#Comment by Catrope (talk | contribs)   21:01, 20 October 2010

Yeah, that makes sense.

Status & tagging log