Nikerabbit has uploaded a new change for review.
https://gerrit.wikimedia.org/r/319571
Change subject: Implement plural aware message content comparison
......................................................................
Implement plural aware message content comparison
When someone uses an inline plural in a messages that is exported
in a format that uses CLDR plural format without support for inline
syntax, on next sync that message is shown as changed. Since the
translation is in fact equal and we would like to keep the inline
version in the wiki, amend the comparison function to compare some
kind of canonical representation instead of plain string equality.
I moved this from ExternalMessageSourceStateComparator to FFS since
this only applies currently to few specific formats, and that there
might be other things that need similar handling in the future. For
example the fuzzy handling could also be moved to FFS classes.
Change-Id: I6417ce19332ee90346a00d5b658d45b39c3a691b
---
M ffs/FFS.php
M ffs/JsonFFS.php
M ffs/SimpleFFS.php
M ffs/YamlFFS.php
M resources/css/ext.translate.messagetable.less
M tests/phpunit/utils/ArrayFlattenerTest.php
M utils/ArrayFlattener.php
M utils/ExternalMessageSourceStateComparator.php
8 files changed, 175 insertions(+), 29 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Translate
refs/changes/71/319571/1
diff --git a/ffs/FFS.php b/ffs/FFS.php
index 3c425e9..ccfc3c3 100644
--- a/ffs/FFS.php
+++ b/ffs/FFS.php
@@ -76,6 +76,16 @@
public function supportsFuzzy();
/**
+ * Checks whether two strings are equal. Sometimes same content might
+ * have multiple representations. The main case are inline plurals,
+ * which in some formats require expansion at export time.
+ *
+ * @return bool
+ * @since 2016.11
+ */
+ public function isContentEqual();
+
+ /**
* Return the commonly used file extensions for these formats.
* Include the dot.
* @return string[]
diff --git a/ffs/JsonFFS.php b/ffs/JsonFFS.php
index b94a7ba..425879b 100644
--- a/ffs/JsonFFS.php
+++ b/ffs/JsonFFS.php
@@ -24,6 +24,14 @@
return is_array( FormatJson::decode( $data, /*as array*/true )
);
}
+ /**
+ * @param $group FileBasedMessageGroup
+ */
+ public function __construct( FileBasedMessageGroup $group ) {
+ parent::__construct( $group );
+ $this->flattener = $this->getFlattener();
+ }
+
public function getFileExtensions() {
return array( '.json' );
}
@@ -48,12 +56,8 @@
unset( $messages['@metadata'] );
- if ( isset( $this->extra['nestingSeparator'] ) ) {
- $parseCLDRPlurals = isset(
$this->extra['parseCLDRPlurals'] ) ?
- $this->extra['parseCLDRPlurals'] : false;
- $flattener = new ArrayFlattener(
$this->extra['nestingSeparator'],
- $parseCLDRPlurals );
- $messages = $flattener->flatten( $messages );
+ if ( $this->flattener ) {
+ $messages = $this->flattener->flatten( $messages );
}
$messages = $this->group->getMangler()->mangle( $messages );
@@ -113,17 +117,33 @@
return '';
}
- if ( isset( $this->extra['nestingSeparator'] ) ) {
- $parseCLDRPlurals = isset(
$this->extra['parseCLDRPlurals'] ) ?
- $this->extra['parseCLDRPlurals'] : false;
- $flattener = new ArrayFlattener(
$this->extra['nestingSeparator'],
- $parseCLDRPlurals );
- $messages = $flattener->unflatten( $messages );
+ if ( $this->flattener ) {
+ $messages = $this->flattener->unflatten( $messages );
}
return FormatJson::encode( $messages, "\t", FormatJson::ALL_OK
) . "\n";
}
+ protected function getFlattener() {
+ if ( !isset( $this->extra['nestingSeparator'] ) ) {
+ return null;
+ }
+
+ $parseCLDRPlurals = isset( $this->extra['parseCLDRPlurals'] ) ?
+ $this->extra['parseCLDRPlurals'] : false;
+ $flattener = new ArrayFlattener(
$this->extra['nestingSeparator'], $parseCLDRPlurals );
+
+ return $flattener;
+ }
+
+ public function isContentEqual( $a, $b ) {
+ if ( $this->flattener ) {
+ return $this->flattener->compareContent( $a, $b );
+ } else {
+ return parent::isContentEqual( $a, $b );
+ }
+ }
+
public static function getExtraSchema() {
$schema = array(
'root' => array(
diff --git a/ffs/SimpleFFS.php b/ffs/SimpleFFS.php
index 444cd2e..fefef3e 100644
--- a/ffs/SimpleFFS.php
+++ b/ffs/SimpleFFS.php
@@ -351,4 +351,8 @@
return $data;
}
+
+ public function isContentEqual( $a, $b ) {
+ return $a === $b;
+ }
}
diff --git a/ffs/YamlFFS.php b/ffs/YamlFFS.php
index 7cfce2b..bf45002 100644
--- a/ffs/YamlFFS.php
+++ b/ffs/YamlFFS.php
@@ -171,6 +171,10 @@
return $this->flattener->unflatten( $messages );
}
+ public function isContentEqual( $a, $b ) {
+ return $this->flattener->compareContent( $a, $b );
+ }
+
public static function getExtraSchema() {
$schema = array(
'root' => array(
diff --git a/resources/css/ext.translate.messagetable.less
b/resources/css/ext.translate.messagetable.less
index 97ae577..5810d8b 100644
--- a/resources/css/ext.translate.messagetable.less
+++ b/resources/css/ext.translate.messagetable.less
@@ -7,6 +7,20 @@
max-width: 800px;
}
+.grid.ext-translate-container .row {
+ min-width: 300px !important;
+}
+
+@media screen and (max-width: 600px) {
+ .grid.ext-translate-container .tux-messagelist .tux-list-message {
+ width: 100%;
+ }
+
+ .tux-list-status, .tux-list-edit {
+ display: none;
+ }
+}
+
.tux-message {
height: auto;
cursor: pointer;
diff --git a/tests/phpunit/utils/ArrayFlattenerTest.php
b/tests/phpunit/utils/ArrayFlattenerTest.php
index 84984ff..630f03d 100644
--- a/tests/phpunit/utils/ArrayFlattenerTest.php
+++ b/tests/phpunit/utils/ArrayFlattenerTest.php
@@ -7,7 +7,7 @@
* @license GPL-2.0+
*/
-class ArrayFlattenerTest extends MediaWikiTestCase {
+class ArrayFlattenerTest extends PHPUnit_Framework_TestCase {
/**
* @dataProvider provideTestFlatten
*/
@@ -163,4 +163,87 @@
return $cases;
}
+ /**
+ * @dataProvider provideMatchingValues
+ */
+ public function testCompareTrue( $input1, $input2 ) {
+ $flattener = new ArrayFlattener( '.', true );
+
+ $this->assertTrue(
+ $flattener->compareContent( $input1, $input2,
$flattener )
+ );
+ }
+
+ /**
+ * @dataProvider provideNonMatchingValues
+ */
+ public function testCompareFalse( $input1, $input2 ) {
+ $flattener = new ArrayFlattener( '.', true );
+
+ $this->assertfalse(
+ $flattener->compareContent( $input1, $input2,
$flattener )
+ );
+ }
+
+ public static function provideMatchingValues() {
+ $cases = array();
+
+ // We include some non-plural data to ensure it is processed
correctly
+ $cases[] = array(
+ 'a',
+ 'a'
+ );
+
+ $cases[] = array(
+ '{{PLURAL|one=cat|cats}}',
+ '{{PLURAL|one=cat|cats}}',
+ );
+
+ $cases[] = array(
+ 'Give me {{PLURAL|one=a cat|cats}}',
+ '{{PLURAL|one=Give me a cat|Give me cats}}',
+ );
+
+ // Order should not matter
+ $cases[] = array(
+ '{{PLURAL|one=Give me a cat|Give me cats}}',
+ 'Give me {{PLURAL|one=a cat|cats}}',
+ );
+
+ // Multiple inlines
+ $cases[] = array(
+ 'Test {{PLURAL|one=one|other}} and
{{PLURAL|one=one|other}} and {{PLURAL|one=one|other}}!',
+ '{{PLURAL|one=Test one and one and one|Test other and
other and other}}!',
+ );
+
+ // Lots of keys
+ $cases[] = array(
+ 'Is
{{PLURAL|zero=zero|one=one|two=two|few=few|many=many|other}}',
+ '{{PLURAL|zero=Is zero|one=Is one|two=Is two|few=Is
few|many=Is many|Is other}}',
+ );
+
+ return $cases;
+ }
+
+ public static function provideNonMatchingValues() {
+ $cases = array();
+
+ $cases[] = array(
+ 'a',
+ 'b'
+ );
+
+ $cases[] = array(
+ '{{PLURAL|one=cat|cats}}',
+ '{{PLURAL|one=dog|dogs}}',
+ );
+
+ // Different set of keys
+ $cases[] = array(
+ 'Is {{PLURAL|zero=zero|one=one|two=two|few=few|other}}',
+ '{{PLURAL|zero=Is zero|two=Is two|few=Is few|many=Is
many|Is other}}',
+ );
+
+ return $cases;
+ }
}
diff --git a/utils/ArrayFlattener.php b/utils/ArrayFlattener.php
index c65081c..7c96691 100644
--- a/utils/ArrayFlattener.php
+++ b/utils/ArrayFlattener.php
@@ -274,4 +274,27 @@
return $alts;
}
+ /**
+ * Compares two strings for equal content, taking PLURAL expansion into
account.
+ *
+ * @param string $a
+ * @param string $b
+ * @return bool Whether two strings are equal
+ */
+ public function compareContent( $a, $b ) {
+ if ( !$this->parseCLDRPlurals ) {
+ return $a === $b;
+ }
+
+ $a2 = $this->unflattenCLDRPlurals( 'prefix', $a );
+ $b2 = $this->unflattenCLDRPlurals( 'prefix', $b );
+
+ // Fall back to regular comparison if parsing fails.
+ if ( $a2 === false || $b2 === false ) {
+ return $a === $b;
+ }
+
+ // Require key-value pairs to match, but ignore order and types
(all should be strings).
+ return $a2 == $b2;
+ }
}
diff --git a/utils/ExternalMessageSourceStateComparator.php
b/utils/ExternalMessageSourceStateComparator.php
index 56de79d..9efe6a0 100644
--- a/utils/ExternalMessageSourceStateComparator.php
+++ b/utils/ExternalMessageSourceStateComparator.php
@@ -149,6 +149,7 @@
$wikiMessage = $wiki[$key];
$wikiContent = $wikiMessage->translation();
+ // @todo: Fuzzy checking can also be moved to
$ffs->isContentEqual();
// If FFS doesn't support it, ignore fuzziness as
difference
$wikiContent = str_replace( TRANSLATE_FUZZY, '',
$wikiContent );
@@ -157,7 +158,7 @@
$wikiContent = TRANSLATE_FUZZY . $wikiContent;
}
- if ( self::compareContent( $sourceContent, $wikiContent
) ) {
+ if ( $ffs->isContentEqual( $sourceContent, $wikiContent
) ) {
// File and wiki stage agree, nothing to do
continue;
}
@@ -173,8 +174,8 @@
* Hence we check that source === cache &&
cache !== wiki
* and if so we skip this string. */
if (
- !self::compareContent( $wikiContent,
$cacheContent ) &&
- self::compareContent( $sourceContent,
$cacheContent )
+ !$ffs->isContentEqual( $wikiContent,
$cacheContent ) &&
+ $ffs->isContentEqual( $sourceContent,
$cacheContent )
) {
continue;
}
@@ -215,18 +216,5 @@
'key' => $key,
'content' => $content,
);
- }
-
- /**
- * Compares two strings.
- * @todo Ignore changes in different way inlined plurals.
- * @todo Handle fuzzy state changes if FFS supports it.
- *
- * @param string $a
- * @param string $b
- * @return bool Whether two strings are equal
- */
- protected static function compareContent( $a, $b ) {
- return $a === $b;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/319571
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6417ce19332ee90346a00d5b658d45b39c3a691b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Translate
Gerrit-Branch: master
Gerrit-Owner: Nikerabbit <niklas.laxst...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
