r112991 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r112990‎ | r112991 | r112992 >
Date:23:56, 4 March 2012
Author:krinkle
Status:ok
Tags:
Comment:
[JSGrammar] Various minor tips, tricks and clean up.
* mediawiki.language.js:
-- Implement mw.language.getData and setData. This avoids having to repeat the "if data[langCode] === undefined" code pattern all over the place (which in practice often results in the check not being programmed or forgotten (such as in mw.lang.convertGrammar)).
setData will initialize the object on-demand when needed.
getData will return 'undefined' if the object doesn't exist (whereas mw.langauge.data[someCode].get would throw an exception if the object doesn't exist)
-- Checking for $.isArray instead of thruthy-ness in convertPlural() and gender(). When isolated a truth check could be sufficient (it would fail when given a non-array thruthy value, but people simply shouldn't do that). However since the second check compares.length === 0 it makes sense to be sure it's an array first.
-- Create 'var language' as local object (for shortcut and slight performance benefit), later exposing into mw.language
-- Remove redundant truthy check for 'mw.language.convertPlural', this is defined the block below
-- Few comment fixes (var types, integer vs. Number in javascript)
-- Applying JS conventions

* ResourceLoaderLanguageModule
-- Rename module from 'language' to 'mediawiki.language.data'
-- Simplify ResourceLoaderLanguageModule::getScript
-- Add more elaborate FIXME note in ResourceLoaderLanguageModule::getModifiedTime. I haven't figured out how to properly cache and (more importantly) detect a change and purge the cache automatically for a PHP global in a ResourceLoader JS module
Modified paths:
  • /branches/jsgrammar/includes/AutoLoader.php (modified) (history)
  • /branches/jsgrammar/includes/resourceloader/ResourceLoaderLanguageDataModule.php (added) (history)
  • /branches/jsgrammar/includes/resourceloader/ResourceLoaderLanguageModule.php (deleted) (history)
  • /branches/jsgrammar/resources/Resources.php (modified) (history)
  • /branches/jsgrammar/resources/mediawiki.language/mediawiki.language.js (modified) (history)
  • /branches/jsgrammar/tests/qunit/QUnitTestResources.php (modified) (history)

Diff [purge]

Index: branches/jsgrammar/tests/qunit/QUnitTestResources.php
——@@ -25,8 +25,8 @@
2626 'tests/qunit/suites/resources/mediawiki/mediawiki.user.test.js',
2727 'tests/qunit/suites/resources/mediawiki/mediawiki.util.test.js',
2828 'tests/qunit/suites/resources/mediawiki.special/mediawiki.special.recentchanges.test.js',
29 - "tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js",
30 - "tests/qunit/suites/resources/mediawiki/mediawiki.language.test.js",
 29+ 'tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js',
 30+ 'tests/qunit/suites/resources/mediawiki/mediawiki.language.test.js',
3131 ),
3232 'dependencies' => array(
3333 'jquery.autoEllipsis',
——@@ -43,7 +43,7 @@
4444 'jquery.tablesorter',
4545 'jquery.textSelection',
4646 'mediawiki',
47 - 'language',
 47+ 'mediawiki.language.data',
4848 'mediawiki.Title',
4949 'mediawiki.user',
5050 'mediawiki.util',
Index: branches/jsgrammar/includes/resourceloader/ResourceLoaderLanguageModule.php
——@@ -1,78 +0,0 @@
2 -<?php
3 -/**
4 - * This program is free software; you can redistribute it and/or modify
5 - * it under the terms of the GNU General Public License as published by
6 - * the Free Software Foundation; either version 2 of the License, or
7 - * (at your option) any later version.
8 - *
9 - * This program is distributed in the hope that it will be useful,
10 - * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 - * GNU General Public License for more details.
13 - *
14 - * You should have received a copy of the GNU General Public License along
15 - * with this program; if not, write to the Free Software Foundation, Inc.,
16 - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17 - * http://www.gnu.org/copyleft/gpl.html
18 - *
19 - * @file
20 - * @author Santhosh Thottingal
21 - * @author Timo Tijhof
22 - */
23 -
24 -/**
25 - * ResourceLoader module for generating language specific scripts/css.
26 - */
27 -class ResourceLoaderLanguageModule extends ResourceLoaderModule {
28 -
29 - /**
30 - * Get the grammer forms for the site content language.
31 - *
32 - * @return Array
33 - */
34 - protected function getSiteLangGrammarForms() {
35 - global $wgContLang;
36 - return $wgContLang->getGrammarForms();
37 - }
38 - /**
39 - * @param $context ResourceLoaderContext
40 - * @return string Javascript code
41 - */
42 - public function getScript( ResourceLoaderContext $context ) {
43 - global $wgContLang;
44 - $code = Xml::encodeJsVar( $wgContLang->getCode() );
45 - $forms = Xml::encodeJsVar( $this->getSiteLangGrammarForms() );
46 -
47 - $js =
48 -<<<JAVASCRIPT
49 -var langCode = $code,
50 -langData = mw.language.data;
51 -if ( langData[langCode] === undefined ) {
52 - langData[langCode] = new mw.Map();
53 -}
54 -langData[langCode].set( "grammarForms", $forms );
55 -JAVASCRIPT;
56 -
57 - return $js;
58 - }
59 - /**
60 - * @param $context ResourceLoaderContext
61 - * @return array|int|Mixed
62 - */
63 - public function getModifiedTime( ResourceLoaderContext $context ) {
64 - global $wgCacheEpoch;
65 - /*
66 - global $wgContLang, $wgCacheEpoch;
67 - return max( $wgCacheEpoch, $wgContLang->getLastModified() );
68 - */
69 -
70 - return $wgCacheEpoch;
71 - }
72 - /**
73 - * @return array
74 - */
75 - public function getDependencies() {
76 - return array( 'mediawiki.language' );
77 - }
78 -}
79 -
Index: branches/jsgrammar/includes/resourceloader/ResourceLoaderLanguageDataModule.php
——@@ -0,0 +1,86 @@
 2+<?php
 3+/**
 4+ * This program is free software; you can redistribute it and/or modify
 5+ * it under the terms of the GNU General Public License as published by
 6+ * the Free Software Foundation; either version 2 of the License, or
 7+ * (at your option) any later version.
 8+ *
 9+ * This program is distributed in the hope that it will be useful,
 10+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
 11+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 12+ * GNU General Public License for more details.
 13+ *
 14+ * You should have received a copy of the GNU General Public License along
 15+ * with this program; if not, write to the Free Software Foundation, Inc.,
 16+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
 17+ * http://www.gnu.org/copyleft/gpl.html
 18+ *
 19+ * @file
 20+ * @author Santhosh Thottingal
 21+ * @author Timo Tijhof
 22+ */
 23+
 24+/**
 25+ * ResourceLoader module for populating language specific data.
 26+ */
 27+class ResourceLoaderLanguageDataModule extends ResourceLoaderModule {
 28+
 29+ /**
 30+ * Get the grammer forms for the site content language.
 31+ *
 32+ * @return array
 33+ */
 34+ protected function getSiteLangGrammarForms() {
 35+ global $wgContLang;
 36+ return $wgContLang->getGrammarForms();
 37+ }
 38+
 39+ /**
 40+ * @param $context ResourceLoaderContext
 41+ * @return string Javascript code
 42+ */
 43+ public function getScript( ResourceLoaderContext $context ) {
 44+ global $wgContLang;
 45+
 46+ return Xml::encodeJsCall( 'mw.language.setData', array(
 47+ $wgContLang->getCode(),
 48+ $this->getSiteLangGrammarForms()
 49+ ) );
 50+ }
 51+
 52+ /**
 53+ * @param $context ResourceLoaderContext
 54+ * @return array|int|Mixed
 55+ */
 56+ public function getModifiedTime( ResourceLoaderContext $context ) {
 57+ global $wgCacheEpoch;
 58+
 59+ /**
 60+ * @todo FIXME: This needs to change whenever the array created by
 61+ * $wgContLang->getGrammarForms() changes. Which gets its data from
 62+ * $wgGrammarForms, which (for standard installations) comes from LocalSettings
 63+ * and $wgCacheEpoch would cover that. However there's two three problems:
 64+ *
 65+ * 1) $wgCacheEpoch is not meant for this use.
 66+ * 2) If $wgInvalidateCacheOnLocalSettingsChange is set to false,
 67+ * $wgCacheEpoch will not be raised if LocalSettings is modified (see #1).
 68+ * 3) $wgGrammarForms can be set from anywhere. For example on WMF it is set
 69+ * by the WikimediaMessages extension. Other farms might set it form
 70+ * their 'CommonSettings.php'-like file or something (see #1).
 71+ *
 72+ * Possible solutions:
 73+ * - Store grammarforms in the language object cache instead of directly
 74+ * from the global everytime. Then use $wgContLang->getLastModified().
 75+ * - Somehow monitor the value of $wgGrammarForms.
 76+ */
 77+
 78+ return $wgCacheEpoch;
 79+ }
 80+
 81+ /**
 82+ * @return array
 83+ */
 84+ public function getDependencies() {
 85+ return array( 'mediawiki.language' );
 86+ }
 87+}
Property changes on: branches/jsgrammar/includes/resourceloader/ResourceLoaderLanguageDataModule.php
___________________________________________________________________
Added: svn:eol-style
188 + native
Index: branches/jsgrammar/includes/AutoLoader.php
——@@ -732,7 +732,7 @@
733733 'ResourceLoaderUserModule' => 'includes/resourceloader/ResourceLoaderUserModule.php',
734734 'ResourceLoaderUserOptionsModule' => 'includes/resourceloader/ResourceLoaderUserOptionsModule.php',
735735 'ResourceLoaderUserTokensModule' => 'includes/resourceloader/ResourceLoaderUserTokensModule.php',
736 - 'ResourceLoaderLanguageModule' => 'includes/resourceloader/ResourceLoaderLanguageModule.php',
 736+ 'ResourceLoaderLanguageDataModule' => 'includes/resourceloader/ResourceLoaderLanguageDataModule.php',
737737 'ResourceLoaderWikiModule' => 'includes/resourceloader/ResourceLoaderWikiModule.php',
738738
739739 # includes/revisiondelete
Index: branches/jsgrammar/resources/Resources.php
——@@ -12,8 +12,8 @@
1313 'user.options' => array( 'class' => 'ResourceLoaderUserOptionsModule' ),
1414 'user.cssprefs' => array( 'class' => 'ResourceLoaderUserCSSPrefsModule' ),
1515 'user.tokens' => array( 'class' => 'ResourceLoaderUserTokensModule' ),
16 - 'language' => array( 'class' => 'ResourceLoaderLanguageModule' ),
1716 'filepage' => array( 'class' => 'ResourceLoaderFilePageModule' ),
 17+ 'mediawiki.language.data' => array( 'class' => 'ResourceLoaderLanguageDataModule' ),
1818
1919 /* Skins */
2020
Index: branches/jsgrammar/resources/mediawiki.language/mediawiki.language.js
——@@ -5,48 +5,78 @@
66 * Language.php in MediaWiki. This object contains methods for loading and
77 * transforming message text.
88 */
9 -( function( $, mw ) {
 9+( function ( $, mw ) {
1010
11 -mw.language = {
 11+var language = {
1212 /**
13 - * @var data {Object} Language related data
14 - * Keyed by language, contains instances of mw.Map
15 - * @example Set data
16 - * <code>
17 - * var langCode = 'nl';
18 - * var langData = mw.language.data;
19 - * if ( langData[langCode] === undefined ) {
20 - * langData[langCode] = new mw.Map();
21 - * }
22 - * langData[langCode].set( .. ); // Will override, extend or create the data
23 - * </code>
24 - * @example Get data
25 - * <code>
26 - * var grammarForms = mw.language.data[langCode].get( 'grammarForms' );
27 - * </code>
28 - */
 13+ * @var data {Object} Language related data (keyed by language,
 14+ * contains instances of mw.Map).
 15+ * @example Set data
 16+ * <code>
 17+ * // Override, extend or create the language data object of 'nl'
 18+ * mw.language.setData( 'nl', 'myKey', 'My value' );
 19+ * </code>
 20+ * @example Get GrammarForms data for language 'nl':
 21+ * <code>
 22+ * var grammarForms = mw.language.getData( 'nl', 'grammarForms' );
 23+ * </code>
 24+ */
2925 data: {},
 26+
3027 /**
 28+ * Convenience method for retreiving language data by language code and data key,
 29+ * covering for the potential inexistance of a data object for this langiage.
 30+ * @param langCode {String}
 31+ * @param dataKey {String}
 32+ * @return {mixed} Value stored in the mw.Map (or undefined if there is no map for
 33+ the specified langCode).
 34+ */
 35+ getData: function ( langCode, dataKey ) {
 36+ var langData = language.data;
 37+ if ( langData[langCode] instanceof mw.Map ) {
 38+ return langData[langCode].get( dataKey );
 39+ }
 40+ return undefined;
 41+ },
 42+
 43+ /**
 44+ * Convenience method for setting language data by language code and data key.
 45+ * Creates a data object if there isn't one for the specified language already.
 46+ * @param langCode {String}
 47+ * @param dataKey {String}
 48+ * @param value {mixed}
 49+ */
 50+ setData: function ( langCode, dataKey, value ) {
 51+ var langData = language.data;
 52+ if ( !( langData[langCode] instanceof mw.Map ) ) {
 53+ langData[langCode] = new mw.Map();
 54+ }
 55+ langData[langCode].set( dataKey, value );
 56+ },
 57+
 58+ /**
3159 * Process the PLURAL template substitution
3260 *
33 - * @param {object} template Template object
 61+ * @param template {Object} Template object
3462 * @format template
35 - * {
36 - * 'title': [title of template],
37 - * 'parameters': [template parameters]
38 - * }
 63+ * {
 64+ * title: [title of template],
 65+ * parameters: [template parameters]
 66+ * }
3967 * @example {{Template:title|params}}
4068 */
41 - procPLURAL: function( template ) {
42 - if ( template.title && template.parameters && mw.language.convertPlural ) {
 69+ procPLURAL: function ( template ) {
 70+ var count;
 71+
 72+ if ( template.title && template.parameters ) {
4373 // Check if we have forms to replace
4474 if ( template.parameters.length === 0 ) {
4575 return '';
4676 }
4777 // Restore the count into a Number ( if it got converted earlier )
48 - var count = mw.language.convertNumber( template.title, true );
 78+ count = language.convertNumber( template.title, true );
4979 // Do convertPlural call
50 - return mw.language.convertPlural( parseInt( count, 10 ), template.parameters );
 80+ return language.convertPlural( parseInt( count, 10 ), template.parameters );
5181 }
5282 // Could not process plural return first form or nothing
5383 if ( template.parameters[0] ) {
——@@ -54,25 +84,27 @@
5585 }
5686 return '';
5787 },
 88+
5889 /**
5990 * Plural form transformations, needed for some languages.
6091 *
61 - * @param count integer Non-localized quantifier
62 - * @param forms array List of plural forms
63 - * @return string Correct form for quantifier in this language
 92+ * @param count {Number} Non-localized quantifier
 93+ * @param forms {Array} List of plural forms
 94+ * @return {String} Correct form for quantifier in this language
6495 */
65 - convertPlural: function( count, forms ){
66 - if ( !forms || forms.length === 0 ) {
 96+ convertPlural: function ( count, forms ){
 97+ if ( !$.isArray( forms ) || forms.length === 0 ) {
6798 return '';
6899 }
69 - return ( parseInt( count, 10 ) == 1 ) ? forms[0] : forms[1];
 100+ return ( parseInt( count, 10 ) === 1 ) ? forms[0] : forms[1];
70101 },
 102+
71103 /**
72104 * Pads an array to a specific length by copying the last one element.
73105 *
74 - * @param forms array Number of forms given to convertPlural
75 - * @param count integer Number of forms required
76 - * @return array Padded array of forms
 106+ * @param forms {Array} List of forms given to convertPlural
 107+ * @param count {Number} Number of forms required
 108+ * @return {Array} Padded array of forms
77109 */
78110 preConvertPlural: function( forms, count ) {
79111 while ( forms.length < count ) {
——@@ -80,32 +112,35 @@
81113 }
82114 return forms;
83115 },
 116+
84117 /**
85118 * Converts a number using digitTransformTable.
86119 *
87 - * @param {num} number Value to be converted
88 - * @param {boolean} integer Convert the return value to an integer
 120+ * @param num {Number} Value to be converted
 121+ * @param integer {Boolean} Convert the return value to an integer
89122 */
90123 convertNumber: function( num, integer ) {
91 - if ( !mw.language.digitTransformTable ) {
 124+ var transformTable, tmp, i, numberString, convertedNumber;
 125+
 126+ if ( !language.digitTransformTable ) {
92127 return num;
93128 }
94129 // Set the target Transform table:
95 - var transformTable = mw.language.digitTransformTable;
 130+ transformTable = language.digitTransformTable;
96131 // Check if the "restore" to Latin number flag is set:
97132 if ( integer ) {
98 - if ( parseInt( num, 10 ) == num ) {
 133+ if ( parseInt( num, 10 ) === num ) {
99134 return num;
100135 }
101 - var tmp = [];
102 - for ( var i in transformTable ) {
 136+ tmp = [];
 137+ for ( i in transformTable ) {
103138 tmp[ transformTable[ i ] ] = i;
104139 }
105140 transformTable = tmp;
106141 }
107 - var numberString = '' + num;
108 - var convertedNumber = '';
109 - for ( var i = 0; i < numberString.length; i++ ) {
 142+ numberString = String( num );
 143+ convertedNumber = '';
 144+ for ( i = 0; i < numberString.length; i++ ) {
110145 if ( transformTable[ numberString[i] ] ) {
111146 convertedNumber += transformTable[numberString[i]];
112147 } else {
——@@ -114,6 +149,7 @@
115150 }
116151 return integer ? parseInt( convertedNumber, 10 ) : convertedNumber;
117152 },
 153+
118154 /**
119155 * Provides an alternative text depending on specified gender.
120156 * Usage {{gender:[gender|user object]|masculine|feminine|neutral}}.
——@@ -121,16 +157,17 @@
122158 *
123159 * These details may be overriden per language.
124160 *
125 - * @param gender string male, female, or anything else for neutral.
126 - * @param forms array List of gender forms
 161+ * @param gender {String} Male, female, or anything else for neutral.
 162+ * @param forms {Array} List of gender forms
127163 *
128 - * @return string
 164+ * @return {String}
129165 */
130 - gender: function( gender, forms ) {
131 - if ( !forms || forms.length === 0 ) {
 166+
 167+ gender: function ( gender, forms ) {
 168+ if ( !$.isArray( forms ) || forms.length === 0 ) {
132169 return '';
133170 }
134 - forms = mw.language.preConvertPlural( forms, 2 );
 171+ forms = language.preConvertPlural( forms, 2 );
135172 if ( gender === 'male' ) {
136173 return forms[0];
137174 }
——@@ -139,24 +176,31 @@
140177 }
141178 return ( forms.length === 3 ) ? forms[2] : forms[0];
142179 },
 180+
143181 /**
144182 * Grammatical transformations, needed for inflected languages.
145183 * Invoked by putting {{grammar:form|word}} in a message.
146 - * The rules can be defined in wgGrammarForms global or grammar
 184+ * The rules can be defined in $wgGrammarForms global or grammar
147185 * forms can be computed dynamically by overriding this method per language
148186 *
149 - * @param word string
150 - * @param form string
151 - * @return string
 187+ * @param word {String}
 188+ * @param form {String}
 189+ * @return {String}
152190 */
153 - convertGrammar: function( word, form ) {
154 - var grammarForms = mw.language.data[mw.config.get( 'wgContentLanguage' )].get( 'grammarForms' );
 191+ convertGrammar: function ( word, form ) {
 192+ var grammarForms = language.getData( mw.config.get( 'wgContentLanguage' ), 'grammarForms' );
155193 if ( grammarForms && grammarForms[form] ) {
156194 return grammarForms[form][word] || word;
157195 }
158196 return word;
159197 },
160 - // Digit Transform Table, populated by language classes where applicable
 198+
 199+ /**
 200+ * @var {Object} Digit Transform Table, populated by language classes where applicable.
 201+ */
161202 digitTransformTable: null
162203 };
163 -} )( jQuery, mediaWiki );
 204+
 205+mw.language = language;
 206+
 207+}( jQuery, mediaWiki ) );

Follow-up revisions

RevisionCommit summaryAuthorDate
r113349* Minor whitespace fix in mediawiki.language.js...santhosh08:34, 8 March 2012
r113798grammar rules should set to the key 'grammarForms' instead of language code....santhosh07:09, 14 March 2012
r113800Impliment getModifiedTime for ResourceLoaderLanguageDataModule, using CACHE_A...santhosh08:34, 14 March 2012

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r111646Add grammar parsing support for mw.jqueryMsg...santhosh15:57, 16 February 2012
r111723First version of ResourceLoaderLanguageModule....santhosh04:30, 17 February 2012

Status & tagging log