jenkins-bot has submitted this change and it was merged.
Change subject: (bug 56684) Use HTML formatters in diffs.
......................................................................
(bug 56684) Use HTML formatters in diffs.
Using HTML formatters in diffs allows us to nicely visualize
complex DataValues like Quantities or Geo-Coordinates.
This introduces "HTML for diffs" as a new output format.
Change-Id: Ie852fce9fbb19c58beb21b88e9af16dcb84ff0d5
---
M lib/includes/ClaimDifferenceVisualizer.php
M lib/includes/DiffOpValueFormatter.php
M lib/includes/formatters/SnakFormatter.php
M lib/includes/formatters/WikibaseSnakFormatterBuilders.php
M lib/includes/formatters/WikibaseValueFormatterBuilders.php
M lib/tests/phpunit/claim/ClaimDifferenceVisualizerTest.php
A lib/tests/phpunit/claim/DiffOpValueFormatterTest.php
M lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php
M lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
M repo/includes/EntityContentDiffView.php
M repo/includes/actions/EditEntityAction.php
11 files changed, 215 insertions(+), 98 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/ClaimDifferenceVisualizer.php
b/lib/includes/ClaimDifferenceVisualizer.php
index e5cbee0..f024902 100644
--- a/lib/includes/ClaimDifferenceVisualizer.php
+++ b/lib/includes/ClaimDifferenceVisualizer.php
@@ -7,9 +7,10 @@
use Diff\ListDiffer;
use Diff\Diff;
use InvalidArgumentException;
+use Message;
use RuntimeException;
+use ValueFormatters\ValueFormatter;
use ValueParsers\FormattingException;
-use Wikibase\Lib\EntityIdLabelFormatter;
use Wikibase\Lib\SnakFormatter;
/**
@@ -21,13 +22,14 @@
* @author Tobias Gritschacher < tobias.gritschac...@wikimedia.de >
* @author Katie Filbert < aude.w...@gmail.com >
* @author Adam Shorland
+ * @author Daniel Kinzler
*/
class ClaimDifferenceVisualizer {
/**
* @since 0.5
*
- * @var EntityIdLabelFormatter
+ * @var ValueFormatter
*/
private $propertyIdFormatter;
@@ -50,17 +52,20 @@
*
* @since 0.4
*
- * @param EntityIdLabelFormatter $propertyIdFormatter
- * @param SnakFormatter $snakFormatter
+ * @param ValueFormatter $propertyIdFormatter Formatter for IDs, must
generate HTML.
+ * @param SnakFormatter $snakFormatter Formatter for Snaks, must
generate HTML.
+ * @param $langCode
*
* @throws InvalidArgumentException
*/
- public function __construct( EntityIdLabelFormatter
$propertyIdFormatter,
+ public function __construct( ValueFormatter $propertyIdFormatter,
SnakFormatter $snakFormatter, $langCode
) {
- if ( $snakFormatter->getFormat() !==
SnakFormatter::FORMAT_PLAIN ) {
+ if ( $snakFormatter->getFormat() !== SnakFormatter::FORMAT_HTML
+ && $snakFormatter->getFormat() !==
SnakFormatter::FORMAT_HTML_DIFF ) {
+
throw new InvalidArgumentException(
- 'Expected $snakFormatter to generate plain
text, not '
+ 'Expected $snakFormatter to generate html, not '
. $snakFormatter->getFormat() );
}
@@ -178,7 +183,7 @@
/**
* @param Snak|null $snak
*
- * @return string
+ * @return string HTML
*/
protected function formatSnak( $snak ) {
if( $snak === null ){
@@ -194,7 +199,7 @@
/**
* @param EntityId
*
- * @return string
+ * @return string HTML
*/
protected function formatPropertyId( EntityId $id ) {
try {
@@ -211,7 +216,7 @@
*
* @param SnakList $snakList
*
- * @return string[]
+ * @return string[] A list if HTML strings
*/
protected function getSnakListValues( SnakList $snakList ) {
$values = array();
@@ -229,6 +234,11 @@
return $values;
}
+ /**
+ * @param DiffOpChange $snakChange
+ *
+ * @return string HTML
+ */
protected function getSnakHeaderFromDiffOp( DiffOpChange $snakChange ){
if( $snakChange->getNewValue() === null ){
return $this->getSnakHeader( $snakChange->getOldValue()
);
@@ -244,12 +254,12 @@
*
* @param Snak $snak
*
- * @return string
+ * @return string HTML
*/
protected function getSnakHeader( Snak $snak ) {
$propertyId = $snak->getPropertyId();
$propertyLabel = $this->formatPropertyId( $propertyId );
- $headerText = wfMessage( 'wikibase-entity-property'
)->inLanguage( $this->langCode )
+ $headerText = wfMessage( 'wikibase-entity-property'
)->inLanguage( $this->langCode )->parse()
. ' / ' . $propertyLabel;
return $headerText;
@@ -262,12 +272,12 @@
*
* @param Diff $changes
* @param Claim $claim
- * @param string $breadCrumb
+ * @param Message $breadCrumb
*
* @return string
* @throws RuntimeException
*/
- protected function visualizeSnakListChanges( Diff $changes, Claim
$claim, $breadCrumb ) {
+ protected function visualizeSnakListChanges( Diff $changes, Claim
$claim, Message $breadCrumb ) {
$html = '';
$claimMainSnak = $claim->getMainSnak();
@@ -289,7 +299,7 @@
}
$valueFormatter = new DiffOpValueFormatter(
- $claimHeader . ' / ' . $breadCrumb,
+ $claimHeader . ' / ' . $breadCrumb->parse(),
$oldVal,
$newVal
);
@@ -346,7 +356,7 @@
}
$valueFormatter = new DiffOpValueFormatter(
- $claimHeader . ' / ' . wfMessage(
'wikibase-diffview-qualifier' )->inLanguage( $this->langCode ),
+ $claimHeader . ' / ' . wfMessage(
'wikibase-diffview-qualifier' )->inLanguage( $this->langCode )->parse(),
$oldVal,
$newVal
);
diff --git a/lib/includes/DiffOpValueFormatter.php
b/lib/includes/DiffOpValueFormatter.php
index 0f8dcb8..d427269 100644
--- a/lib/includes/DiffOpValueFormatter.php
+++ b/lib/includes/DiffOpValueFormatter.php
@@ -6,31 +6,14 @@
use Diff;
/**
- * Class for formatting diffs, @todo might be renamed or something....
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
+ * Class for generating diff rows for a given set of old and new values.
*
* @since 0.4
- *
- * @file
- * @ingroup WikibaseLib
*
* @licence GNU GPL v2+
* @author Tobias Gritschacher < tobias.gritschac...@wikimedia.de >
* @author Katie Filbert < aude.w...@gmail.com >
+ * @author Daniel Kinzler
*/
class DiffOpValueFormatter {
@@ -44,14 +27,14 @@
/**
* @since 0.4
*
- * @var string|string[]
+ * @var string|string[]|null
*/
protected $oldValues;
/**
* @since 0.4
*
- * @var string|string[]
+ * @var string|string[]|null
*/
protected $newValues;
@@ -60,9 +43,9 @@
*
* @since 0.4
*
- * @param string $name
- * @param string|string[] $oldValues
- * @param string|string[] $newValues
+ * @param string $name HTML of name
+ * @param string|string[]|null $oldValues HTML of old value(s)
+ * @param string|string[]|null $newValues HTML of new value(s)
*/
public function __construct( $name, $oldValues, $newValues ) {
$this->name = $name;
@@ -75,15 +58,15 @@
*
* @since 0.4
*
- * @return string
+ * @return string HTML
*/
protected function generateHeaderHtml() {
$oldHeader = is_array( $this->oldValues ) || is_string(
$this->oldValues ) ? $this->name : '';
$newHeader = is_array( $this->newValues ) || is_string(
$this->newValues ) ? $this->name : '';
$html = Html::openElement( 'tr' );
- $html .= Html::element( 'td', array( 'colspan'=>'2', 'class' =>
'diff-lineno' ), $oldHeader );
- $html .= Html::element( 'td', array( 'colspan'=>'2', 'class' =>
'diff-lineno' ), $newHeader );
+ $html .= Html::rawElement( 'td', array( 'colspan'=>'2', 'class'
=> 'diff-lineno' ), $oldHeader );
+ $html .= Html::rawElement( 'td', array( 'colspan'=>'2', 'class'
=> 'diff-lineno' ), $newHeader );
$html .= Html::closeElement( 'tr' );
return $html;
@@ -94,7 +77,7 @@
*
* @since 0.4
*
- * @return string
+ * @return string HTML
*/
protected function generateChangeOpHtml() {
$html = Html::openElement( 'tr' );
@@ -102,12 +85,12 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-deletedline' ),
Html::rawElement( 'div', array(),
Html::rawElement( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->generateSafeValueHtml(
$this->oldValues ) ) ) );
+ $this->generateValueHtml(
$this->oldValues ) ) ) );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-marker' ), '+' );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-addedline' ),
Html::rawElement( 'div', array(),
Html::rawElement( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->generateSafeValueHtml(
$this->newValues ) ) ) );
+ $this->generateValueHtml(
$this->newValues ) ) ) );
$html .= Html::closeElement( 'tr' );
$html .= Html::closeElement( 'tr' );
@@ -119,7 +102,7 @@
*
* @since 0.4
*
- * @return string
+ * @return string HTML
*/
protected function generateAddOpHtml() {
$html = Html::openElement( 'tr' );
@@ -128,7 +111,7 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-addedline' ),
Html::rawElement( 'div', array(),
Html::rawElement( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->generateSafeValueHtml(
$this->newValues ) )
+ $this->generateValueHtml(
$this->newValues ) )
)
);
$html .= Html::closeElement( 'tr' );
@@ -141,7 +124,7 @@
*
* @since 0.4
*
- * @return string
+ * @return string HTML
*/
protected function generateRemoveOpHtml() {
$html = Html::openElement( 'tr' );
@@ -149,7 +132,7 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-deletedline' ),
Html::rawElement( 'div', array(),
Html::rawElement( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->generateSafeValueHtml(
$this->oldValues ) ) ) );
+ $this->generateValueHtml(
$this->oldValues ) ) ) );
$html .= Html::rawElement( 'td', array( 'colspan'=>'2' ),
' ' );
$html .= Html::closeElement( 'tr' );
@@ -157,24 +140,23 @@
}
/**
- * Generates safe HTML from a given value or array of values
+ * Generates HTML from a given value or array of values
*
* @since 0.4
*
- * @param string|string[] $values
+ * @param string|string[] $values HTML
*
- * @return string
+ * @return string HTML
*/
- protected function generateSafeValueHtml( $values ) {
- if ( is_string( $values ) ) {
- return Html::Element( 'span', array(), $values );
- }
+ protected function generateValueHtml( $values ) {
+ $values = (array)$values;
+
$html = '';
foreach ( $values as $value ) {
if ( $html !== '' ) {
$html .= Html::rawElement( 'br', array(), '' );
}
- $html .= Html::Element( 'span', array(), $value );
+ $html .= Html::rawElement( 'span', array(), $value );
}
return $html;
@@ -185,7 +167,7 @@
*
* @since 0.4
*
- * @return string
+ * @return string HTML
*/
public function generateHtml() {
$html = '';
diff --git a/lib/includes/formatters/SnakFormatter.php
b/lib/includes/formatters/SnakFormatter.php
index 1e73a5d..9b9ce54 100644
--- a/lib/includes/formatters/SnakFormatter.php
+++ b/lib/includes/formatters/SnakFormatter.php
@@ -16,6 +16,7 @@
const FORMAT_WIKI = 'text/x-wiki';
const FORMAT_HTML = 'text/html';
const FORMAT_HTML_WIDGET = 'text/html; disposition=widget';
+ const FORMAT_HTML_DIFF = 'text/html; disposition=diff';
const FORMAT_JSON = 'application/json';
/**
diff --git a/lib/includes/formatters/WikibaseSnakFormatterBuilders.php
b/lib/includes/formatters/WikibaseSnakFormatterBuilders.php
index 4966f0f..3375869 100644
--- a/lib/includes/formatters/WikibaseSnakFormatterBuilders.php
+++ b/lib/includes/formatters/WikibaseSnakFormatterBuilders.php
@@ -2,12 +2,8 @@
namespace Wikibase\Lib;
-use Language;
use ValueFormatters\FormatterOptions;
use ValueFormatters\ValueFormatter;
-use Wikibase\Item;
-use Wikibase\LanguageFallbackChainFactory;
-use Wikibase\LanguageWithConversion;
/**
* Defines the snak formatters supported by Wikibase.
@@ -58,6 +54,7 @@
SnakFormatter::FORMAT_PLAIN =>
$buildDispatchingSnakFormatter,
SnakFormatter::FORMAT_HTML =>
$buildDispatchingSnakFormatter,
SnakFormatter::FORMAT_HTML_WIDGET =>
$buildDispatchingSnakFormatter,
+ SnakFormatter::FORMAT_HTML_DIFF =>
$buildDispatchingSnakFormatter,
);
return $types;
diff --git a/lib/includes/formatters/WikibaseValueFormatterBuilders.php
b/lib/includes/formatters/WikibaseValueFormatterBuilders.php
index 3019f8f..b5b4ff7 100644
--- a/lib/includes/formatters/WikibaseValueFormatterBuilders.php
+++ b/lib/includes/formatters/WikibaseValueFormatterBuilders.php
@@ -89,6 +89,11 @@
// Falls back to HTML display formatters.
SnakFormatter::FORMAT_HTML_WIDGET => array(
),
+
+ // Formatters to use for HTML in diffs.
+ // Falls back to HTML display formatters.
+ SnakFormatter::FORMAT_HTML_DIFF => array(
+ ),
);
/**
@@ -270,6 +275,9 @@
case SnakFormatter::FORMAT_HTML_WIDGET:
$formatters = $this->getWidgetFormatters(
$options );
break;
+ case SnakFormatter::FORMAT_HTML_DIFF:
+ $formatters = $this->getDiffFormatters(
$options );
+ break;
default:
throw new \InvalidArgumentException(
'Unsupported format: ' . $format );
}
@@ -361,6 +369,30 @@
return $widgetFormatters;
}
+
+ /**
+ * Returns a full set of formatters for generating HTML for use in
diffs.
+ * If there are formatters defined for HTML that are not defined for
diffs,
+ * the HTML formatters are used.
+ *
+ * @param FormatterOptions $options
+ * @param string[] $skip A list of types to be skipped. Useful when the
caller already has
+ * formatters for some types.
+ *
+ * @return ValueFormatter[] A map from prefixed type IDs to
ValueFormatter instances.
+ */
+ public function getDiffFormatters( FormatterOptions $options, array
$skip = array() ) {
+ $diffFormatters = $this->buildDefinedFormatters(
SnakFormatter::FORMAT_HTML_DIFF, $options );
+ $htmlFormatters = $this->getHtmlFormatters( $options,
array_merge( $skip, array_keys( $diffFormatters ) ) );
+
+ $diffFormatters = array_merge(
+ $diffFormatters,
+ $htmlFormatters
+ );
+
+ return $diffFormatters;
+ }
+
/**
* Instantiates the formatters defined for the given format in
* WikibaseValueFormatterBuilders::$valueFormatterSpecs.
diff --git a/lib/tests/phpunit/claim/ClaimDifferenceVisualizerTest.php
b/lib/tests/phpunit/claim/ClaimDifferenceVisualizerTest.php
index cb7e342..a08b2cf 100644
--- a/lib/tests/phpunit/claim/ClaimDifferenceVisualizerTest.php
+++ b/lib/tests/phpunit/claim/ClaimDifferenceVisualizerTest.php
@@ -11,7 +11,6 @@
use Wikibase\ClaimDifference;
use Wikibase\ClaimDifferenceVisualizer;
use Wikibase\DataModel\Entity\PropertyId;
-use Wikibase\Lib\EntityIdLabelFormatter;
use Wikibase\Lib\SnakFormatter;
use Wikibase\PropertyNoValueSnak;
use Wikibase\PropertySomeValueSnak;
@@ -33,7 +32,7 @@
*/
class ClaimDifferenceVisualizerTest extends \MediaWikiTestCase {
- public function newSnakFormatter( $format = SnakFormatter::FORMAT_PLAIN
){
+ public function newSnakFormatter( $format = SnakFormatter::FORMAT_HTML
){
$instance = $this->getMock( 'Wikibase\Lib\SnakFormatter' );
$instance->expects( $this->atLeastOnce() )
->method( 'getFormat' )
@@ -43,7 +42,7 @@
->will( $this->returnValue( true ) );
$instance->expects( $this->any() )
->method( 'formatSnak' )
- ->will( $this->returnValue( 'SNAK' ) );
+ ->will( $this->returnValue( '<i>SNAK</i>' ) );
return $instance;
}
@@ -55,7 +54,7 @@
$instance->expects( $this->any() )
->method( 'format' )
- ->will( $this->returnValue( 'PID' ) );
+ ->will( $this->returnValue( '<a>PID</a>' ) );
return $instance;
}
@@ -100,11 +99,11 @@
)
),
new Claim( new PropertyValueSnak( new
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
- '<tr><td colspan="2"
class="diff-lineno">property / PID</td><td colspan="2"
class="diff-lineno">property / PID</td></tr>'.
+ '<tr><td colspan="2"
class="diff-lineno">property / <a>PID</a></td><td colspan="2"
class="diff-lineno">property / <a>PID</a></td></tr>'.
'<tr><td class="diff-marker">-</td><td
class="diff-deletedline">'.
- '<div><del class="diffchange
diffchange-inline"><span>SNAK</span></del></div></td>'.
+ '<div><del class="diffchange
diffchange-inline"><span><i>SNAK</i></span></del></div></td>'.
'<td class="diff-marker">+</td><td
class="diff-addedline">'.
- '<div><ins class="diffchange
diffchange-inline"><span>SNAK</span></ins></div></td></tr></tr>'
+ '<div><ins class="diffchange
diffchange-inline"><span><i>SNAK</i></span></ins></div></td></tr></tr>'
),
//2 +qualifiers
array(
@@ -115,9 +114,9 @@
) )
),
new Claim( new PropertyValueSnak( new
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
- '<tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property / PID /
qualifier</td></tr>'.
+ '<tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property /
<a>PID</a> / qualifier</td></tr>'.
'<tr><td colspan="2"> </td><td
class="diff-marker">+</td><td class="diff-addedline">'.
- '<div><ins class="diffchange
diffchange-inline"><span>PID: SNAK</span></ins></div></td></tr>'
+ '<div><ins class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div></td></tr>'
),
//3 +references
array(
@@ -129,9 +128,9 @@
) )
),
new Claim( new PropertyValueSnak( new
PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
- '<tr><td colspan="2"
class="diff-lineno">property / PID / reference</td><td colspan="2"
class="diff-lineno"></td></tr>'.
+ '<tr><td colspan="2"
class="diff-lineno">property / <a>PID</a> / reference</td><td colspan="2"
class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td
class="diff-deletedline">'.
- '<div><del class="diffchange
diffchange-inline"><span>PID: SNAK</span></del></div></td><td
colspan="2"> </td></tr>'
+ '<div><del class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div></td><td
colspan="2"> </td></tr>'
),
//4 ranks
//TODO no diff is currently created for RANKS,
Implement this!
@@ -154,19 +153,19 @@
public function testVisualizeClaimChange( $difference, $baseClaim,
$expectedHtml ){
$visualizer = $this->newClaimDifferenceVisualizer();
$html = $visualizer->visualizeClaimChange( $difference,
$baseClaim );
- $this->assertEquals( $expectedHtml, $html );
+ $this->assertHtmlEquals( $expectedHtml, $html );
}
public function testVisualizeNewClaim(){
- $expect = '<tr><td colspan="2" class="diff-lineno"></td><td
colspan="2" class="diff-lineno">property / PID</td></tr>'.
+ $expect = '<tr><td colspan="2" class="diff-lineno"></td><td
colspan="2" class="diff-lineno">property / <a>PID</a></td></tr>'.
'<tr><td colspan="2"> </td><td
class="diff-marker">+</td><td class="diff-addedline">'.
- '<div><ins class="diffchange
diffchange-inline"><span>SNAK</span></ins></div>'.
- '</td></tr><tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property / PID /
qualifier</td></tr>'.
+ '<div><ins class="diffchange
diffchange-inline"><span><i>SNAK</i></span></ins></div>'.
+ '</td></tr><tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property /
<a>PID</a> / qualifier</td></tr>'.
'<tr><td colspan="2"> </td><td
class="diff-marker">+</td><td class="diff-addedline">'.
- '<div><ins class="diffchange
diffchange-inline"><span>PID: SNAK</span></ins></div>'.
- '</td></tr><tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property / PID /
reference</td></tr>'.
+ '<div><ins class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div>'.
+ '</td></tr><tr><td colspan="2"
class="diff-lineno"></td><td colspan="2" class="diff-lineno">property /
<a>PID</a> / reference</td></tr>'.
'<tr><td colspan="2"> </td><td
class="diff-marker">+</td><td class="diff-addedline">'.
- '<div><ins class="diffchange
diffchange-inline"><span>PID: SNAK</span></ins></div></td></tr>';
+ '<div><ins class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></ins></div></td></tr>';
$visualizer = $this->newClaimDifferenceVisualizer();
$claim = new Statement(
@@ -179,19 +178,19 @@
) ) ) ) ) );
$html = $visualizer->visualizeNewClaim( $claim );
- $this->assertEquals( $expect, $html );
+ $this->assertHtmlEquals( $expect, $html );
}
public function testVisualizeRemovedClaim(){
- $expect = '<tr><td colspan="2" class="diff-lineno">property /
PID</td><td colspan="2" class="diff-lineno"></td></tr>'.
+ $expect = '<tr><td colspan="2" class="diff-lineno">property /
<a>PID</a></td><td colspan="2" class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td
class="diff-deletedline">'.
- '<div><del class="diffchange
diffchange-inline"><span>SNAK</span></del></div>'.
- '</td><td colspan="2"> </td></tr><tr><td
colspan="2" class="diff-lineno">property / PID / qualifier</td>'.
+ '<div><del class="diffchange
diffchange-inline"><span><i>SNAK</i></span></del></div>'.
+ '</td><td colspan="2"> </td></tr><tr><td
colspan="2" class="diff-lineno">property / <a>PID</a> / qualifier</td>'.
'<td colspan="2" class="diff-lineno"></td></tr><tr><td
class="diff-marker">-</td><td class="diff-deletedline">'.
- '<div><del class="diffchange
diffchange-inline"><span>PID: SNAK</span></del></div>'.
- '</td><td colspan="2"> </td></tr><tr><td
colspan="2" class="diff-lineno">property / PID / reference</td>'.
+ '<div><del class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div>'.
+ '</td><td colspan="2"> </td></tr><tr><td
colspan="2" class="diff-lineno">property / <a>PID</a> / reference</td>'.
'<td colspan="2" class="diff-lineno"></td></tr><tr><td
class="diff-marker">-</td><td class="diff-deletedline">'.
- '<div><del class="diffchange
diffchange-inline"><span>PID: SNAK</span></del></div></td><td
colspan="2"> </td></tr>';
+ '<div><del class="diffchange
diffchange-inline"><span><a>PID</a>: <i>SNAK</i></span></del></div></td><td
colspan="2"> </td></tr>';
$visualizer = $this->newClaimDifferenceVisualizer();
$claim = new Statement(
@@ -204,10 +203,7 @@
) ) ) ) ) );
$html = $visualizer->visualizeRemovedClaim( $claim );
- $this->assertEquals( $expect, $html );
+ $this->assertHtmlEquals( $expect, $html );
}
-
-
-
}
diff --git a/lib/tests/phpunit/claim/DiffOpValueFormatterTest.php
b/lib/tests/phpunit/claim/DiffOpValueFormatterTest.php
new file mode 100644
index 0000000..8e7b203
--- /dev/null
+++ b/lib/tests/phpunit/claim/DiffOpValueFormatterTest.php
@@ -0,0 +1,58 @@
+<?php
+
+namespace Wikibase\Test;
+
+use Wikibase\DiffOpValueFormatter;
+
+/**
+ * @covers Wikibase\DiffOpValueFormatter
+ *
+ * @group Wikibase
+ * @group WikibaseLib
+ * @group WikibaseClaim
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Kinzler
+ */
+class DiffOpValueFormatterTest extends \PHPUnit_Framework_TestCase {
+
+ /**
+ * @dataProvider provideGenerateHtml
+ *
+ * @param $name
+ * @param $oldValues
+ * @param $newValues
+ * @param $pattern
+ */
+ public function testGenerateHtml( $name, $oldValues, $newValues,
$pattern ) {
+ $formatter = new DiffOpValueFormatter( $name, $oldValues,
$newValues );
+
+ $html = $formatter->generateHtml();
+ $this->assertRegExp( $pattern, $html );
+ }
+
+ public function provideGenerateHtml() {
+ return array(
+ array( 'null', null, null, '@<tr>.*</tr>@' ),
+ array( 'empty strings', '', '', '@<tr>.*</tr>@' ),
+ array( 'empty array', array(), array(), '@<tr>.*</tr>@'
),
+
+ array( 'old string', '<i>old</i>', null,
+ '@<i>old</i>@' ),
+ array( 'new string', null, '<i>new</i>',
+ '@<i>new</i>@' ),
+ array( 'old and new string', '<i>old</i>', '<i>new</i>',
+ '@<i>old</i>.*<i>new</i>@' ),
+
+ array( 'old array', array( '<i>old 1</i>', '<i>old
2</i>' ), null,
+ '@<i>old 1</i>.*<i>old 2</i>@' ),
+ array( 'new array', null, array( '<i>new 1</i>',
'<i>new 2</i>' ),
+ '@<i>new 1</i>.*<i>new 2</i>@' ),
+ array( 'old and new array',
+ array( '<i>old 1</i>', '<i>old 2</i>' ),
+ array( '<i>new 1</i>', '<i>new 2</i>' ),
+ '@<i>old 1</i>.*<i>old 2</i>.*<i>new
1</i>.*<i>new 2</i>@' ),
+ );
+ }
+
+}
diff --git a/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php
b/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php
index 8584178..0135411 100644
--- a/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php
+++ b/lib/tests/phpunit/formatters/WikibaseSnakFormatterBuildersTest.php
@@ -145,6 +145,13 @@
new PropertyValueSnak( 7, new EntityIdValue(
new ItemId( 'Q5' ) ) ),
'Label for Q5' // compare mock object created
in newBuilders()
),
+ 'diff <url>' => array(
+ SnakFormatter::FORMAT_HTML_DIFF,
+ $options,
+ 'url',
+ new PropertyValueSnak( 7, new StringValue(
'<http://acme.com/>' ) ),
+ '<http://acme.com/>'
+ ),
'bad value' => array(
SnakFormatter::FORMAT_PLAIN,
$options,
diff --git
a/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
b/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
index 8653cf9..404ab67 100644
--- a/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
+++ b/lib/tests/phpunit/formatters/WikibaseValueFormatterBuildersTest.php
@@ -1,7 +1,6 @@
<?php
namespace Wikibase\Lib\Test;
-use DataValues\GlobeCoordinateValue;
use DataValues\StringValue;
use DataValues\TimeValue;
use Language;
@@ -11,7 +10,6 @@
use Wikibase\DataModel\Entity\EntityId;
use Wikibase\DataModel\Entity\EntityIdValue;
use Wikibase\DataModel\Entity\ItemId;
-use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\EntityFactory;
use Wikibase\LanguageFallbackChain;
use Wikibase\LanguageFallbackChainFactory;
@@ -100,6 +98,12 @@
$options,
new EntityIdValue( new ItemId( 'Q5' ) ),
'Label for Q5' // compare mock object created
in newBuilders()
+ ),
+ 'diff <url>' => array(
+ SnakFormatter::FORMAT_HTML_DIFF,
+ $options,
+ new StringValue( '<http://acme.com/>' ),
+ '<http://acme.com/>'
),
);
}
@@ -261,6 +265,28 @@
}
/**
+ * @covers WikibaseValueFormatterBuilders::getDiffFormatters
+ */
+ public function testGetDiffFormatters() {
+ $builders = $this->newBuilders( 'string', new ItemId( 'Q5' ) );
+ $options = new FormatterOptions();
+
+ // check for all the required types, that is, the ones
supported by the fallback format
+ $required = array_keys( $builders->getHtmlFormatters( $options
) );
+ $this->assertIncluded(
+ $required,
+ array_keys( $builders->getDiffFormatters( $options ) )
+ );
+
+ // skip two of the required entries
+ $skip = array_slice( $required, 2 );
+ $this->assertExcluded(
+ $skip,
+ array_keys( $builders->getDiffFormatters( $options,
$skip ) )
+ );
+ }
+
+ /**
* Asserts that $actualTypes contains all types listed in
$requiredTypes.
*
* @param string[] $requiredTypes
diff --git a/repo/includes/EntityContentDiffView.php
b/repo/includes/EntityContentDiffView.php
index 2cc2fa5..0ce12ed 100644
--- a/repo/includes/EntityContentDiffView.php
+++ b/repo/includes/EntityContentDiffView.php
@@ -15,6 +15,7 @@
use ValueFormatters\FormatterOptions;
use ValueFormatters\ValueFormatter;
use Wikibase\Lib\EntityIdLabelFormatter;
+use Wikibase\Lib\EscapingValueFormatter;
use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\WikibaseRepo;
use WikiPage;
@@ -66,8 +67,11 @@
ValueFormatter::OPT_LANG => $langCode
) );
- $this->propertyNameFormatter = new EntityIdLabelFormatter(
$options, WikibaseRepo::getDefaultInstance()->getEntityLookup() );
- $this->snakValueFormatter =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getSnakFormatter(
SnakFormatter::FORMAT_PLAIN, $options );
+ $labelFormatter = new EntityIdLabelFormatter( $options,
WikibaseRepo::getDefaultInstance()->getEntityLookup() );
+ $this->propertyNameFormatter = new EscapingValueFormatter(
$labelFormatter, 'htmlspecialchars' );
+
+ $formatterFactory =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory();
+ $this->snakValueFormatter =
$formatterFactory->getSnakFormatter( SnakFormatter::FORMAT_HTML_DIFF, $options
);
// @fixme inject!
$this->diffVisualizer = new EntityDiffVisualizer(
diff --git a/repo/includes/actions/EditEntityAction.php
b/repo/includes/actions/EditEntityAction.php
index 0560eb6..809a2f5 100644
--- a/repo/includes/actions/EditEntityAction.php
+++ b/repo/includes/actions/EditEntityAction.php
@@ -17,6 +17,7 @@
use ValueFormatters\ValueFormatter;
use WebRequest;
use Wikibase\Lib\EntityIdLabelFormatter;
+use Wikibase\Lib\EscapingValueFormatter;
use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\WikibaseRepo;
@@ -59,8 +60,11 @@
ValueFormatter::OPT_LANG => $langCode
) );
- $this->propertyNameFormatter = new EntityIdLabelFormatter(
$options, WikibaseRepo::getDefaultInstance()->getEntityLookup() );
- $this->snakValueFormatter =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getSnakFormatter(
SnakFormatter::FORMAT_PLAIN, $options );
+ $labelFormatter = new EntityIdLabelFormatter( $options,
WikibaseRepo::getDefaultInstance()->getEntityLookup() );
+ $this->propertyNameFormatter = new EscapingValueFormatter(
$labelFormatter, 'htmlspecialchars' );
+
+ $formatterFactory =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory();
+ $this->snakValueFormatter =
$formatterFactory->getSnakFormatter( SnakFormatter::FORMAT_HTML_DIFF, $options
);
$this->diffVisualizer = new EntityDiffVisualizer(
$this->getContext(),
--
To view, visit https://gerrit.wikimedia.org/r/98110
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie852fce9fbb19c58beb21b88e9af16dcb84ff0d5
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
