r111272 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111271‎ | r111272 | r111273 >
Date:04:45, 12 February 2012
Author:siebrand
Status:resolved (Comments)
Tags:i18nreview 
Comment:
* Move hard coded styles in MessageWebImporter to CSS.
* Break long line.
* Bump version.
* Update an unanswered @todo.
* Add MessageGroups::getAllMessageGroups() as a replacement of MessageGroups::getGroups()
* Update MessageGroups::getAllMessageGroups() to reduce code duplication in export.php and sync-group.php. Added two optional parameters:
** $groups: Get an array of groups for an array of group IDs.
** $groupPrefix: Get an array of groups with a given prefix.
* Exit export.php if no valid group IDs are given.
* i18n for two hard coded strings.
* Add FIXME for missing i18n.
Modified paths:
  • /trunk/extensions/Translate/Groups.php (modified) (history)
  • /trunk/extensions/Translate/MediaWikiMessageChecker.php (modified) (history)
  • /trunk/extensions/Translate/MessageGroups.php (modified) (history)
  • /trunk/extensions/Translate/Translate.i18n.php (modified) (history)
  • /trunk/extensions/Translate/Translate.php (modified) (history)
  • /trunk/extensions/Translate/resources/ext.translate.messagewebimporter.css (added) (history)
  • /trunk/extensions/Translate/scripts/export.php (modified) (history)
  • /trunk/extensions/Translate/scripts/sync-group.php (modified) (history)
  • /trunk/extensions/Translate/utils/MessageWebImporter.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Translate/scripts/sync-group.php
——@@ -45,35 +45,12 @@
4646 exit( 1 );
4747 }
4848
49 -$groups = array();
 49+// In case both group and groupprefix would be set, MessageGroups::getMessageGroups
 50+// will give preference to groupIds.
 51+$groupIds = isset( $options['group'] ) ? explode( ',', trim( $options['group'] ) ) : null;
 52+$groupPrefix = isset( $options['groupprefix'] ) ? $options['groupprefix'] : null;
 53+$groups = MessageGroups::getMessageGroups( $groupIds, $groupPrefix );
5054
51 -// @todo FIXME: Code duplication with export.php
52 -if ( isset( $options['group'] ) ) {
53 - // Explode parameter
54 - $groupIds = explode( ',', trim( $options['group'] ) );
55 -
56 - // Get groups and add groups to array
57 - foreach ( $groupIds as $groupId ) {
58 - $group = MessageGroups::getGroup( $groupId );
59 -
60 - if ( $group !== null ) {
61 - $groups[$groupId] = $group;
62 - } else {
63 - STDERR( "Invalid group $groupId" );
64 - }
65 - }
66 -} else {
67 - // Apparently using option groupprefix. Find groups that match.
68 - $allGroups = MessageGroups::singleton()->getGroups();
69 -
70 - // Add matching groups to groups array.
71 - foreach ( $allGroups as $groupId => $messageGroup ) {
72 - if ( strpos( $groupId, $options['groupprefix'] ) === 0 && !$messageGroup->isMeta() ) {
73 - $groups[$groupId] = $messageGroup;
74 - }
75 - }
76 -}
77 -
7855 if ( !count( $groups ) ) {
7956 STDERR( "ESG2: No valid message groups identified." );
8057 exit( 1 );
Index: trunk/extensions/Translate/scripts/export.php
——@@ -88,33 +88,15 @@
8989
9090 $reqLangs = Cli::parseLanguageCodes( $options['lang'] );
9191
92 -$groups = array();
 92+// In case both group and groupprefix would be set, MessageGroups::getMessageGroups
 93+// will give preference to groupIds.
 94+$groupIds = isset( $options['group'] ) ? explode( ',', trim( $options['group'] ) ) : null;
 95+$groupPrefix = isset( $options['groupprefix'] ) ? $options['groupprefix'] : null;
 96+$groups = MessageGroups::getMessageGroups( $groupIds, $groupPrefix );
9397
94 -// @todo FIXME: Code duplication with sync-group.php
95 -if ( isset( $options['group'] ) ) {
96 - // Explode parameter
97 - $groupIds = explode( ',', trim( $options['group'] ) );
98 -
99 - // Get groups and add groups to array
100 - foreach ( $groupIds as $groupId ) {
101 - $group = MessageGroups::getGroup( $groupId );
102 -
103 - if ( $group !== null ) {
104 - $groups[$groupId] = $group;
105 - } else {
106 - STDERR( "Invalid group $groupId" );
107 - }
108 - }
109 -} else {
110 - // Apparently using option groupprefix. Find groups that match.
111 - $allGroups = MessageGroups::singleton()->getGroups();
112 -
113 - // Add matching groups to groups array.
114 - foreach ( $allGroups as $groupId => $messageGroup ) {
115 - if ( strpos( $groupId, $options['groupprefix'] ) === 0 && !$messageGroup->isMeta() ) {
116 - $groups[$groupId] = $messageGroup;
117 - }
118 - }
 98+if ( !count( $groups ) ) {
 99+ STDERR( "No valid message groups identified." );
 100+ exit( 1 );
119101 }
120102
121103 foreach ( $groups as $groupId => $group ) {
Index: trunk/extensions/Translate/MessageGroups.php
——@@ -1032,7 +1032,6 @@
10331033 * @todo Clean up the mixed static/member method interface.
10341034 */
10351035 class MessageGroups {
1036 -
10371036 /// Initialises the list of groups (but not the groups itself if possible).
10381037 public static function init() {
10391038 static $loaded = false;
——@@ -1300,7 +1299,7 @@
13011300 * Get all enabled message groups.
13021301 * @return \array
13031302 */
1304 - public function getGroups() {
 1303+ public function getAllMessageGroups() {
13051304 if ( $this->classes === null ) {
13061305 $this->classes = array();
13071306 global $wgTranslateEC, $wgTranslateCC;
——@@ -1313,10 +1312,48 @@
13141313 $this->classes[$g->getId()] = $g;
13151314 }
13161315 }
 1316+
13171317 return $this->classes;
13181318 }
13191319
13201320 /**
 1321+ * Get message groups.
 1322+ *
 1323+ * @para $groups \array Group IDs
 1324+ * @para $groupPrefix \string Prefix for groups
 1325+ * @return \array
 1326+ */
 1327+ public function getGroups( $groups = null, $groupPrefix = null ) {
 1328+ if ( count( $groups ) ) {
 1329+ // Get groups and add groups to array
 1330+ foreach ( $groupIds as $groupId ) {
 1331+ $group = self::getGroup( $groupId );
 1332+
 1333+ if ( $group !== null ) {
 1334+ $groups[$groupId] = $group;
 1335+ } else {
 1336+ wfDebug( __METHOD__ . ": Invalid group $groupId\n" );
 1337+ }
 1338+ }
 1339+
 1340+ return $groups;
 1341+ } elseif ( $groupPrefix !== null ) {
 1342+ $allGroups = self::singleton()->getGroups();
 1343+
 1344+ // Add matching groups to groups array.
 1345+ foreach ( $allGroups as $groupId => $messageGroup ) {
 1346+ if ( strpos( $groupId, $groupPrefix ) === 0 && !$messageGroup->isMeta() ) {
 1347+ $groups[$groupId] = $messageGroup;
 1348+ }
 1349+ }
 1350+
 1351+ return $groups;
 1352+ } else {
 1353+ return self::getAllMessageGroups();
 1354+ }
 1355+ }
 1356+
 1357+ /**
13211358 * Contents on these groups changes on a whim.
13221359 * @since 2011-12-28
13231360 */
Index: trunk/extensions/Translate/Translate.php
——@@ -15,7 +15,7 @@
1616 /**
1717 * Version number used in extension credits and in other placed where needed.
1818 */
19 -define( 'TRANSLATE_VERSION', '2012-01-31' );
 19+define( 'TRANSLATE_VERSION', '2012-02-12' );
2020
2121 /**
2222 * Extension credits properties.
——@@ -226,10 +226,20 @@
227227 ),
228228 ) + $resourcePaths;
229229
 230+$wgResourceModules['ext.translate.messagewebimporter'] = array(
 231+ 'styles' => 'resources/ext.translate.messagewebimporter.css',
 232+ 'position' => 'top',
 233+) + $resourcePaths;
 234+
230235 $wgResourceModules['ext.translate.special.languagestats'] = array(
231236 'scripts' => 'resources/ext.translate.special.languagestats.js',
232237 'styles' => 'resources/ext.translate.special.languagestats.css',
233 - 'messages' => array( 'translate-langstats-expandall', 'translate-langstats-collapseall', 'translate-langstats-expand', 'translate-langstats-collapse' ),
 238+ 'messages' => array(
 239+ 'translate-langstats-expandall',
 240+ 'translate-langstats-collapseall',
 241+ 'translate-langstats-expand',
 242+ 'translate-langstats-collapse'
 243+ ),
234244 ) + $resourcePaths;
235245
236246 $wgResourceModules['ext.translate.special.pagetranslation'] = array(
Index: trunk/extensions/Translate/MediaWikiMessageChecker.php
——@@ -203,7 +203,6 @@
204204 $definition = $message->definition();
205205 $translation = $message->translation();
206206
207 -
208207 if ( in_array( strtolower( $key ), $timeList, true ) ) {
209208 $defArray = explode( ',', $definition );
210209 $traArray = explode( ',', $translation );
——@@ -212,10 +211,16 @@
213212 $defCount = count( $defArray );
214213 $traCount = count( $traArray );
215214 if ( $defCount !== $traCount ) {
 215+ global $wgLang;
 216+
216217 $warnings[$key][] = array(
217218 array( 'miscmw', $subcheck, $key, $code ),
218219 'translate-checks-format',
219 - "Parameter count is $traCount; should be $defCount", // @todo Missing i18n.
 220+ wfMessage(
 221+ 'translate-checks-parametersnotequal',
 222+ $wgLang->formatNum( $traCount ),
 223+ $wgLang->formatNum( $defCount )
 224+ )->text()
220225 );
221226 continue;
222227 }
——@@ -229,7 +234,11 @@
230235 $warnings[$key][] = array(
231236 array( 'miscmw', $subcheck, $key, $code ),
232237 'translate-checks-format',
233 - "<nowiki>$traArray[$i]</nowiki> is malformed", // @todo Missing i18n.
 238+ wfMessage(
 239+ 'translate-checks-malformed',
 240+ $defArray,
 241+ $i
 242+ )->text()
234243 );
235244 continue;
236245 }
——@@ -239,7 +248,7 @@
240249 $warnings[$key][] = array(
241250 array( 'miscmw', $subcheck, $key, $code ),
242251 'translate-checks-format',
243 - "<tt><nowiki>$traItems[1] !== $defItems[1]</nowiki></tt>",
 252+ "<tt><nowiki>$traItems[1] !== $defItems[1]</nowiki></tt>", // @todo FIXME: i18n missing.
244253 );
245254 continue;
246255 }
Index: trunk/extensions/Translate/utils/MessageWebImporter.php
——@@ -15,7 +15,6 @@
1616 * displays them in pretty way with diffs and finally executes the actions the user choices.
1717 */
1818 class MessageWebImporter {
19 -
2019 /**
2120 * @var Title
2221 */
——@@ -203,7 +202,7 @@
204203
205204 if ( $old === false ) {
206205 $name = wfMsgHtml( 'translate-manage-import-new',
207 - '<code style="font-weight:normal;">' . htmlspecialchars( $key ) . '</code>'
 206+ '<code class="mw-tmi-new">' . htmlspecialchars( $key ) . '</code>'
208207 );
209208 $text = TranslateUtils::convertWhiteSpaceToHTML( $value );
210209 $changed[] = self::makeSectionElement( $name, 'new', $text );
——@@ -268,7 +267,7 @@
269268 }
270269
271270 $name = wfMsg( 'translate-manage-import-diff',
272 - '<code style="font-weight:normal;">' . htmlspecialchars( $key ) . '</code>',
 271+ '<code class="mw-tmi-diff">' . htmlspecialchars( $key ) . '</code>',
273272 implode( ' ', $act )
274273 );
275274
——@@ -283,9 +282,8 @@
284283 $diff = array_diff( $keys, array_keys( $messages ) );
285284
286285 foreach ( $diff as $s ) {
287 - // @todo FIXME: Use CSS file.
288286 $name = wfMsgHtml( 'translate-manage-import-deleted',
289 - '<code style="font-weight:normal;">' . htmlspecialchars( $s ) . '</code>'
 287+ '<code class="mw-tmi-deleted">' . htmlspecialchars( $s ) . '</code>'
290288 );
291289 $text = TranslateUtils::convertWhiteSpaceToHTML( $collection[$s]->translation() );
292290 $changed[] = self::makeSectionElement( $name, 'deleted', $text );
Index: trunk/extensions/Translate/Translate.i18n.php
——@@ -146,6 +146,8 @@
147147 'translate-checks-pagename' => 'Namespace changed from the definition',
148148 'translate-checks-format' => 'This translation does not follow the definition or has invalid syntax: $1',
149149 'translate-checks-escape' => 'The following escapes may be accidental: <strong>$1</strong>',
 150+ 'translate-checks-parametersnotequal' => 'Parameter count is {{PLURAL:$1|$1}}; should be {{PLURAL:$2|$2}}.',
 151+ 'translate-checks-malformed' => '<nowiki>$traArray[$i]</nowiki> is malformed.',
150152 'translate-checks-fudforum-syntax' => 'Use <nowiki>$1</nowiki> instead of <nowiki>$2</nowiki> in this project.',
151153
152154 'translate-pref-nonewsletter' => 'Do not send me e-mail newsletters',
——@@ -521,13 +523,18 @@
522524 'translate-magic-cm-current' => '{{Identical|Current}}',
523525 'translate-magic-cm-comment' => '{{Identical|Comment}}',
524526 'translate-magic-cm-save' => '{{Identical|Save}}',
525 - 'translate-checks-balance' => 'This translation warning is displayed if the number of opening brackets ("[", "{", and "(") is different from the number of closing brackets ("]", "}", and ")").
526 -
 527+ 'translate-checks-balance' => 'This translation warning is displayed if the number of opening brackets ("[", "{", and "(") is different from the number of closing brackets ("]", "}", and ")"). Parameters:
527528 * Parameter $1 is a list of the unbalanced brackets, for example "\'\'\'[]: 1\'\'\'" which means that there is one missing closing square brackets.
528529 * Parameter $2 is the number of types of parentheses that are unbalanced.',
529530 'translate-checks-pagename' => 'A warning when editing a message.
530531
531532 This warning indicates that the namespace in the translation does not match the namespace appearing in the message definition (original English message).',
 533+ 'translate-checks-parametersnotequal' => 'Warning message from automated syntax check for translators. Parameters:
 534+* $1 is the number of parameters used in the source message
 535+* $2 is the number of parameters used in the translated message.',
 536+ 'translate-checks-malformed' => 'Warning message from automated syntax check for translators. Parameters:
 537+* $1 is ...
 538+* $2 is ....',
532539 'translate-pref-nonewsletter' => "Option in [[Special:Preferences]], 'Misc' tab.",
533540 'translate-pref-editassistlang' => 'Used in [[Special:Preferences]] under the {{msg-mw|prefs-editing}} tab.',
534541 'prefs-translate' => 'Caption of a section at [[Special:Preferences#prefsection-3|Special:Preferences]]',
Index: trunk/extensions/Translate/Groups.php
——@@ -520,7 +520,7 @@
521521
522522 /**
523523 * New style message group for %MediaWiki.
524 - * @todo Currently unused?
 524+ * @todo Currently unused
525525 */
526526 class MediaWikiMessageGroup extends FileBasedMessageGroup {
527527 public function mapCode( $code ) {
Index: trunk/extensions/Translate/resources/ext.translate.messagewebimporter.css
——@@ -0,0 +1,3 @@
 2+.mw-tmi-deleted .mw-tmi-diff .mw-tmi-new {
 3+ font-weight: normal;
 4+}
Property changes on: trunk/extensions/Translate/resources/ext.translate.messagewebimporter.css
___________________________________________________________________
Added: svn:eol-style
15 + native
Added: svn:keywords
26 + Id

Follow-up revisions

RevisionCommit summaryAuthorDate
r111278Fix fatal in r111272.siebrand07:00, 12 February 2012
r111279Follow-up r111272:...siebrand07:03, 12 February 2012
r111358Address some of the issues in r111272nikerabbit10:37, 13 February 2012
r111821Use numbered variables. Follows-up r111272.siebrand12:10, 18 February 2012
r111825Follow-up r111272, r111821: *facepalm*siebrand13:38, 18 February 2012
r112484Fix regression in r111272 addressed in CR of r111825.siebrand13:33, 27 February 2012

Comments

#Comment by Nikerabbit (talk | contribs)   08:13, 12 February 2012 
+		if ( count( $groups ) ) {
+			// Get groups and add groups to array
+			foreach ( $groupIds as $groupId ) {

Count will return 1 for strings, but foreach will throw a warning if it doesn't get an array. 

+	'translate-checks-malformed'          => '$traArray[$i] is malformed.',

That is not supposed to be literal, is it?

I would split the new getGroups into two methods, getGroupsById and getGroupsByPrefix since they don't share common code.

#Comment by Raymond (talk | contribs)   19:51, 12 February 2012

PHP Fatal error: Call to a member function isMeta() on a non-object in /www/w/extensions/Translate/scripts/sync-group.php on line 80

#Comment by Raymond (talk | contribs)   20:29, 12 February 2012

Export (bpmw) is broken too: 

Exporting ext-wikimediamessages
Array
(
)
#Comment by Nikerabbit (talk | contribs)   09:03, 13 February 2012

@para should be @param

#Comment by Nikerabbit (talk | contribs)   08:24, 17 February 2012

Setting back to fixme due to this issue: 

+	'translate-checks-malformed'          => '$traArray[$i] is malformed.',

That is not supposed to be literal, is it?

#Comment by Siebrand (talk | contribs)   12:12, 18 February 2012

Correct. Addresssed in r111821.

Status & tagging log