From 0d7816b55d07f51c3347d539350f445bff8c64ba Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Tue, 16 Oct 2012 17:10:54 +1300 Subject: [PATCH] BUG Fixed issue with Deprecation failing to extract the module from a stacktrace, especially on non-unix systems API Added Convert::nl2os function to normalise end of line characters across systems with tests BUG Fixed i18n unit tests in non-unix systems constantly failing BUG Fixed problems with HTMLCleaner tests failing in non-unix systems --- core/Convert.php | 11 +++++++ core/HTMLCleaner.php | 4 ++- dev/Deprecation.php | 4 +-- i18n/i18nTextCollector.php | 3 +- tests/core/ConvertTest.php | 48 +++++++++++++++++++++++++++- tests/core/HTMLCleanerTest.php | 8 ++--- tests/i18n/i18nTextCollectorTest.php | 2 +- 7 files changed, 70 insertions(+), 10 deletions(-) diff --git a/core/Convert.php b/core/Convert.php index 205a95040..3df0b37b7 100644 --- a/core/Convert.php +++ b/core/Convert.php @@ -317,4 +317,15 @@ class Convert { $f = URLSegmentFilter::create(); return $f->filter($title); } + + /** + * Normalises newline sequences to conform to (an) OS specific format. + * @param string $data Text containing potentially mixed formats of newline + * sequences including \r, \r\n, \n, or unicode newline characters + * @param string $nl The newline sequence to normalise to. Defaults to that + * specified by the current OS + */ + public static function nl2os($data, $nl = PHP_EOL) { + return preg_replace('~\R~u', $nl, $data); + } } diff --git a/core/HTMLCleaner.php b/core/HTMLCleaner.php index a37275442..53cf02210 100644 --- a/core/HTMLCleaner.php +++ b/core/HTMLCleaner.php @@ -93,6 +93,8 @@ class TidyHTMLCleaner extends HTMLCleaner { public function cleanHTML($content) { $tidy = new tidy(); $output = $tidy->repairString($content, $this->config); - return $output; + + // Clean leading/trailing whitespace + return preg_replace('/(^\s+)|(\s+$)/', '', $output); } } diff --git a/dev/Deprecation.php b/dev/Deprecation.php index 98f7403a0..d673cb28f 100644 --- a/dev/Deprecation.php +++ b/dev/Deprecation.php @@ -86,11 +86,11 @@ class Deprecation { protected static function get_calling_module_from_trace($backtrace) { if (!isset($backtrace[1]['file'])) return; - $callingfile = $backtrace[1]['file']; + $callingfile = realpath($backtrace[1]['file']); global $manifest; foreach ($manifest->getModules() as $name => $path) { - if (strpos($callingfile, $path) === 0) { + if (strpos($callingfile, realpath($path)) === 0) { return $name; } } diff --git a/i18n/i18nTextCollector.php b/i18n/i18nTextCollector.php index e7ffa3ba5..40da3f8e6 100644 --- a/i18n/i18nTextCollector.php +++ b/i18n/i18nTextCollector.php @@ -522,7 +522,8 @@ class i18nTextCollector_Writer_Php implements i18nTextCollector_Writer { $php .= (count($entitySpec) == 1) ? var_export($entitySpec[0], true) : var_export($entitySpec, true); $php .= ";$eol"; - return $php; + // Normalise linebreaks due to fix var_export output + return Convert::nl2os($php, $eol); } } diff --git a/tests/core/ConvertTest.php b/tests/core/ConvertTest.php index 2e7027f13..14132b38d 100644 --- a/tests/core/ConvertTest.php +++ b/tests/core/ConvertTest.php @@ -139,5 +139,51 @@ class ConvertTest extends SapphireTest { $this->assertEquals('foos-bar-2', Convert::raw2url('foo\'s [bar] (2)')); URLSegmentFilter::$default_allow_multibyte = $orig; } - + + /** + * Helper function for comparing characters with significant whitespaces + * @param type $expected + * @param type $actual + */ + protected function assertEqualsQuoted($expected, $actual) { + $message = sprintf( + "Expected \"%s\" but given \"%s\"", + addcslashes($expected, "\r\n"), + addcslashes($actual, "\r\n") + ); + $this->assertEquals($expected, $actual, $message); + } + + public function testNL2OS() { + + foreach(array("\r\n", "\r", "\n") as $nl) { + + // Base case: no action + $this->assertEqualsQuoted( + "Base case", + Convert::nl2os("Base case", $nl) + ); + + // Mixed formats + $this->assertEqualsQuoted( + "Test{$nl}Text{$nl}Is{$nl}{$nl}Here{$nl}.", + Convert::nl2os("Test\rText\r\nIs\n\rHere\r\n.", $nl) + ); + + // Test that multiple runs are non-destructive + $expected = "Test{$nl}Text{$nl}Is{$nl}{$nl}Here{$nl}."; + $this->assertEqualsQuoted( + $expected, + Convert::nl2os($expected, $nl) + ); + + // Check repeated sequence behaves correctly + $expected = "{$nl}{$nl}{$nl}{$nl}{$nl}{$nl}{$nl}{$nl}"; + $input = "\r\r\n\r\r\n\n\n\n\r"; + $this->assertEqualsQuoted( + $expected, + Convert::nl2os($input, $nl) + ); + } + } } diff --git a/tests/core/HTMLCleanerTest.php b/tests/core/HTMLCleanerTest.php index 8bd91c109..e2f6667f1 100644 --- a/tests/core/HTMLCleanerTest.php +++ b/tests/core/HTMLCleanerTest.php @@ -11,13 +11,13 @@ class HTMLCleanerTest extends SapphireTest { if ($cleaner) { $this->assertEquals( - $cleaner->cleanHTML('

wrong nesting

' . "\n"), - '

wrong nesting

' . "\n", + $cleaner->cleanHTML('

wrong nesting

'), + '

wrong nesting

', "HTML cleaned properly" ); $this->assertEquals( - $cleaner->cleanHTML('

unclosed paragraph' . "\n"), - '

unclosed paragraph

' . "\n", + $cleaner->cleanHTML('

unclosed paragraph'), + '

unclosed paragraph

', "HTML cleaned properly" ); } else { diff --git a/tests/i18n/i18nTextCollectorTest.php b/tests/i18n/i18nTextCollectorTest.php index f9b59ab59..7218ac7ee 100644 --- a/tests/i18n/i18nTextCollectorTest.php +++ b/tests/i18n/i18nTextCollectorTest.php @@ -413,7 +413,7 @@ de: OtherEntityName: 'Other Text' YAML; - $this->assertEquals($yaml, $writer->getYaml($entities, 'de')); + $this->assertEquals($yaml, Convert::nl2os($writer->getYaml($entities, 'de'))); } public function testCollectFromIncludedTemplates() {