Tobias Gritschacher has uploaded a new change for review.
https://gerrit.wikimedia.org/r/84496
Change subject: (bug 46366) Use SnakFormatter for diffs
......................................................................
(bug 46366) Use SnakFormatter for diffs
Change-Id: I2381dd2a256ebd99bb3852a0e1d97ca38c1f0554
---
M lib/includes/ClaimDifferenceVisualizer.php
M repo/includes/EntityContentDiffView.php
M repo/includes/actions/EditEntityAction.php
3 files changed, 75 insertions(+), 96 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/96/84496/1
diff --git a/lib/includes/ClaimDifferenceVisualizer.php
b/lib/includes/ClaimDifferenceVisualizer.php
index cd35dc5..c99e98f 100644
--- a/lib/includes/ClaimDifferenceVisualizer.php
+++ b/lib/includes/ClaimDifferenceVisualizer.php
@@ -8,7 +8,8 @@
use Html;
use Diff\Diff;
use RuntimeException;
-use Wikibase\Lib\EntityIdFormatter;
+use Wikibase\Lib\EntityIdLabelFormatter;
+use Wikibase\Lib\SnakFormatter;
/**
* Class for generating HTML for Claim Diffs.
@@ -27,37 +28,43 @@
/**
* @since 0.4
*
- * @var EntityLookup
- */
- private $entityLookup;
-
- /**
- * @since 0.4
- *
* @var string
*/
private $langCode;
/**
- * @since 0.4
+ * @since 0.5
*
- * @var EntityIdFormatter
+ * @var EntityIdLabelFormatter
*/
- private $idFormatter;
+ private $propertyFormatter;
+
+ /**
+ * @since 0.5
+ *
+ * @var SnakFormatter
+ */
+ private $snakFormatter;
/**
* Constructor.
*
* @since 0.4
*
- * @param EntityLookup $entityLookup
- * @param string $langCode
- * @param EntityIdFormatter $idFormatter
+ * @param EntityIdLabelFormatter $propertyFormatter
+ * @param SnakFormatter $snakFormatter
+ *
+ * @throws \InvalidArgumentException
*/
- public function __construct( $entityLookup, $langCode,
EntityIdFormatter $idFormatter ) {
- $this->entityLookup = $entityLookup;
- $this->langCode = $langCode;
- $this->idFormatter = $idFormatter;
+ public function __construct( EntityIdLabelFormatter $propertyFormatter,
SnakFormatter $snakFormatter ) {
+ if ( $snakFormatter->getFormat() !==
SnakFormatter::FORMAT_PLAIN ) {
+ throw new \InvalidArgumentException(
+ 'Expected $snakFormatter to generate plain
text, not '
+ . $snakFormatter->getFormat() );
+ }
+
+ $this->propertyFormatter = $propertyFormatter;
+ $this->snakFormatter = $snakFormatter;
}
/**
@@ -155,8 +162,8 @@
$valueFormatter = new DiffOpValueFormatter(
// todo: should show specific headers for both columns
$this->getSnakHeader( $mainSnakChange->getNewValue() ),
- $this->getSnakValue( $mainSnakChange->getOldValue() ),
- $this->getSnakValue( $mainSnakChange->getNewValue() )
+ $this->snakFormatter->formatSnak(
$mainSnakChange->getOldValue() ),
+ $this->snakFormatter->formatSnak(
$mainSnakChange->getNewValue() )
);
return $valueFormatter->generateHtml();
@@ -210,11 +217,11 @@
$newValue = null;
if ( $oldSnak instanceof Snak ) {
- $oldValue = $this->getSnakValue( $oldSnak );
+ $oldValue = $this->snakFormatter->formatSnak( $oldSnak
);
}
if ( $newSnak instanceof Snak ) {
- $newValue = $this->getSnakValue( $newSnak );
+ $newValue = $this->snakFormatter->formatSnak( $newSnak
);
}
$valueFormatter = new DiffOpValueFormatter( $snakHeader,
$oldValue, $newValue );
@@ -238,9 +245,9 @@
// TODO: change hardcoded ": " so something like
wfMessage( 'colon-separator' ),
// but this will require further refactoring as it
would add HTML which gets escaped
$values[] =
- $this->getEntityLabel( $snak->getPropertyId() )
.
+ $this->propertyFormatter->format(
$snak->getPropertyId() ) .
': '.
- $this->getSnakValue( $snak );
+ $this->snakFormatter->formatSnak( $snak );
}
return $values;
@@ -257,65 +264,10 @@
*/
protected function getSnakHeader( Snak $snak ) {
$propertyId = $snak->getPropertyId();
- $propertyLabel = $this->getEntityLabel( $propertyId );
+ $propertyLabel = $this->propertyFormatter->format( $propertyId
);
$headerText = wfMessage( 'wikibase-entity-property' ) . ' / ' .
$propertyLabel;
return $headerText;
- }
-
- /**
- * Get snak value in string form
- *
- * @since 0.4
- *
- * @param Snak $snak
- *
- * @return string
- */
- protected function getSnakValue( Snak $snak ) {
- $snakType = $snak->getType();
-
- if ( $snakType === 'value' ) {
- $dataValue = $snak->getDataValue();
-
- // FIXME! should use some value formatter
- if ( $dataValue instanceof EntityId ) {
- $diffValueString = $this->getEntityLabel(
$dataValue );
- } else if ( $dataValue instanceof TimeValue ) {
- // TODO: this will just display the plain
ISO8601-string,
- // we should instead use a decent formatter
- $diffValueString = $dataValue->getTime();
- } else {
- $diffValueString = $dataValue->getValue();
- }
-
- return $diffValueString;
- } else {
- return $snakType;
- }
- }
-
- /**
- * Get an entity label
- *
- * @since 0.4
- *
- * @param EntityId $entityId
- *
- * @return string
- */
- protected function getEntityLabel( EntityId $entityId ) {
- $entity = $this->entityLookup->getEntity( $entityId );
-
- if ( $entity instanceof Entity ) {
- $lookedUpLabel = $this->entityLookup->getEntity(
$entityId )->getLabel( $this->langCode );
-
- if ( $lookedUpLabel !== false ) {
- return $lookedUpLabel;
- }
- }
-
- return $this->idFormatter->format( $entityId );
}
/**
@@ -387,23 +339,23 @@
// but this will require further refactoring as it
would add HTML which gets escaped
if ( $change instanceof DiffOpAdd ) {
$newVal =
- $this->getEntityLabel(
$change->getNewValue()->getPropertyId() ) .
+ $this->propertyFormatter->format(
$change->getNewValue()->getPropertyId() ) .
': ' .
- $this->getSnakValue(
$change->getNewValue() );
+ $this->snakFormatter->formatSnak(
$change->getNewValue() );
} else if ( $change instanceof DiffOpRemove ) {
$oldVal =
- $this->getEntityLabel(
$change->getOldValue()->getPropertyId() ) .
+ $this->propertyFormatter->format(
$change->getOldValue()->getPropertyId() ) .
': ' .
- $this->getSnakValue(
$change->getOldValue() );
+ $this->snakFormatter->formatSnak(
$change->getOldValue() );
} else if ( $change instanceof DiffOpChange ) {
$oldVal =
- $this->getEntityLabel(
$change->getOldValue()->getPropertyId() ) .
+ $this->propertyFormatter->format(
$change->getOldValue()->getPropertyId() ) .
': ' .
- $this->getSnakValue(
$change->getOldValue() );
+ $this->snakFormatter->formatSnak(
$change->getOldValue() );
$newVal =
- $this->getEntityLabel(
$change->getNewValue()->getPropertyId() ) .
+ $this->propertyFormatter->format(
$change->getNewValue()->getPropertyId() ) .
': ' .
- $this->getSnakValue(
$change->getNewValue() );
+ $this->snakFormatter->formatSnak(
$change->getNewValue() );
} else {
throw new RuntimeException( 'Diff operation of
unknown type.' );
}
diff --git a/repo/includes/EntityContentDiffView.php
b/repo/includes/EntityContentDiffView.php
index e459a96..252658e 100644
--- a/repo/includes/EntityContentDiffView.php
+++ b/repo/includes/EntityContentDiffView.php
@@ -5,6 +5,10 @@
use Diff\ListDiffer;
use Content, Html;
+use ValueFormatters\FormatterOptions;
+use ValueFormatters\ValueFormatter;
+use Wikibase\Lib\EntityIdLabelFormatter;
+use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\WikibaseRepo;
/**
@@ -49,7 +53,14 @@
public function __construct( $context = null, $old = 0, $new = 0, $rcid
= 0, $refreshCache = false, $unhide = false ) {
parent::__construct( $context, $old, $new, $rcid,
$refreshCache, $unhide );
- $this->mRefreshCache = true; //FIXME: debug only!
+ //TODO: proper injection
+ $options = new FormatterOptions( array(
+ //TODO: fallback chain
+ ValueFormatter::OPT_LANG =>
$this->getContext()->getLanguage()->getCode()
+ ) );
+
+ $this->propertyNameFormatter = new EntityIdLabelFormatter(
$options, WikibaseRepo::getDefaultInstance()->getEntityLookup() );
+ $this->snakValueFormatter =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getFormatter(
SnakFormatter::FORMAT_PLAIN, $options );
}
/**
@@ -134,7 +145,6 @@
* @var EntityContent $new
*/
$diff = $old->getEntity()->getDiff( $new->getEntity() );
- $langCode = $this->getContext()->getLanguage()->getCode();
$comparer = function( \Comparable $old, \Comparable $new ) {
return $old->equals( $new );
@@ -145,9 +155,8 @@
$this->getContext(),
new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
new ClaimDifferenceVisualizer(
- new WikiPageEntityLookup(),
- $langCode,
-
WikibaseRepo::getDefaultInstance()->getIdFormatter()
+ $this->propertyNameFormatter,
+ $this->snakValueFormatter
)
);
diff --git a/repo/includes/actions/EditEntityAction.php
b/repo/includes/actions/EditEntityAction.php
index 2db7579..c1f9451 100644
--- a/repo/includes/actions/EditEntityAction.php
+++ b/repo/includes/actions/EditEntityAction.php
@@ -3,6 +3,10 @@
namespace Wikibase;
use Diff\CallbackListDiffer;
use Html, Linker, Skin, Status, Revision;
+use ValueFormatters\FormatterOptions;
+use ValueFormatters\ValueFormatter;
+use Wikibase\Lib\EntityIdLabelFormatter;
+use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\WikibaseRepo;
/**
@@ -24,6 +28,21 @@
* @since 0.1
*/
abstract class EditEntityAction extends ViewEntityAction {
+
+ public function __construct( \Page $page, \IContextSource $context =
null ) {
+ parent::__construct( $page, $context );
+
+ //TODO: proper injection
+ $options = new FormatterOptions( array(
+ //TODO: fallback chain
+ ValueFormatter::OPT_LANG =>
$this->getContext()->getLanguage()->getCode()
+ ) );
+
+ $this->propertyNameFormatter = new EntityIdLabelFormatter(
$options, WikibaseRepo::getDefaultInstance()->getEntityLookup() );
+ $this->snakValueFormatter =
WikibaseRepo::getDefaultInstance()->getSnakFormatterFactory()->getFormatter(
SnakFormatter::FORMAT_PLAIN, $options );
+
+ }
+
/**
* @see Action::getName()
*
@@ -440,9 +459,8 @@
$this->getContext(),
new ClaimDiffer( new CallbackListDiffer( $comparer ) ),
new ClaimDifferenceVisualizer(
- new WikiPageEntityLookup(),
- $langCode,
-
WikibaseRepo::getDefaultInstance()->getIdFormatter()
+ $this->propertyNameFormatter,
+ $this->snakValueFormatter
)
);
--
To view, visit https://gerrit.wikimedia.org/r/84496
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2381dd2a256ebd99bb3852a0e1d97ca38c1f0554
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
