Denny Vrandecic has submitted this change and it was merged. Change subject: Simple diff visualization ......................................................................
Simple diff visualization - todo: make sure all cases are covered, such as reference change - further design improvements - tests - there's opportunity for DiffView and other places to share code Change-Id: I4459f274b092a4c0aa707af8bafd175775b2c51e --- M lib/WikibaseLib.php A lib/includes/DiffOpValueFormatter.php M lib/includes/claim/ClaimDifferenceVisualizer.php M lib/includes/entity/EntityDiffVisualizer.php M repo/includes/EntityContentDiffView.php M repo/includes/actions/EditEntityAction.php 6 files changed, 414 insertions(+), 25 deletions(-) Approvals: Denny Vrandecic: Verified; Looks good to me, approved diff --git a/lib/WikibaseLib.php b/lib/WikibaseLib.php index 2a8b548..f657410 100644 --- a/lib/WikibaseLib.php +++ b/lib/WikibaseLib.php @@ -94,6 +94,7 @@ $wgAutoloadClasses['Wikibase\ChangeNotifier'] = $dir . 'includes/ChangeNotifier.php'; $wgAutoloadClasses['Wikibase\ChangeNotificationJob'] = $dir . 'includes/ChangeNotificationJob.php'; $wgAutoloadClasses['Wikibase\ChangesTable'] = $dir . 'includes/ChangesTable.php'; +$wgAutoloadClasses['Wikibase\DiffOpValueFormatter'] = $dir . 'includes/DiffOpValueFormatter.php'; $wgAutoloadClasses['Wikibase\DiffView'] = $dir . 'includes/DiffView.php'; $wgAutoloadClasses['Wikibase\Lib\GuidGenerator'] = $dir . 'includes/GuidGenerator.php'; $wgAutoloadClasses['Wikibase\Lib\V4GuidGenerator'] = $dir . 'includes/GuidGenerator.php'; diff --git a/lib/includes/DiffOpValueFormatter.php b/lib/includes/DiffOpValueFormatter.php new file mode 100644 index 0000000..f1411e9 --- /dev/null +++ b/lib/includes/DiffOpValueFormatter.php @@ -0,0 +1,150 @@ +<?php + +namespace Wikibase; + +use Html; +use Diff; + +/** + * Class for generating HTML for Claim Diffs. + * + * 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 + * + * @since 0.4 + * + * @file + * @ingroup WikibaseLib + * + * @licence GNU GPL v2+ + * @author Tobias Gritschacher < [email protected] > + * @author Katie Filbert < [email protected] > + */ +class DiffOpValueFormatter { + + protected $name; + + protected $oldValue; + + protected $newValue; + + public function __construct( $name, $oldValue, $newValue ) { + $this->name = $name; + $this->oldValue = $oldValue; + $this->newValue = $newValue; + } + + /** + * Generates HTML for the header of the diff operation + * + * @since 0.4 + * + * @return string + */ + protected function generateHeaderHtml() { + $html = Html::openElement( 'tr' ); + $html .= Html::rawElement( 'td', array( 'colspan'=>'2', 'class' => 'diff-lineno' ), $this->name ); + $html .= Html::rawElement( 'td', array( 'colspan'=>'2', 'class' => 'diff-lineno' ), $this->name ); + $html .= Html::closeElement( 'tr' ); + + return $html; + } + + /** + * Generates HTML for an change diffOp + * + * @since 0.4 + * + * @return string + */ + protected function generateChangeOpHtml() { + $html = Html::openElement( 'tr' ); + $html .= Html::rawElement( 'td', array( 'class' => 'diff-marker' ), '-' ); + $html .= Html::rawElement( 'td', array( 'class' => 'diff-deletedline' ), + Html::rawElement( 'div', array(), + Html::rawElement( 'del', array( 'class' => 'diffchange diffchange-inline' ), + $this->oldValue ) ) ); + $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->newValue ) ) ); + $html .= Html::closeElement( 'tr' ); + $html .= Html::closeElement( 'tr' ); + + return $html; + } + + /** + * Generates HTML for an add diffOp + * + * @since 0.4 + * + * @return string + */ + protected function generateAddOpHtml() { + $html = Html::openElement( 'tr' ); + $html .= Html::rawElement( 'td', array( 'colspan'=>'2' ), ' ' ); + $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->newValue ) + ) + ); + $html .= Html::closeElement( 'tr' ); + + return $html; + } + + /** + * Generates HTML for an remove diffOp + * + * @since 0.4 + * + * @return string + */ + protected function generateRemoveOpHtml() { + $html = Html::openElement( 'tr' ); + $html .= Html::rawElement( 'td', array( 'class' => 'diff-marker' ), '-' ); + $html .= Html::rawElement( 'td', array( 'class' => 'diff-deletedline' ), + Html::rawElement( 'div', array(), + Html::rawElement( 'del', array( 'class' => 'diffchange diffchange-inline' ), + $this->oldValue ) ) ); + $html .= Html::rawElement( 'td', array( 'colspan'=>'2' ), ' ' ); + $html .= Html::closeElement( 'tr' ); + + return $html; + } + + public function generateHtml() { + $html = ''; + + if ( is_string( $this->name ) ) { + $html .= $this->generateHeaderHtml(); + } + + if ( is_string( $this->oldValue ) && is_string( $this->newValue ) ) { + $html .= $this->generateChangeOpHtml(); + } else if ( is_string( $this->newValue ) ) { + $html .= $this->generateAddOpHtml(); + } else if ( is_string( $this->oldValue ) ) { + $html .= $this->generateRemoveOpHtml(); + } + + return $html; + } + +} diff --git a/lib/includes/claim/ClaimDifferenceVisualizer.php b/lib/includes/claim/ClaimDifferenceVisualizer.php index 22a841d..0e45232 100644 --- a/lib/includes/claim/ClaimDifferenceVisualizer.php +++ b/lib/includes/claim/ClaimDifferenceVisualizer.php @@ -1,9 +1,7 @@ <?php - namespace Wikibase; use Html; -use Diff; /** * Class for generating HTML for Claim Diffs. @@ -30,6 +28,7 @@ * * @licence GNU GPL v2+ * @author Tobias Gritschacher < [email protected] > + * @author Katie Filbert < [email protected] > */ class ClaimDifferenceVisualizer { @@ -46,9 +45,11 @@ * @since 0.4 * * @param EntityLookup $entityLookup + * @param string $langCode */ - public function __construct( $entityLookup ) { + public function __construct( $entityLookup, $langCode ) { $this->entityLookup = $entityLookup; + $this->langCode = $langCode; } /** @@ -58,8 +59,235 @@ * * @return string */ - public function visualizeDiff( ClaimDifference $claimDifference ) { - return 'TODO'; // TODO + public function visualizeDiff( ClaimDifference $claimDifference, $langCode ) { + $html = ''; + + if ( $claimDifference->getMainSnakChange() !== null ) { + $mainSnakChange = $claimDifference->getMainSnakChange(); + $valueFormatter = new DiffOpValueFormatter( + // todo: should shoe specific headers for both columns + $this->getMainSnakHeader( $mainSnakChange->getNewValue() ), + $this->getMainSnakValue( $mainSnakChange->getOldValue() ), + $this->getMainSnakValue( $mainSnakChange->getNewValue() ) + ); + $html .= $valueFormatter->generateHtml(); + } + + if ( $claimDifference->getRankChange() !== null ) { + $rankChange = $claimDifference->getRankChange(); + $valueFormatter = new DiffOpValueFormatter( + wfMessage( 'wikibase-diffview-rank' ), + $rankChange->getOldValue(), + $rankChange->getNewValue() + ); + $html .= $valueFormatter->generateHtml(); + } + + // TODO: html for qualifier changes + + if ( $claimDifference->getReferenceChanges() !== null ) { + $referenceChanges = $claimDifference->getReferenceChanges(); + + // somehow changing a reference value is treated as a diffop add and diffop remove + // for diff visualization, it should be more like a change + // @todo assert that both reference changes refer to the same reference + if ( count( $referenceChanges ) === 2 ) { + + $oldValue = $newValue = null; + + foreach( $referenceChanges as $referenceChange ) { + if ( $referenceChange instanceof \Diff\DiffOpAdd ) { + $newValue = $referenceChange->getNewValue(); + } else if ( $referenceChange instanceof \Diff\DiffOpRemove ) { + $oldValue = $referenceChange->getOldValue(); + } + } + + $html .= $this->getRefHtml( $oldValue, $newValue, 'change' ); + } else { + foreach( $referenceChanges as $referenceChange ) { + if ( $referenceChange instanceof \Diff\DiffOpAdd ) { + $html .= $this->getRefHtml( null, $referenceChange->getNewValue(), 'add' ); + } else if ( $referenceChange instanceof \Diff\DiffOpRemove ) { + $html .= $this->getRefHtml( $referenceChange->getOldValue(), null, 'remove' ); + } else if ( $referenceChange instanceof \Diff\DiffOpChange ) { + $html .= $this->getRefHtml( $referenceChange->getOldValue(), + $reference->getNewValue(), 'change' ); + } + } + } + } + + return $html; + } + + /** + * Format a snak + * + * @since 0.4 + * + * @param Snak|null $oldSnak + * @param Snak|null $newSnak + * @param string $prependHeader + * + * @return string + */ + public function getSnakHtml( $oldSnak, $newSnak, $prependHeader = null ) { + $snakHeader = ''; + // @todo fix ugly cruft! + if ( $prependHeader !== null ) { + $snakHeader = $prependHeader; + } + + if ( $newSnak instanceof Snak || $oldSnak instanceof Snak ) { + $headerSnak = $newSnak instanceof Snak ? $newSnak : $oldSnak; + $snakHeader .= $this->getMainSnakHeader( $headerSnak ); + } else { + // something went wrong + throw new \MWException( 'Snak parameters not provided.' ); + } + + $oldValue = null; + $newValue = null; + + if ( $oldSnak instanceof Snak ) { + $oldValue = $this->getMainSnakValue( $oldSnak ); + } + + if ( $newSnak instanceof Snak ) { + $newValue = $this->getMainSnakValue( $newSnak ); + } + + $valueFormatter = new DiffOpValueFormatter( $snakHeader, $oldValue, $newValue ); + + return $valueFormatter->generateHtml(); + } + + /** + * Get formatted SnakList + * + * @since 0.4 + * + * @param SnakList $snakList + * + * @return string + */ + protected function getSnakListHtml( $snakList ) { + $html = ''; + + foreach ( $snakList as $snak ) { + + if ( $html !== '' ) { + $html .= Html::rawElement( 'br', array(), '' ); + } + // @fixme + $html .= $this->getMainSnakValue( $snak ); + } + + return $html; + } + + /** + * Get formatted header for a snak + * + * @since 0.4 + * + * @param Snak $mainSnak + * + * @return string + */ + protected function getMainSnakHeader( Snak $mainSnak ) { + $propertyId = $mainSnak->getPropertyId(); + $property = $this->entityLookup->getEntity( $propertyId ); + $dataTypeLabel = $property->getDataType()->getLabel( $this->langCode ); + + $label = $property->getLabel( $this->langCode ); + $propertyLabel = $label !== false ? $label : $property->getPrefixedId(); + + $headerText = wfMessage( 'wikibase-entity-property' ) . ' / ' . $dataTypeLabel . ' / ' . $label; + + return $headerText; + } + + /** + * Get main snak value in string form + * + * @since 0.4 + * + * @param Snak $snak + * + * @return string + */ + protected function getMainSnakValue( Snak $snak ) { + $snakType = $snak->getType(); + + if ( $snakType === 'value' ) { + $dataValue = $snak->getDataValue(); + + // FIXME! + if ( $dataValue instanceof EntityId ) { + $diffValueString = $this->getEntityLabel( $dataValue ); + } else { + $diffValueString = $dataValue->getValue(); + } + + return $diffValueString; + } + + // fixme, error handling for unknown snak value types + return ''; + } + + /** + * Get an entity label + * + * @since 0.4 + * + * @param EntityId $entityId + * + * @return string + */ + protected function getEntityLabel( EntityId $entityId ) { + $label = $entityId->getPrefixedId(); + + $lookedUpLabel = $this->entityLookup->getEntity( $entityId )->getLabel( $this->langCode ); + + if ( $lookedUpLabel !== false ) { + $label = $lookedUpLabel; + } + + return $label; + } + + /** + * Format reference change + * + * @since 0.4 + * + * @param $oldRef Reference|null + * @param $newRef Reference|null + * @param $opType string + * + * @return string + */ + protected function getRefHtml( $oldRef, $newRef, $opType ) { + $html = $oldHtml = $newHtml = ''; + + if ( $oldRef !== null ) { + $oldHtml .= $this->getSnakListHtml( $oldRef->getSnaks() ); + } + + if ( $newRef !== null ) { + $newHtml .= $this->getSnakListHtml( $newRef->getSnaks() ); + } + + $valueFormatter = new DiffOpValueFormatter( + wfMessage( 'wikibase-diffview-reference' ), + $oldHtml, + $newHtml + ); + + return $valueFormatter->generateHtml(); } } diff --git a/lib/includes/entity/EntityDiffVisualizer.php b/lib/includes/entity/EntityDiffVisualizer.php index e29ebe0..4eb7e4f 100644 --- a/lib/includes/entity/EntityDiffVisualizer.php +++ b/lib/includes/entity/EntityDiffVisualizer.php @@ -70,7 +70,8 @@ * @param ClaimDiffer $claimDiffer * @param ClaimDifferenceVisualizer $claimDiffView */ - public function __construct( IContextSource $contextSource, ClaimDiffer $claimDiffer, ClaimDifferenceVisualizer $claimDiffView ) { + public function __construct( IContextSource $contextSource, ClaimDiffer $claimDiffer, + ClaimDifferenceVisualizer $claimDiffView ) { $this->context = $contextSource; $this->claimDiffer = $claimDiffer; $this->claimDiffVisualizer = $claimDiffView; @@ -132,29 +133,35 @@ */ protected function getClaimDiffHtml( DiffOp $claimDiffOp ) { if ( $claimDiffOp instanceof DiffOpChange ) { - $claimDifference = $this->claimDiffer->diffClaims( $claimDiffOp->getOldValue(), $claimDiffOp->getNewValue() ); - return $this->claimDiffVisualizer->visualizeDiff( $claimDifference ); + $claimDifference = $this->claimDiffer->diffClaims( + $claimDiffOp->getOldValue(), + $claimDiffOp->getNewValue() + ); + return $this->claimDiffVisualizer->visualizeDiff( + $claimDifference, + $this->context->getLanguage()->getCode() + ); } if ( $claimDiffOp instanceof DiffOpAdd || $claimDiffOp instanceof DiffOpRemove ) { - $claim = $claimDiffOp instanceof DiffOpAdd ? $claimDiffOp->getNewValue() : $claimDiffOp->getOldValue(); - return $this->getClaimHtml( $claim ); + if ( $claimDiffOp instanceof DiffOpAdd ) { + $claim = $claimDiffOp->getNewValue(); + $opType = 'add'; + } else { + $claim = $claimDiffOp->getOldValue(); + $opType = 'remove'; + } + + $mainSnak = $claim->getMainSnak(); + + return $this->claimDiffVisualizer->getSnakHtml( + $mainSnak, + $this->context->getLanguage()->getCode(), + $opType + ); } throw new MWException( 'Encountered an unexpected diff operation type for a claim' ); - } - - /** - * Returns the HTML for a single Claim. - * - * @since 0.4 - * - * @param Claim $claim - * - * @return string - */ - protected function getClaimHtml( Claim $claim ) { - return 'TODO'; // TODO } } diff --git a/repo/includes/EntityContentDiffView.php b/repo/includes/EntityContentDiffView.php index 13d8430..cf58ddd 100644 --- a/repo/includes/EntityContentDiffView.php +++ b/repo/includes/EntityContentDiffView.php @@ -132,12 +132,13 @@ * @var EntityContent $new */ $diff = $old->getEntity()->getDiff( $new->getEntity() ); + $langCode = $this->getContext()->getLanguage()->getCode(); // TODO: derp inject the EntityDiffVisualizer $diffVisualizer = new EntityDiffVisualizer( $this->getContext(), new ClaimDiffer( new ListDiffer() ), - new ClaimDifferenceVisualizer( new WikiPageEntityLookup() ) + new ClaimDifferenceVisualizer( new WikiPageEntityLookup(), $langCode ) ); return $diffVisualizer->visualizeDiff( $diff ); diff --git a/repo/includes/actions/EditEntityAction.php b/repo/includes/actions/EditEntityAction.php index d6a9ac4..e822e93 100644 --- a/repo/includes/actions/EditEntityAction.php +++ b/repo/includes/actions/EditEntityAction.php @@ -412,11 +412,13 @@ $this->getOutput()->addHTML( Html::rawElement( 'td', array( 'colspan' => '2' ), Html::rawElement( 'div', array( 'id' => 'mw-diff-ntitle1' ), $new ) ) ); $this->getOutput()->addHTML( Html::closeElement( 'tr' ) ); + $langCode = $this->getContext()->getLanguage()->getCode(); + // TODO: derp inject the EntityDiffVisualizer $diffVisualizer = new EntityDiffVisualizer( $this->getContext(), new ClaimDiffer( new \Diff\ListDiffer() ), - new ClaimDifferenceVisualizer( new WikiPageEntityLookup() ) + new ClaimDifferenceVisualizer( new WikiPageEntityLookup(), $langCode ) ); $this->getOutput()->addHTML( $diffVisualizer->visualizeDiff( $diff ) ); -- To view, visit https://gerrit.wikimedia.org/r/49672 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4459f274b092a4c0aa707af8bafd175775b2c51e Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: mw1.21-wmf10 Gerrit-Owner: Aude <[email protected]> Gerrit-Reviewer: Denny Vrandecic <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
