r66820 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r66819‎ | r66820 | r66821 >
Date:09:56, 24 May 2010
Author:nikerabbit
Status:ok
Tags:
Comment:
Improvements to page translation feature

* Allow to define tags statically to avoid queries
* More tests and more strict parsing
* Use EditFilter hook to produce nice error messages on
attempted save instead of edit conflict screen
Modified paths:
  • /trunk/extensions/Translate/PageTranslation.i18n.php (modified) (history)
  • /trunk/extensions/Translate/Translate.php (modified) (history)
  • /trunk/extensions/Translate/scripts/pagetranslation-test-parser.php (modified) (history)
  • /trunk/extensions/Translate/tag/PageTranslationHooks.php (modified) (history)
  • /trunk/extensions/Translate/tag/TPParse.php (modified) (history)
  • /trunk/extensions/Translate/tag/TranslatablePage.php (modified) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/FailEmptySection.ptfile (added) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/FailMultipleSectionMarkers.pttarget (deleted) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/FailNotAtomic.pttarget (deleted) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/FailSectionMarkerPlace.ptfile (added) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/SimpleWithMarker.ptfile (added) (history)
  • /trunk/extensions/Translate/tests/pagetranslation/SimpleWithMarker.pttarget (added) (history)

Diff [purge]

Index: trunk/extensions/Translate/scripts/pagetranslation-test-parser.php
——@@ -37,11 +37,20 @@
3838
3939 $pattern = realpath( "$testDirectory" ) . "/$pagename";
4040
 41+ $failureExpected = strpos( $pagename, 'Fail' ) === 0;
 42+
4143 try {
4244 $parse = $translatablePage->getParse();
 45+ if ( $failureExpected ) {
 46+ $target = $parse->getTranslationPageText( new TestMessageCollection( "foo" ) );
 47+ $this->output( "Testfile $filename should have failed... see $pattern.pttarget.fail" );
 48+ file_put_contents( "$pattern.pttarget.fail", $target );
 49+ }
4350 } catch ( TPException $e ) {
44 - $this->output( "Testfile $filename failed to parse... see $pattern.ptfile.fail" );
45 - file_put_contents( "$pattern.ptfile.fail", $e->getMessage() );
 51+ if ( !$failureExpected ) {
 52+ $this->output( "Testfile $filename failed to parse... see $pattern.ptfile.fail" );
 53+ file_put_contents( "$pattern.ptfile.fail", $e->getMessage() );
 54+ }
4655 continue;
4756 }
4857
Index: trunk/extensions/Translate/tests/pagetranslation/FailNotAtomic.pttarget
——@@ -1 +0,0 @@
2 -This file should not parse
\ No newline at end of file
Index: trunk/extensions/Translate/tests/pagetranslation/FailMultipleSectionMarkers.pttarget
——@@ -1 +0,0 @@
2 -This parse should fail
Index: trunk/extensions/Translate/tests/pagetranslation/SimpleWithMarker.pttarget
——@@ -0,0 +1 @@
 2+A cat sleeps.
Index: trunk/extensions/Translate/tests/pagetranslation/FailSectionMarkerPlace.ptfile
——@@ -0,0 +1,3 @@
 2+<translate>
 3+Once upon time <!--T:1--> there was a planet called meow.
 4+</translate>
\ No newline at end of file
Index: trunk/extensions/Translate/tests/pagetranslation/FailEmptySection.ptfile
——@@ -0,0 +1,4 @@
 2+<translate>
 3+<!--T:1-->
 4+
 5+</translate>
\ No newline at end of file
Index: trunk/extensions/Translate/tests/pagetranslation/SimpleWithMarker.ptfile
——@@ -0,0 +1,4 @@
 2+<translate>
 3+<!--T:1-->
 4+A cat sleeps.
 5+</translate>
Index: trunk/extensions/Translate/tag/TranslatablePage.php
——@@ -2,6 +2,10 @@
33
44 /**
55 * Class to parse translatable wiki pages.
 6+ *
 7+ * @author Niklas Laxström
 8+ * @copyright Copyright © 2009-2010 Niklas Laxström
 9+ * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
610 */
711 class TranslatablePage {
812 /**
——@@ -142,7 +146,6 @@
143147 $re = '~(<translate>)\s*(.*?)(</translate>)~s';
144148 $matches = array();
145149 $ok = preg_match_all( $re, $text, $matches, PREG_OFFSET_CAPTURE );
146 - if ( $ok === false ) return array( 'pt-error', 'tag', $text );
147150 if ( $ok === 0 ) break; // No matches
148151
149152 // Do-placehold for the whole stuff
——@@ -159,22 +162,28 @@
160163 $len = strlen( $matches[2][0][0] ); // len of the content
161164 $end = $start + $len;
162165
163 - $ret = $this->sectionise( $sections, substr( $contents, $start, $len ) );
 166+ $sectiontext = substr( $contents, $start, $len );
164167
 168+ if ( strpos( $sectiontext, '<translate>' ) !== false ) {
 169+ throw new TPException( array( 'pt-parse-nested', $sectiontext ) );
 170+ }
 171+
 172+ $ret = $this->sectionise( $sections, $sectiontext );
 173+
165174 $tagPlaceHolders[$ph] =
166175 self::index_replace( $contents, $ret, $start, $end );
167176 }
168177
 178+ $prettyTemplate = $text;
 179+ foreach ( $tagPlaceHolders as $ph => $value ) {
 180+ $prettyTemplate = str_replace( $ph, '[...]', $prettyTemplate );
 181+ }
169182 if ( strpos( $text, '<translate>' ) !== false ) {
170 - $text = str_replace( "\n", '\n', $text );
171 - throw new TPException( array( 'pt-parse-open', $text ) );
 183+ throw new TPException( array( 'pt-parse-open', $prettyTemplate ) );
 184+ } elseif ( strpos( $text, '</translate>' ) !== false ) {
 185+ throw new TPException( array( 'pt-parse-close', $prettyTemplate ) );
172186 }
173187
174 - if ( strpos( $text, '</translate>' ) !== false ) {
175 - $text = str_replace( "\n", '\n', $text );
176 - throw new TPException( array( 'pt-parse-close', $text ) );
177 - }
178 -
179188 foreach ( $tagPlaceHolders as $ph => $value ) {
180189 $text = str_replace( $ph, $value, $text );
181190 }
——@@ -242,7 +251,9 @@
243252 $re = '~<!--T:(.*?)-->~';
244253 $matches = array();
245254 $count = preg_match_all( $re, $content, $matches, PREG_SET_ORDER );
246 - if ( $count > 1 ) throw new TPException( array( 'pt-shake-multiple', $content ) );
 255+ if ( $count > 1 ) {
 256+ throw new TPException( array( 'pt-shake-multiple', $content ) );
 257+ }
247258
248259 $section = new TPSection;
249260 if ( $count === 1 ) {
——@@ -250,10 +261,17 @@
251262 list( /*full*/, $id ) = $match;
252263 $section->id = $id;
253264
254 - $rer1 = '~^\s*<!--T:(.*?)-->\s*~'; // Normal sections
255 - $rer2 = '~\s*<!--T:(.*?)-->~'; // Sections with title
 265+ // Currently handle only these two standard places.
 266+ // Is this too strict?
 267+ $rer1 = '~^<!--T:(.*?)-->\n~'; // Normal sections
 268+ $rer2 = '~\s*<!--T:(.*?)-->\n~'; // Sections with title
256269 $content = preg_replace( $rer1, '', $content );
257270 $content = preg_replace( $rer2, '', $content );
 271+ if ( preg_match( $re, $content ) === 1 ) {
 272+ throw new TPException( array( 'pt-shake-position', $content ) );
 273+ } elseif ( trim( $content ) === '' ) {
 274+ throw new TPException( array( 'pt-shake-empty', $id ) );
 275+ }
258276 }
259277 } else {
260278 // New section
——@@ -453,6 +471,11 @@
454472 throw new MWException( "Invalid tag $tag requested" );
455473 }
456474
 475+ global $wgTranslateStaticTags;
 476+ if ( is_array($wgTranslateStaticTags) ) {
 477+ return $wgTranslateStaticTags[$tag];
 478+ }
 479+
457480 // Simple static cache
458481 static $tagcache = array();
459482
——@@ -482,14 +505,23 @@
483506 return $page;
484507 }
485508
486 - public static function changeTitleText( Title $title, $text ) {
 509+ protected static function changeTitleText( Title $title, $text ) {
487510 return Title::makeTitleSafe( $title->getNamespace(), $text );
488511 }
489512 }
490513
 514+/**
 515+ * Class to signal translatable page parser exceptions.
 516+ */
491517 class TPException extends MWException {
 518+ protected $msg = null;
492519 public function __construct( $msg ) {
 520+ $this->msg = $msg;
493521 parent::__construct( call_user_func_array( 'wfMsg', $msg ) );
494522 }
 523+
 524+ public function getMsg() {
 525+ return $this->msg;
 526+ }
495527 }
496528
Index: trunk/extensions/Translate/tag/TPParse.php
——@@ -4,7 +4,7 @@
55 * extracted sections and a template.
66 *
77 * @author Niklas Laxström
8 - * @copyright Copyright © 2009 Niklas Laxström
 8+ * @copyright Copyright © 2009-2010 Niklas Laxström
99 * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
1010 */
1111 class TPParse {
Index: trunk/extensions/Translate/tag/PageTranslationHooks.php
——@@ -237,21 +237,32 @@
238238 FOO;
239239 }
240240
241 - // When attempting to save
 241+ // To display nice error for editpage
 242+ public static function tpSyntaxCheckForEditPage( $editpage, $text, $section, &$error, $summary ) {
 243+ if ( strpos( $text, '<translate>' ) === false ) return true;
 244+ $page = TranslatablePage::newFromText( $editpage->mTitle, $text );
 245+ try {
 246+ $page->getParse();
 247+ } catch ( TPException $e ) {
 248+ $error .= Html::rawElement( 'div', array( 'class' => 'error' ), $e->getMessage() );
 249+ }
 250+ return true;
 251+ }
 252+
 253+ /**
 254+ * When attempting to save, last resort. Edit page would only display
 255+ * edit conflict if there wasn't tpSyntaxCheckForEditPage
 256+ */
242257 public static function tpSyntaxCheck( $article, $user, $text, $summary,
243258 $minor, $_, $_, $flags, $status ) {
244259 // Quick escape on normal pages
245 - if ( strpos( $text, '</translate>' ) === false ) return true;
 260+ if ( strpos( $text, '<translate>' ) === false ) return true;
246261
247262 $page = TranslatablePage::newFromText( $article->getTitle(), $text );
248263 try {
249 - /* This does not catch all problems yet,
250 - * like markup spanning between sections. */
251264 $page->getParse();
252265 } catch ( TPException $e ) {
253 - // FIXME: throws "PHP Notice: Undefined variable: ret" when <translate>/</translate> is uneven
254 - // and an 'edit conflict'.
255 - call_user_func_array( array( $status, 'fatal' ), $ret );
 266+ call_user_func_array( array( $status, 'fatal' ), $e->getMsg() );
256267 return false;
257268 }
258269
——@@ -281,7 +292,6 @@
282293 if ( $group === null ) {
283294 // No group means that the page is currently not
284295 // registered to any page translation message groups
285 - wfLoadExtensionMessages( 'PageTranslation' );
286296 $result = array( 'tpt-unknown-page' );
287297 return false;
288298 }
——@@ -297,8 +307,6 @@
298308
299309 if ( $page->getMarkedTag() ) {
300310 list( , $code ) = TranslateUtils::figureMessage( $title->getText() );
301 - wfLoadExtensionMessages( 'PageTranslation' );
302 - // FIXME: Core chokes on this, passing an array as first parameter to wfMsgNoTrans
303311 $result = array(
304312 'tpt-target-page',
305313 $page->getTitle()->getPrefixedText(),
Index: trunk/extensions/Translate/Translate.php
——@@ -77,6 +77,7 @@
7878
7979 $wgEnablePageTranslation = false;
8080 $wgPageTranslationNamespace = 1198;
 81+$wgTranslateStaticTags = false;
8182
8283 $wgJobClasses['RenderJob'] = 'RenderJob';
8384 $wgAvailableRights[] = 'translate';
——@@ -277,7 +278,9 @@
278279
279280 // Check syntax for <translate>
280281 $wgHooks['ArticleSave'][] = 'PageTranslationHooks::tpSyntaxCheck';
 282+ $wgHooks['EditFilter'][] = 'PageTranslationHooks::tpSyntaxCheckForEditPage';
281283
 284+
282285 // Add transtag to page props for discovery
283286 $wgHooks['ArticleSaveComplete'][] = 'PageTranslationHooks::addTranstag';
284287
Index: trunk/extensions/Translate/PageTranslation.i18n.php
——@@ -74,11 +74,19 @@
7575
7676 'tpt-download-page' => 'Export page with translations',
7777
 78+ 'pt-parse-open' => 'Unbalanced &lt;translate> tag.
 79+Translation template: <pre>$1</pre>',
 80+ 'pt-parse-close' => 'Unbalanced &lt;/translate> tag.
 81+Translation template: <pre>$1</pre>',
 82+ 'pt-parse-nested' => 'Nested &lt;translate> sections are not allowed.
 83+Tag text: <pre>$1</pre>',
7884 'pt-shake-multiple' => 'Multiple section markers for one section.
79 -Section text:
80 -$1
81 -',
 85+Section text: <pre>$1</pre>',
 86+ 'pt-shake-position' => 'Section markers in unexpected position.
 87+Section text: <pre>$1</pre>',
 88+ 'pt-shake-empty' => 'Empty section for marker $1.',
8289
 90+
8391 );
8492
8593 /** Message documentation (Message documentation)

Status & tagging log