content: Refactor normalization of line endings code
authorKunal Mehta <legoktm@member.fsf.org>
Tue, 16 Aug 2016 21:58:15 +0000 (14:58 -0700)
committerKunal Mehta <legoktm@member.fsf.org>
Tue, 23 Aug 2016 18:09:59 +0000 (11:09 -0700)
The code that normalizes line endings ("\r\n" and "\r" to "\n") and
trims trailing whitespace is buried in Parser::preSaveTransform(), and
was duplicated to TextContent in 96b6afb31dfcff, as non-wikitext content
models should still be normalizing line endings.

This splits the duplicated code into
TextContent::normalizeLineEndings(), and utilize it in the Parser.
Additionally, expand the documentation of
TextContent::preSaveTransform() to document that subclasses should make
sure they normalize line endings during the PST stage.

And remove a useless rtrim() call from WikitextContent that did nothing.

Change-Id: I9094c671d4bbd23d75436f8f1d682d6dd6e6d2fc

includes/content/JsonContent.php
includes/content/TextContent.php
includes/content/WikitextContent.php
includes/parser/Parser.php
tests/phpunit/includes/content/TextContentTest.php

index 40d9277..14c8182 100644 (file)
@@ -86,7 +86,7 @@ class JsonContent extends TextContent {
                        return $this;
                }
 
-               return new static( $this->beautifyJSON() );
+               return new static( self::normalizeLineEndings( $this->beautifyJSON() ) );
        }
 
        /**
index de650b9..7bb4def 100644 (file)
@@ -147,9 +147,28 @@ class TextContent extends AbstractContent {
                }
        }
 
+       /**
+        * Do a "\r\n" -> "\n" and "\r" -> "\n" transformation
+        * as well as trim trailing whitespace
+        *
+        * This was formerly part of Parser::preSaveTransform, but
+        * for non-wikitext content models they probably still want
+        * to normalize line endings without all of the other PST
+        * changes.
+        *
+        * @since 1.28
+        * @param $text
+        * @return string
+        */
+       public static function normalizeLineEndings( $text ) {
+               return str_replace( [ "\r\n", "\r" ], "\n", rtrim( $text ) );
+       }
+
        /**
         * Returns a Content object with pre-save transformations applied.
-        * This implementation just trims trailing whitespace and normalizes newlines.
+        *
+        * At a minimum, subclasses should make sure to call TextContent::normalizeLineEndings()
+        * either directly or part of Parser::preSaveTransform().
         *
         * @param Title $title
         * @param User $user
@@ -159,8 +178,7 @@ class TextContent extends AbstractContent {
         */
        public function preSaveTransform( Title $title, User $user, ParserOptions $popts ) {
                $text = $this->getNativeData();
-               $pst = rtrim( $text );
-               $pst = str_replace( [ "\r\n", "\r" ], "\n", $pst );
+               $pst = self::normalizeLineEndings( $text );
 
                return ( $text === $pst ) ? $this : new static( $pst, $this->getModel() );
        }
index a63819d..9296728 100644 (file)
@@ -138,7 +138,6 @@ class WikitextContent extends TextContent {
 
                $text = $this->getNativeData();
                $pst = $wgParser->preSaveTransform( $text, $title, $user, $popts );
-               rtrim( $pst );
 
                return ( $text === $pst ) ? $this : new static( $pst );
        }
index 38eb621..b116bd4 100644 (file)
@@ -4364,7 +4364,11 @@ class Parser {
                $this->startParse( $title, $options, self::OT_WIKI, $clearState );
                $this->setUser( $user );
 
-               $text = str_replace( [ "\r\n", "\r" ], "\n", $text );
+               // We still normalize line endings for backwards-compatibility
+               // with other code that just calls PST, but this should already
+               // be handled in TextContent subclasses
+               $text = TextContent::normalizeLineEndings( $text );
+
                if ( $options->getPreSaveTransform() ) {
                        $text = $this->pstPass2( $text, $user );
                }
@@ -4442,9 +4446,6 @@ class Parser {
                        $text = preg_replace( $p2, '[[\\1]]', $text );
                }
 
-               # Trim trailing whitespace
-               $text = rtrim( $text );
-
                return $text;
        }
 
index b4ae765..b290f8f 100644 (file)
@@ -459,4 +459,30 @@ class TextContentTest extends MediaWikiLangTestCase {
                        $this->assertEquals( $expectedNative, $converted->getNativeData() );
                }
        }
+
+       /**
+        * @covers TextContent::normalizeLineEndings
+        * @dataProvider provideNormalizeLineEndings
+        */
+       public function testNormalizeLineEndings( $input, $expected ) {
+               $this->assertEquals( $expected, TextContent::normalizeLineEndings( $input ) );
+       }
+
+       public static function provideNormalizeLineEndings() {
+               return [
+                       [
+                               "Foo\r\nbar",
+                               "Foo\nbar"
+                       ],
+                       [
+                               "Foo\rbar",
+                               "Foo\nbar"
+                       ],
+                       [
+                               "Foobar\n  ",
+                               "Foobar"
+                       ]
+               ];
+       }
+
 }