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

Reply via email to