Jarry1250 has uploaded a new change for review.
https://gerrit.wikimedia.org/r/213799
Change subject: Overhaul replaceIndicesRecursive, adding tests
......................................................................
Overhaul replaceIndicesRecursive, adding tests
Despite the fact that I wrote it, I'm not sure exactly what capabilities
this function was supposed to have. Per TDD, add tests to specify
that (in particular) recursive/multi-layer replacement is required, and
reimplement the underlying function to provide this functionality.
Note that the relatively clunky syntax is a result of the difference
Zend's and HipHop's DOM implementations.
Change-Id: Ie9759089ba76c1294fda73570e2df5a588ff2e4f
---
M SVGFile.php
M tests/phpunit/SVGFileTest.php
M tests/phpunit/SVGMessageGroupTest.php
3 files changed, 72 insertions(+), 18 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/TranslateSvg
refs/changes/99/213799/1
diff --git a/SVGFile.php b/SVGFile.php
index 8daa201..eb270a2 100644
--- a/SVGFile.php
+++ b/SVGFile.php
@@ -636,7 +636,10 @@
/**
* Recursively replaces $1, $2, etc. with text tags, if required. Text
content
- * is formalised as actual text nodes
+ * is formalised as actual text nodes. The basic approach is to jump to
recursively split
+ * on each dollar tag, working through each segment in turn.
+ *
+ * Note that because we're using appendChild(), timing matter: we work
precisely beginning to end.
*
* @param string $text The text to search for $1, $2 etc.
* @param array &$newNodes An array of DOMNodes, indexed by which $
number they represent
@@ -645,27 +648,40 @@
* @return void
*/
public static function replaceIndicesRecursive( $text, &$newNodes,
DOMNode &$parentNode, DOMDocument $document ) {
+ preg_match_all( '/\$([0-9]+)/', $text, $matches );
+
// If nothing to replace, just fire back a text node
- if ( count( $newNodes ) === 0 ) {
- if ( strlen( $text ) > 0 ) {
+ if ( count( $matches[0] ) === 0 ) {
+ if( $parentNode->nodeValue !== $text ){
$parentNode->appendChild(
$document->createTextNode( $text ) );
}
+ return;
}
- // Otherwise, loop through $1, $2, etc. replacing each
- preg_match_all( '/\$([0-9]+)/', $text, $matches );
- foreach ( $newNodes as $index => $node ) {
- // One-indexed (no $0)
- $realIndex = $index + 1;
- if ( !in_array( $realIndex, $matches[1] ) ) {
- // Sanity check
- continue;
- }
- list( $before, $after ) = preg_split( '/\$' .
$realIndex . '(?=[^0-9]|$)/', $text );
- $newNodeToProcess = $newNodes[$index];
- unset( $newNodes[$index] );
+ if( $parentNode->nodeValue == $text ){
+ // Original structure is still present -- remove it so
we have a clean slate
+ // (details are safely stashed in $text)
+ $parentNode->parentNode->replaceChild(
+ $n = $parentNode->cloneNode( false ),
+ $parentNode
+ );
+ $parentNode = $n;
+ }
+
+ // Replace first match and recurse. before-current-after order
matters.
+ $index = $matches[1][0];
+ list( $before, $after ) = preg_split( '/\$' . $index .
'(?=[^0-9]|$)/', $text );
+ if( strlen( $before ) > 0 ){
self::replaceIndicesRecursive( $before, $newNodes,
$parentNode, $document );
- $parentNode->appendChild( $newNodeToProcess );
+ }
+ if( isset( $newNodes[$index-1] ) ){
+ // Implicitly, if $9 doesn't exist, leave a blank space
+ $newNode = $newNodes[$index - 1];
+ $parentNode->appendChild( $newNode );
+ $newNode = $parentNode->lastChild;
+ self::replaceIndicesRecursive( $newNode->nodeValue,
$newNodes, $newNode, $document );
+ }
+ if( strlen( $after ) > 0 ){
self::replaceIndicesRecursive( $after, $newNodes,
$parentNode, $document );
}
}
diff --git a/tests/phpunit/SVGFileTest.php b/tests/phpunit/SVGFileTest.php
index 6b8fa58..92ddef5 100644
--- a/tests/phpunit/SVGFileTest.php
+++ b/tests/phpunit/SVGFileTest.php
@@ -443,6 +443,45 @@
$this->assertArrayEquals( $expected,
$this->svg->getFilteredTextNodes() );
}
+ public function testReplaceIndicesRecursive () {
+ $document = new DOMDocument( '1.0' );
+ $fragment = $document->createTextNode( 'foo' );
+
+ // Simple: single replacement case
+ $parent = $document->createElement( 'text' );
+ $nodes = array( $fragment->cloneNode( true ) );
+ $this->svg->replaceIndicesRecursive( '$1', $nodes, $parent,
$document );
+ $this->assertEquals( 'foo', $parent->nodeValue );
+
+ // Simple multiple replacement case
+ $parent = $document->createElement( 'text' );
+ $nodes = array( $fragment->cloneNode( true ),
$fragment->cloneNode( true ) );
+ $this->svg->replaceIndicesRecursive( '$1bar$2', $nodes,
$parent, $document );
+ $this->assertEquals( 'foobarfoo', $parent->nodeValue );
+
+ // Harder: nested.
+ $parent = $document->createElement( 'text' );
+ $tspan = $document->createElement( 'tspan' );
+ $tspan->appendChild( $document->createTextNode( 'mum$2nun' ) );
+ $tspan2 = $document->createElement( 'tspan' );
+ $tspan2->appendChild( $fragment->cloneNode( true ) );
+ $nodes = array( $tspan, $tspan2 );
+ $this->svg->replaceIndicesRecursive( 'bar$1baz', $nodes,
$parent, $document );
+ $document->appendChild( $parent );
+ $this->assertEquals( '<?xml version="1.0"?>' . "\n" .
'<text>bar<tspan>mum<tspan>foo</tspan>nun</tspan>baz</text>'."\n",
$document->saveXML() );
+
+ // Harder: check attributes are preserved
+ $document = new DOMDocument( '1.0' );
+ $parent = $document->createElement( 'text' );
+ $node = $document->createElement( 'tspan' );
+ $node->setAttribute( 'style', 'font-weight:bold' );
+ $node->appendChild( $document->createTextNode( 'foo' ) );
+ $nodes = array( $node );
+ $this->svg->replaceIndicesRecursive( 'bar$1baz', $nodes,
$parent, $document );
+ $document->appendChild( $parent );
+ $this->assertEquals( '<?xml version="1.0"?>' . "\n" .
'<text>bar<tspan style="font-weight:bold">foo</tspan>baz</text>'."\n",
$document->saveXML() );
+ }
+
public function testSwitchTranslationSetRoundtrip() {
// Functions already tested above
$current = $this->svg->getInFileTranslations();
diff --git a/tests/phpunit/SVGMessageGroupTest.php
b/tests/phpunit/SVGMessageGroupTest.php
index 40b8418..5b48273 100644
--- a/tests/phpunit/SVGMessageGroupTest.php
+++ b/tests/phpunit/SVGMessageGroupTest.php
@@ -73,8 +73,7 @@
public function testGetDescription() {
// Should be normalised to spaces
$name = str_replace( '_', ' ', self::$name );
- $expected = "[[File:$name|thumb|right|upright|275x275px]]
-<div style=\"overflow:auto; padding:2px;\">Created during testing</div>";
+ $expected = "[[File:$name|thumb|right|upright|275x275px]]\n<div
style=\"overflow:auto; padding:2px;\">Created during testing</div>";
$this->assertEquals( $expected,
$this->messageGroup->getDescription() );
}
}
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/213799
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie9759089ba76c1294fda73570e2df5a588ff2e4f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/TranslateSvg
Gerrit-Branch: master
Gerrit-Owner: Jarry1250 <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
