Tobias Gritschacher has submitted this change and it was merged. 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(-) Approvals: Tobias Gritschacher: Verified; Looks good to me, approved jenkins-bot: Verified 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: merged Gerrit-Change-Id: I2381dd2a256ebd99bb3852a0e1d97ca38c1f0554 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Tobias Gritschacher <tobias.gritschac...@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: 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