jenkins-bot has submitted this change and it was merged. Change subject: Major refactoring of ClaimDifferenceVisualizer ......................................................................
Major refactoring of ClaimDifferenceVisualizer PS1 was a major refactoring but is now obsolete because of Adrian's refactoring. I redid my refactoring. Now it's a minor one. * Simpler variable names. * Refactoring to avoid some warnings in PHPStorm. * Drop non-informative doc lines. Change-Id: Ia49bdcce115a34f9f9416a2ad0bd67008d475b02 --- M repo/includes/Diff/ClaimDifferenceVisualizer.php M repo/includes/Diff/DiffOpValueFormatter.php M repo/includes/Diff/DifferencesSnakVisualizer.php 3 files changed, 75 insertions(+), 85 deletions(-) Approvals: Hoo man: Looks good to me, approved jenkins-bot: Verified diff --git a/repo/includes/Diff/ClaimDifferenceVisualizer.php b/repo/includes/Diff/ClaimDifferenceVisualizer.php index d261357..4edf0ec 100644 --- a/repo/includes/Diff/ClaimDifferenceVisualizer.php +++ b/repo/includes/Diff/ClaimDifferenceVisualizer.php @@ -7,12 +7,9 @@ use Diff\DiffOp\DiffOpAdd; use Diff\DiffOp\DiffOpChange; use Diff\DiffOp\DiffOpRemove; -use Message; -use RuntimeException; use Wikibase\DataModel\Claim\Claim; use Wikibase\DataModel\Snak\Snak; -use Wikibase\DataModel\Snak\SnakList; -use Wikibase\Lib\EntityIdFormatter; +use Wikibase\DataModel\Snak\Snaks; use Wikibase\Lib\Serializers\ClaimSerializer; /** @@ -26,6 +23,7 @@ * @author Adam Shorland * @author Daniel Kinzler * @author Adrian Heine < [email protected] > + * @author Thiemo Mättig */ class ClaimDifferenceVisualizer { @@ -55,12 +53,13 @@ /** * Generates HTML of a claim change. + * * @since 0.4 * * @param ClaimDifference $claimDifference * @param Claim $baseClaim The new claim, if it exists; otherwise, the old claim * - * @return string + * @return string HTML */ public function visualizeClaimChange( ClaimDifference $claimDifference, Claim $baseClaim ) { $newestMainSnak = $baseClaim->getMainSnak(); @@ -93,7 +92,7 @@ $referenceChanges = $claimDifference->getReferenceChanges(); if ( $referenceChanges !== null ) { - $html .= $this->visualizeSnakListChanges( + $html .= $this->visualizeReferenceChanges( $referenceChanges, $oldestMainSnak, $newestMainSnak @@ -110,7 +109,7 @@ * * @param Claim $claim * - * @return string + * @return string HTML */ public function visualizeNewClaim( Claim $claim ) { $claimDiffer = new ClaimDiffer( new ListDiffer() ); @@ -125,7 +124,7 @@ * * @param Claim $claim * - * @return string + * @return string HTML */ public function visualizeRemovedClaim( Claim $claim ) { $claimDiffer = new ClaimDiffer( new ListDiffer() ); @@ -134,15 +133,11 @@ } /** - * Get Html for a main snak change - * - * @since 0.4 - * * @param DiffOpChange $mainSnakChange * @param Snak $oldestMainSnak The old main snak, if present; otherwise, the new main snak * @param Snak $newestMainSnak The new main snak, if present; otherwise, the old main snak * - * @return string + * @return string HTML */ private function visualizeMainSnakChange( DiffOpChange $mainSnakChange, @@ -161,21 +156,13 @@ } /** - * Get Html for rank change - * - * @since 0.4 - * * @param DiffOpChange $rankChange * @param Snak $oldestMainSnak The old main snak, if present; otherwise, the new main snak * @param Snak $newestMainSnak The new main snak, if present; otherwise, the old main snak * - * @return string + * @return string HTML */ - private function visualizeRankChange( - DiffOpChange $rankChange, - Snak $oldestMainSnak, - Snak $newestMainSnak - ) { + private function visualizeRankChange( DiffOpChange $rankChange, Snak $oldestMainSnak, Snak $newestMainSnak ) { $msg = wfMessage( 'wikibase-diffview-rank' )->inLanguage( $this->languageCode ); $header = $msg->parse(); @@ -212,23 +199,13 @@ } /** - * Get Html for snaklist changes - * - * @since 0.4 - * * @param Diff $changes - * @param Message $breadCrumb * @param Snak $oldestMainSnak The old main snak, if present; otherwise, the new main snak * @param Snak $newestMainSnak The new main snak, if present; otherwise, the old main snak * - * @return string - * @throws RuntimeException + * @return string HTML */ - private function visualizeSnakListChanges( - Diff $changes, - Snak $oldestMainSnak, - Snak $newestMainSnak - ) { + private function visualizeReferenceChanges( Diff $changes, Snak $oldestMainSnak, Snak $newestMainSnak ) { $html = ''; $msg = wfMessage( 'wikibase-diffview-reference' )->inLanguage( $this->languageCode ); @@ -240,25 +217,30 @@ . ' / ' . $header; foreach ( $changes as $change ) { - $newVal = $oldVal = null; - if ( $change instanceof DiffOpAdd || $change instanceof DiffOpChange ) { - $newVal = $this->mapSnakList( - $change->getNewValue()->getSnaks(), - array( $this->snakVisualizer, 'getPropertyAndDetailedValue' ) - ); + $oldValue = null; + $newValue = null; + + if ( $change instanceof DiffOpAdd ) { + $newValue = $change->getNewValue(); + } elseif ( $change instanceof DiffOpChange ) { + $oldValue = $change->getOldValue(); + $newValue = $change->getNewValue(); + } elseif ( $change instanceof DiffOpRemove ) { + $oldValue = $change->getOldValue(); } - if ( $change instanceof DiffOpRemove || $change instanceof DiffOpChange ) { - $oldVal = $this->mapSnakList( - $change->getOldValue()->getSnaks(), - array( $this->snakVisualizer, 'getPropertyAndDetailedValue' ) - ); - } + + $oldValuesHtml = $oldValue !== null + ? $this->visualizeSnaks( $oldValue->getSnaks() ) + : null; + $newValuesHtml = $newValue !== null + ? $this->visualizeSnaks( $newValue->getSnaks() ) + : null; $valueFormatter = new DiffOpValueFormatter( $oldClaimHeader, $newClaimHeader, - $oldVal, - $newVal + $oldValuesHtml, + $newValuesHtml ); $html .= $valueFormatter->generateHtml(); @@ -268,38 +250,28 @@ } /** - * Map SnakList + * @param Snaks $snaks * - * @param SnakList $snakList - * @param callable $mapper - * - * @return mixed[] + * @return string[] HTML */ - private function mapSnakList( SnakList $snakList, $mapper ) { - $ret = array(); - foreach ( $snakList as $snak ) { - $ret[] = call_user_func( $mapper, $snak ); + private function visualizeSnaks( Snaks $snaks ) { + $html = array(); + + foreach ( $snaks as $snak ) { + $html[] = $this->snakVisualizer->getPropertyAndDetailedValue( $snak ); } - return $ret; + + return $html; } /** - * Get Html for qualifier changes - * - * @since 0.4 - * * @param Diff $changes * @param Snak $oldestMainSnak The old main snak, if present; otherwise, the new main snak * @param Snak $newestMainSnak The new main snak, if present; otherwise, the old main snak * - * @return string - * @throws RuntimeException + * @return string HTML */ - private function visualizeQualifierChanges( - Diff $changes, - Snak $oldestMainSnak, - Snak $newestMainSnak - ) { + private function visualizeQualifierChanges( Diff $changes, Snak $oldestMainSnak, Snak $newestMainSnak ) { $html = ''; $msg = wfMessage( 'wikibase-diffview-qualifier' )->inLanguage( $this->languageCode ); @@ -311,20 +283,30 @@ . ' / ' . $header; foreach ( $changes as $change ) { - $newVal = $oldVal = null; + $oldValue = null; + $newValue = null; - if ( $change instanceof DiffOpAdd || $change instanceof DiffOpChange ) { - $newVal = $this->snakVisualizer->getPropertyAndDetailedValue( $change->getNewValue() ); + if ( $change instanceof DiffOpAdd ) { + $newValue = $change->getNewValue(); + } elseif ( $change instanceof DiffOpChange ) { + $oldValue = $change->getOldValue(); + $newValue = $change->getNewValue(); + } elseif ( $change instanceof DiffOpRemove ) { + $oldValue = $change->getOldValue(); } - if ( $change instanceof DiffOpRemove || $change instanceof DiffOpChange ) { - $oldVal = $this->snakVisualizer->getPropertyAndDetailedValue( $change->getOldValue() ); - } + + $oldValueHtml = $oldValue !== null + ? $this->snakVisualizer->getPropertyAndDetailedValue( $oldValue ) + : null; + $newValueHtml = $newValue !== null + ? $this->snakVisualizer->getPropertyAndDetailedValue( $newValue ) + : null; $valueFormatter = new DiffOpValueFormatter( - $oldClaimHeader, - $newClaimHeader, - $oldVal, - $newVal + $oldClaimHeader, + $newClaimHeader, + $oldValueHtml, + $newValueHtml ); $html .= $valueFormatter->generateHtml(); diff --git a/repo/includes/Diff/DiffOpValueFormatter.php b/repo/includes/Diff/DiffOpValueFormatter.php index c6deef1..a4e8017 100644 --- a/repo/includes/Diff/DiffOpValueFormatter.php +++ b/repo/includes/Diff/DiffOpValueFormatter.php @@ -41,14 +41,14 @@ * * @param string $oldName HTML of old name * @param string $newName HTML of new name - * @param string|string[]|null $oldValues HTML of old value(s) - * @param string|string[]|null $newValues HTML of new value(s) + * @param string|string[]|null $oldValuesHtml HTML of old value(s) + * @param string|string[]|null $newValuesHtml HTML of new value(s) */ - public function __construct( $oldName, $newName, $oldValues, $newValues ) { + public function __construct( $oldName, $newName, $oldValuesHtml, $newValuesHtml ) { $this->oldName = $oldName; $this->newName = $newName; - $this->oldValues = is_string( $oldValues ) ? array( $oldValues ) : $oldValues; - $this->newValues = is_string( $newValues ) ? array( $newValues ) : $newValues; + $this->oldValues = is_string( $oldValuesHtml ) ? array( $oldValuesHtml ) : $oldValuesHtml; + $this->newValues = is_string( $newValuesHtml ) ? array( $newValuesHtml ) : $newValuesHtml; } /** diff --git a/repo/includes/Diff/DifferencesSnakVisualizer.php b/repo/includes/Diff/DifferencesSnakVisualizer.php index f50eef2..fcece28 100644 --- a/repo/includes/Diff/DifferencesSnakVisualizer.php +++ b/repo/includes/Diff/DifferencesSnakVisualizer.php @@ -152,6 +152,8 @@ * Get a detailed formatted snak, including the snak's property label and value. * * @param Snak $snak + * + * @return string HTML */ public function getPropertyAndDetailedValue( Snak $snak ) { return $this->getColonSeparatedHtml( @@ -160,6 +162,12 @@ ); } + /** + * @param string $before HTML + * @param string $after HTML + * + * @return string HTML + */ private function getColonSeparatedHtml( $before, $after ) { $colon = wfMessage( 'colon-separator' )->inLanguage( $this->languageCode )->escaped(); -- To view, visit https://gerrit.wikimedia.org/r/187384 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia49bdcce115a34f9f9416a2ad0bd67008d475b02 Gerrit-PatchSet: 3 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]> Gerrit-Reviewer: Adrian Lang <[email protected]> Gerrit-Reviewer: Aude <[email protected]> Gerrit-Reviewer: Daniel Kinzler <[email protected]> Gerrit-Reviewer: Hoo man <[email protected]> Gerrit-Reviewer: Jeroen De Dauw <[email protected]> Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
