Nikerabbit has submitted this change and it was merged. Change subject: PageTranslation: do not add line breaks to inline usage of <translate> tags ......................................................................
PageTranslation: do not add line breaks to inline usage of <translate> tags This causes problems for example with headings. This should also make the source a bit more readable in places when line breaks are not forced. Bug: T150188 Change-Id: Ia54091ba857e8d513ea302a4b80342b5f5b97b22 --- M tag/TPSection.php M tag/TranslatablePage.php A tests/phpunit/TPSectionTest.php A tests/phpunit/TranslatablePageTest.php M tests/phpunit/pagetranslation/Inline.ptsource M tests/phpunit/pagetranslation/Whitespace.ptsource 6 files changed, 166 insertions(+), 20 deletions(-) Approvals: Amire80: Looks good to me, approved jenkins-bot: Verified diff --git a/tag/TPSection.php b/tag/TPSection.php index 105dd98..747812a 100644 --- a/tag/TPSection.php +++ b/tag/TPSection.php @@ -4,7 +4,6 @@ * * @file * @author Niklas Laxström - * @copyright Copyright © 2009-2013 Niklas Laxström * @license GPL-2.0+ */ @@ -40,6 +39,20 @@ public $oldText; /** + * @var bool Whether this section is inline section. + * E.g. "Something <translate>foo</translate> bar". + */ + protected $inline = false; + + public function setIsInline( $value ) { + $this->inline = (bool)$value; + } + + public function isInline( $value ) { + return $this->inline; + } + + /** * Returns section text unmodified. * @return string Wikitext. */ @@ -70,6 +83,7 @@ /** * Returns the section text with updated or added section marker. + * * @return string Wikitext. */ public function getMarkedText() { @@ -82,7 +96,11 @@ $text = preg_replace( $re, $rep, $this->text, 1, $count ); if ( $count === 0 ) { - $text = $header . "\n" . $this->text; + if ( $this->inline ) { + $text = $header . ' ' . $this->text; + } else { + $text = $header . "\n" . $this->text; + } } return $text; diff --git a/tag/TranslatablePage.php b/tag/TranslatablePage.php index c0236b9..3b9a5bb 100644 --- a/tag/TranslatablePage.php +++ b/tag/TranslatablePage.php @@ -286,7 +286,7 @@ $tagPlaceHolders = array(); while ( true ) { - $re = '~(<translate>)\s*(.*?)(</translate>)~s'; + $re = '~(<translate>)(.*?)(</translate>)~s'; $matches = array(); $ok = preg_match_all( $re, $text, $matches, PREG_OFFSET_CAPTURE ); @@ -316,10 +316,11 @@ $sectiontext = self::unArmourNowiki( $nowiki, $sectiontext ); - $ret = $this->sectionise( $sections, $sectiontext ); + $parse = self::sectionise( $sectiontext ); + $sections += $parse['sections']; $tagPlaceHolders[$ph] = - self::index_replace( $contents, $ret, $start, $end ); + self::index_replace( $contents, $parse['template'], $start, $end ); } $prettyTemplate = $text; @@ -400,27 +401,36 @@ /** * Splits the content marked with \<translate> tags into sections, which * are separated with with two or more newlines. Extra whitespace is captured - * in the template and not included in the sections. - * @param array $sections Array of placeholder => TPSection. + * in the template and is not included in the sections. + * * @param string $text Contents of one pair of \<translate> tags. - * @return string Template with placeholders for sections, which itself are added to $sections. + * @return array Contains a template and array of unparsed sections. */ - protected function sectionise( &$sections, $text ) { + public static function sectionise( $text ) { $flags = PREG_SPLIT_NO_EMPTY | PREG_SPLIT_DELIM_CAPTURE; - $parts = preg_split( '~(\s*\n\n\s*|\s*$)~', $text, -1, $flags ); + $parts = preg_split( '~(^\s*|\s*\n\n\s*|\s*$)~', $text, -1, $flags ); + + $inline = preg_match( '~\n~', $text ) === 0; $template = ''; + $sections = []; + foreach ( $parts as $_ ) { if ( trim( $_ ) === '' ) { $template .= $_; } else { $ph = TranslateUtils::getPlaceholder(); - $sections[$ph] = $this->shakeSection( $_ ); + $tpsection = self::shakeSection( $_ ); + $tpsection->setIsInline( $inline ); + $sections[$ph] = $tpsection; $template .= $ph; } } - return $template; + return array( + 'template' => $template, + 'sections' => $sections, + ); } /** @@ -435,7 +445,7 @@ * @throws TPException * @return TPSection */ - protected function shakeSection( $content ) { + public static function shakeSection( $content ) { $re = '~<!--T:(.*?)-->~'; $matches = array(); $count = preg_match_all( $re, $content, $matches, PREG_SET_ORDER ); @@ -452,7 +462,7 @@ // Currently handle only these two standard places. // Is this too strict? - $rer1 = '~^<!--T:(.*?)-->\n~'; // Normal sections + $rer1 = '~^<!--T:(.*?)-->( |\n)~'; // Normal sections $rer2 = '~\s*<!--T:(.*?)-->$~m'; // Sections with title $content = preg_replace( $rer1, '', $content ); $content = preg_replace( $rer2, '', $content ); diff --git a/tests/phpunit/TPSectionTest.php b/tests/phpunit/TPSectionTest.php new file mode 100644 index 0000000..49cc75a --- /dev/null +++ b/tests/phpunit/TPSectionTest.php @@ -0,0 +1,65 @@ +<?php +/** + * Unit tests for class TPSection + * + * @author Niklas Laxström + * @license GPL-2.0+ + * @file + */ + +/** + * Unit tests for class TPSection + * @ingroup PageTranslation + */ +class TPSectionTest extends PHPUnit_Framework_TestCase { + /** + * @dataProvider providerTestGetMarkedText + */ + public function testGetMarkedText( $name, $text, $inline, $expected ) { + $section = new TPSection(); + $section->name = $name; + $section->text = $text; + $section->setIsInline( $inline ); + + $output = $section->getMarkedText(); + + $this->assertEquals( $expected, $output ); + } + + public static function providerTestGetMarkedText() { + $cases = array(); + + // Inline syntaxs + $cases[] = array( + 'name', + 'Hello', + true, + '<!--T:name--> Hello', + ); + + // Normal syntax + $cases[] = array( + 'name', + 'Hello', + false, + "<!--T:name-->\nHello", + ); + + // Inline should not matter for headings, which have special syntax, but test both values + $cases[] = array( + 'name', + '== Hello ==', + true, + '== Hello == <!--T:name-->', + ); + + $cases[] = array( + 'name', + '====== Hello ======', + false, + '====== Hello ====== <!--T:name-->', + ); + + return $cases; + } +} diff --git a/tests/phpunit/TranslatablePageTest.php b/tests/phpunit/TranslatablePageTest.php new file mode 100644 index 0000000..0403ed5 --- /dev/null +++ b/tests/phpunit/TranslatablePageTest.php @@ -0,0 +1,56 @@ +<?php +/** + * Unit tests for class TPSection + * + * @author Niklas Laxström + * @license GPL-2.0+ + * @file + */ + +/** + * Unit tests for class TPSection + * @ingroup PageTranslation + */ +class TranslatablePageTest extends PHPUnit_Framework_TestCase { + /** + * @dataProvider provideTestSectionise + */ + public function testSectionise( $input, $pattern, $comment ) { + $result = TranslatablePage::sectionise( $input ); + $pattern = addcslashes( $pattern, '~' ); + $this->assertRegExp( "~^$pattern$~", $result['template'], $comment ); + } + + public static function provideTestSectionise() { + // Ugly implicit assumption + $ph = "\x7fUNIQ[a-z0-9]{8,16}-\d+"; + + $cases = array(); + + $cases[] = array( + 'Hello', + "$ph", + 'No surrounding whitespace', + ); + + $cases[] = array( + "\nHello", + "\n$ph", + 'With surrounding whitespace', + ); + + $cases[] = array( + "\nHello world\n\nBunny\n", + "\n$ph\n\n$ph\n", + 'Splitting at one empty line', + ); + + $cases[] = array( + "First\n\n\n\n\nSecond\n\nThird", + "$ph\n\n\n\n\n$ph\n\n$ph", + 'Splitting with multiple empty lines', + ); + + return $cases; + } +} diff --git a/tests/phpunit/pagetranslation/Inline.ptsource b/tests/phpunit/pagetranslation/Inline.ptsource index 85c5545..78c4146 100644 --- a/tests/phpunit/pagetranslation/Inline.ptsource +++ b/tests/phpunit/pagetranslation/Inline.ptsource @@ -1,2 +1 @@ -We had a nice <translate><!--T:-1--> -day</translate> today. +We had a nice <translate><!--T:-1--> day</translate> today. diff --git a/tests/phpunit/pagetranslation/Whitespace.ptsource b/tests/phpunit/pagetranslation/Whitespace.ptsource index 5bc38c0..e677341 100644 --- a/tests/phpunit/pagetranslation/Whitespace.ptsource +++ b/tests/phpunit/pagetranslation/Whitespace.ptsource @@ -15,7 +15,5 @@ <!--T:-1--> This line is prefixed with a space, with two trailing spaces </translate> -<translate><!--T:-1--> -line1</translate> -<translate><!--T:-1--> -line2</translate> +<translate><!--T:-1--> line1</translate> +<translate><!--T:-1--> line2</translate> -- To view, visit https://gerrit.wikimedia.org/r/320571 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia54091ba857e8d513ea302a4b80342b5f5b97b22 Gerrit-PatchSet: 4 Gerrit-Project: mediawiki/extensions/Translate Gerrit-Branch: master Gerrit-Owner: Nikerabbit <niklas.laxst...@gmail.com> Gerrit-Reviewer: Amire80 <amir.ahar...@mail.huji.ac.il> Gerrit-Reviewer: Nemo bis <federicol...@tiscali.it> Gerrit-Reviewer: Nikerabbit <niklas.laxst...@gmail.com> Gerrit-Reviewer: Subramanya Sastry <ssas...@wikimedia.org> Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits