r111867 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r111866‎ | r111867 | r111868 >
Date:15:27, 19 February 2012
Author:siebrand
Status:resolved (Comments)
Tags:i18nreview 
Comment:
* Some refactoring for parseGettextData()
* Remove remaining pre 1.18 compat.
Modified paths:
  • /trunk/extensions/Translate/ffs/Gettext.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Translate/ffs/Gettext.php
——@@ -58,12 +58,11 @@
5959 * or use msgctxt (non-standard po-files)
6060 * @param $mangler StringMangler
6161 * @return \array
62 - * @todo Refactor method into smaller parts.
6362 */
6463 public static function parseGettextData( $data, $useCtxtAsKey = false, $mangler ) {
6564 $potmode = false;
6665
67 - // Normalise newlines, to make processing easier lates
 66+ // Normalise newlines, to make processing easier
6867 $data = str_replace( "\r\n", "\n", $data );
6968
7069 /* Delimit the file into sections, which are separated by two newlines.
——@@ -83,7 +82,9 @@
8483
8584 // Check for pot-mode by checking if the header is fuzzy
8685 $flags = self::parseFlags( $headerSection );
87 - if ( in_array( 'fuzzy', $flags, true ) ) $potmode = true;
 86+ if ( in_array( 'fuzzy', $flags, true ) ) {
 87+ $potmode = true;
 88+ }
8889 } else {
8990 throw new MWException( "Gettext file header was not found:\n\n$data" );
9091 }
——@@ -112,111 +113,139 @@
113114
114115 // Then parse the messages
115116 foreach ( $sections as $section ) {
116 - if ( trim( $section ) === '' ) {
117 - continue;
118 - }
 117+ self::parseGettextSection(
 118+ $section,
 119+ $useCtxtAsKey,
 120+ $pluralCount,
 121+ $mangler,
 122+ $messages,
 123+ $template,
 124+ $metadata,
 125+ $headers
 126+ );
 127+ }
119128
120 - /* These inactive section are of no interest to us. Multiline mode
121 - * is needed because there may be flags or other annoying stuff
122 - * before the commented out sections.
123 - */
124 - if ( preg_match( '/^#~/m', $section ) ) continue;
 129+ return array(
 130+ 'MESSAGES' => $messages,
 131+ 'TEMPLATE' => $template,
 132+ 'METADATA' => $metadata,
 133+ 'HEADERS' => $headers
 134+ );
 135+ }
125136
126 - $item = array(
127 - 'ctxt' => '',
128 - 'id' => '',
129 - 'str' => '',
130 - 'flags' => array(),
131 - 'comments' => array(),
132 - );
 137+ public static function parseGettextSection( $section, $useCtxtAsKey, $pluralCount, $manger, &$messages, &$template, &$metadata, &$headers ) {
 138+ if ( trim( $section ) === '' ) {
 139+ return false;
 140+ }
133141
134 - $match = self::expectKeyword( 'msgid', $section );
135 - if ( $match !== null ) {
136 - $item['id'] = self::formatForWiki( $match );
137 - } else {
138 - throw new MWException( "Unable to parse msgid:\n\n$section" );
139 - }
 142+ /* These inactive sections are of no interest to us. Multiline mode
 143+ * is needed because there may be flags or other annoying stuff
 144+ * before the commented out sections.
 145+ */
 146+ if ( preg_match( '/^#~/m', $section ) ) {
 147+ return false;
 148+ }
140149
141 - $match = self::expectKeyword( 'msgctxt', $section );
142 - if ( $match !== null ) {
143 - $item['ctxt'] = self::formatForWiki( $match );
144 - } elseif ( $useCtxtAsKey ) { // Invalid message
145 - $metadata['warnings'][] = "Ctxt missing for {$item['id']}";
146 - error_log( "Ctxt missing for {$item['id']}" );
147 - }
 150+ $item = array(
 151+ 'ctxt' => '',
 152+ 'id' => '',
 153+ 'str' => '',
 154+ 'flags' => array(),
 155+ 'comments' => array(),
 156+ );
148157
 158+ $match = self::expectKeyword( 'msgid', $section );
 159+ if ( $match !== null ) {
 160+ $item['id'] = self::formatForWiki( $match );
 161+ } else {
 162+ throw new MWException( "Unable to parse msgid:\n\n$section" );
 163+ }
149164
150 - $pluralMessage = false;
151 - $match = self::expectKeyword( 'msgid_plural', $section );
152 - if ( $match !== null ) {
153 - $pluralMessage = true;
154 - $plural = self::formatForWiki( $match );
155 - $item['id'] = "{{PLURAL:GETTEXT|{$item['id']}|$plural}}";
156 - }
 165+ $match = self::expectKeyword( 'msgctxt', $section );
 166+ if ( $match !== null ) {
 167+ $item['ctxt'] = self::formatForWiki( $match );
 168+ } elseif ( $useCtxtAsKey ) { // Invalid message
 169+ $metadata['warnings'][] = "Ctxt missing for {$item['id']}";
 170+ error_log( "Ctxt missing for {$item['id']}" );
 171+ }
157172
158 - if ( $pluralMessage ) {
159 - $actualForms = array();
160 - for ( $i = 0; $i < $pluralCount; $i++ ) {
161 - $match = self::expectKeyword( "msgstr\\[$i\\]", $section );
162 - if ( $match !== null ) {
163 - $actualForms[] = self::formatForWiki( $match );
164 - } else {
165 - $actualForms[] = '';
166 - error_log( "Plural $i not found, expecting total of $pluralCount for {$item['id']}" );
167 - }
168 - }
 173+ $pluralMessage = false;
 174+ $match = self::expectKeyword( 'msgid_plural', $section );
 175+ if ( $match !== null ) {
 176+ $pluralMessage = true;
 177+ $plural = self::formatForWiki( $match );
 178+ $item['id'] = "{{PLURAL:GETTEXT|{$item['id']}|$plural}}";
 179+ }
169180
170 - // Keep the translation empty if no form has translation
171 - if ( array_sum( array_map( 'strlen', $actualForms ) ) > 0 ) {
172 - $item['str'] = '{{PLURAL:GETTEXT|' . implode( '|', $actualForms ) . '}}';
173 - }
 181+ if ( $pluralMessage ) {
 182+ $pluralMessageText = self::processGettextPluralMessage( $pluralCount, $section );
 183+
 184+ // Keep the translation empty if no form has translation
 185+ if( $pluralMessageText !== '' ) {
 186+ $item['str'] = $pluralMessageText;
 187+ }
 188+ } else {
 189+ $match = self::expectKeyword( 'msgstr', $section );
 190+ if ( $match !== null ) {
 191+ $item['str'] = self::formatForWiki( $match );
174192 } else {
175 - $match = self::expectKeyword( 'msgstr', $section );
176 - if ( $match !== null ) {
177 - $item['str'] = self::formatForWiki( $match );
178 - } else {
179 - throw new MWException( "Unable to parse msgstr:\n\n$section" );
180 - }
 193+ throw new MWException( "Unable to parse msgstr:\n\n$section" );
181194 }
 195+ }
182196
183 - // Parse flags
184 - $flags = self::parseFlags( $section );
185 - foreach ( $flags as $key => $flag ) {
186 - if ( $flag === 'fuzzy' ) {
187 - $item['str'] = TRANSLATE_FUZZY . $item['str'];
188 - unset( $flags[$key] );
189 - }
 197+ // Parse flags
 198+ $flags = self::parseFlags( $section );
 199+ foreach ( $flags as $key => $flag ) {
 200+ if ( $flag === 'fuzzy' ) {
 201+ $item['str'] = TRANSLATE_FUZZY . $item['str'];
 202+ unset( $flags[$key] );
190203 }
191 - $item['flags'] = $flags;
 204+ }
 205+ $item['flags'] = $flags;
192206
193 - // Rest of the comments
194 - $matches = array();
195 - if ( preg_match_all( '/^#(.?) (.*)$/m', $section, $matches, PREG_SET_ORDER ) ) {
196 - foreach ( $matches as $match ) {
197 - if ( $match[1] !== ',' && strpos( $match[1], '[Wiki]' ) !== 0 ) {
198 - $item['comments'][$match[1]][] = $match[2];
199 - }
 207+ // Rest of the comments
 208+ $matches = array();
 209+ if ( preg_match_all( '/^#(.?) (.*)$/m', $section, $matches, PREG_SET_ORDER ) ) {
 210+ foreach ( $matches as $match ) {
 211+ if ( $match[1] !== ',' && strpos( $match[1], '[Wiki]' ) !== 0 ) {
 212+ $item['comments'][$match[1]][] = $match[2];
200213 }
201214 }
 215+ }
202216
203 - if ( $useCtxtAsKey ) {
204 - $key = $item['ctxt'];
 217+ if ( $useCtxtAsKey ) {
 218+ $key = $item['ctxt'];
 219+ } else {
 220+ $key = self::generateKeyFromItem( $item );
 221+ }
 222+
 223+ $key = $mangler->mangle( $key );
 224+
 225+ $messages[$key] = $potmode ? $item['id'] : $item['str'];
 226+ $template[$key] = $item;
 227+
 228+ return true;
 229+ }
 230+
 231+ public static function processGettextPluralMessage( $pluralCount, $section ) {
 232+ $actualForms = array();
 233+
 234+ for ( $i = 0; $i < $pluralCount; $i++ ) {
 235+ $match = self::expectKeyword( "msgstr\\[$i\\]", $section );
 236+
 237+ if ( $match !== null ) {
 238+ $actualForms[] = self::formatForWiki( $match );
205239 } else {
206 - $key = self::generateKeyFromItem( $item );
 240+ $actualForms[] = '';
 241+ error_log( "Plural $i not found, expecting total of $pluralCount for {$item['id']}" );
207242 }
 243+ }
208244
209 - $key = $mangler->mangle( $key );
210 -
211 - $messages[$key] = $potmode ? $item['id'] : $item['str'];
212 - $template[$key] = $item;
 245+ if ( array_sum( array_map( 'strlen', $actualForms ) ) > 0 ) {
 246+ return '{{PLURAL:GETTEXT|' . implode( '|', $actualForms ) . '}}';
 247+ } else {
 248+ return '';
213249 }
214 -
215 - return array(
216 - 'MESSAGES' => $messages,
217 - 'TEMPLATE' => $template,
218 - 'METADATA' => $metadata,
219 - 'HEADERS' => $headers
220 - );
221250 }
222251
223252 public static function parseFlags( $section ) {
——@@ -346,14 +375,8 @@
347376 $output = trim( $output ) . "\n";
348377
349378 // @todo portal is twn specific
350 - // BC for MW <1.18
351 - if ( method_exists( 'Title', 'getCanonicalUrl' ) ) {
352 - $portal = Title::makeTitle( NS_PORTAL, $code )->getCanonicalUrl();
353 - $server = $wgCanonicalServer;
354 - } else {
355 - $portal = Title::makeTitle( NS_PORTAL, $code )->getFullUrl();
356 - $server = $wgServer;
357 - }
 379+ $portal = Title::makeTitle( NS_PORTAL, $code )->getCanonicalUrl();
 380+ $server = $wgCanonicalServer;
358381
359382 $specs = isset( $template['HEADERS'] ) ? $template['HEADERS'] : array();
360383
——@@ -576,5 +599,4 @@
577600
578601 return $splitPlurals;
579602 }
580 -
581603 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r111889Fix typo in r111867.siebrand20:58, 19 February 2012
r111913Fix Undefined variable: potmode in Gettext.php on line 224siebrand07:45, 20 February 2012
r111914$headers not used in this method, ping r111867nikerabbit07:51, 20 February 2012

Comments

#Comment by Raymond (talk | contribs)   20:30, 19 February 2012

PHP Fatal error: Call to a member function mangle() on a non-object in /www/w/extensions/Translate/ffs/Gettext.php on line 222

#Comment by Nikerabbit (talk | contribs)   07:17, 20 February 2012

Please do not combine unrelated stuff in one commit.

#Comment by Nikerabbit (talk | contribs)   13:02, 27 February 2012

Good enough for now. I'd combine the four output parameters passed by reference into one parameter.

Status & tagging log