Thiemo Kreuz (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/403969 )
Change subject: Fix mistakes and refactor FormDiffView and related ...................................................................... Fix mistakes and refactor FormDiffView and related I believe this is all very much trivial refactoring. This patch also fixes a few mistakes, but these happened in comments. Bug: T182424 Change-Id: I4e6b9531d56405e8d2c72d501c322ab71e6ead29 --- M src/Diff/FormDiffView.php M src/Diff/LexemeDiffVisualizer.php M tests/phpunit/mediawiki/Diff/FormDiffViewTest.php 3 files changed, 54 insertions(+), 60 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseLexeme refs/changes/69/403969/1 diff --git a/src/Diff/FormDiffView.php b/src/Diff/FormDiffView.php index 1d82223..38c46e8 100644 --- a/src/Diff/FormDiffView.php +++ b/src/Diff/FormDiffView.php @@ -22,12 +22,12 @@ class FormDiffView extends BasicDiffView { /** - * @var ClaimDiffer|null + * @var ClaimDiffer */ private $claimDiffer; /** - * @var ClaimDifferenceVisualizer|null + * @var ClaimDifferenceVisualizer */ private $claimDiffVisualizer; @@ -61,26 +61,29 @@ * @param string[] $path * @param DiffOp $op * - * @return string - * @throws MWException + * @return string HTML */ protected function generateOpHtml( array $path, DiffOp $op ) { - $html = ''; if ( $op->isAtomic() ) { return parent::generateOpHtml( $path, $op ); } + + $html = ''; + foreach ( $op as $key => $subOp ) { - if ( !$subOp instanceof ChangeFormDiffOp ) { - $html .= $this->generateOpHtml( array_merge( $path, [ $key ] ), $subOp ); - } else { + if ( $subOp instanceof ChangeFormDiffOp ) { $html .= $this->generateFormOpHtml( $path, $subOp, $key ); + } else { + $html .= $this->generateOpHtml( array_merge( $path, [ $key ] ), $subOp ); } } + return $html; } - private function generateFormOpHtml( $path, ChangeFormDiffOp $op, $key ) { + private function generateFormOpHtml( array $path, ChangeFormDiffOp $op, $key ) { $html = ''; + foreach ( $op->getStatementsDiffOps() as $claimDiffOp ) { $html .= $this->getClaimDiffHtml( $claimDiffOp, @@ -108,33 +111,31 @@ } /** - * @param DiffOp $claimDiffOp + * @param DiffOp $diffOp * * @return string HTML * @throws MWException */ - protected function getClaimDiffHtml( DiffOp $claimDiffOp, array $path ) { - if ( $claimDiffOp instanceof DiffOpChange ) { - $claimDifference = $this->claimDiffer->diffClaims( - $claimDiffOp->getOldValue(), - $claimDiffOp->getNewValue() - ); - return $this->claimDiffVisualizer->visualizeClaimChange( - $claimDifference, - $claimDiffOp->getNewValue(), - $path - ); - } + private function getClaimDiffHtml( DiffOp $diffOp, array $path ) { + switch ( true ) { + case $diffOp instanceof DiffOpChange: + return $this->claimDiffVisualizer->visualizeClaimChange( + $this->claimDiffer->diffClaims( + $diffOp->getOldValue(), + $diffOp->getNewValue() + ), + $diffOp->getNewValue(), + $path + ); - if ( $claimDiffOp instanceof DiffOpAdd ) { - return $this->claimDiffVisualizer->visualizeNewClaim( $claimDiffOp->getNewValue(), $path ); - } elseif ( $claimDiffOp instanceof DiffOpRemove ) { - return $this->claimDiffVisualizer->visualizeRemovedClaim( - $claimDiffOp->getOldValue(), - $path - ); - } else { - throw new MWException( 'Encountered an unexpected diff operation type for a claim' ); + case $diffOp instanceof DiffOpAdd: + return $this->claimDiffVisualizer->visualizeNewClaim( $diffOp->getNewValue(), $path ); + + case $diffOp instanceof DiffOpRemove: + return $this->claimDiffVisualizer->visualizeRemovedClaim( $diffOp->getOldValue(), $path ); + + default: + throw new MWException( 'Encountered an unexpected diff operation type for a claim' ); } } diff --git a/src/Diff/LexemeDiffVisualizer.php b/src/Diff/LexemeDiffVisualizer.php index a8210f2..7c4c610 100644 --- a/src/Diff/LexemeDiffVisualizer.php +++ b/src/Diff/LexemeDiffVisualizer.php @@ -4,7 +4,7 @@ use Diff\DiffOp\Diff\Diff; use MessageLocalizer; -use Wikibase\DataModel\Services\Diff\EntityDiff; +use Wikibase\Lexeme\DataModel\Services\Diff\LexemeDiff; use Wikibase\Repo\Content\EntityContentDiff; use Wikibase\Repo\Diff\BasicDiffView; use Wikibase\Repo\Diff\BasicEntityDiffVisualizer; @@ -13,8 +13,6 @@ use Wikibase\Repo\Diff\EntityDiffVisualizer; /** - * Class for generating views of EntityDiff objects. - * * @license GPL-2.0+ * @author Amir Sarabadani */ @@ -31,12 +29,12 @@ private $basicEntityDiffVisualizer; /** - * @var ClaimDiffer|null + * @var ClaimDiffer */ private $claimDiffer; /** - * @var ClaimDifferenceVisualizer|null + * @var ClaimDifferenceVisualizer */ private $claimDiffVisualizer; @@ -53,31 +51,26 @@ } /** - * Generates and returns an HTML visualization of the provided EntityContentDiff. - * * @param EntityContentDiff $diff * - * @return string + * @return string HTML */ public function visualizeEntityContentDiff( EntityContentDiff $diff ) { if ( $diff->isEmpty() ) { return ''; } - $basicHtml = $this->basicEntityDiffVisualizer->visualizeEntityContentDiff( $diff ); - - return $basicHtml . $this->visualizeEntityDiff( $diff->getEntityDiff() ); + return $this->basicEntityDiffVisualizer->visualizeEntityContentDiff( $diff ) + . $this->visualizeEntityDiff( $diff->getEntityDiff() ); } /** - * Generates and returns an HTML visualization of the provided EntityDiff. + * @param LexemeDiff $diff * - * @param EntityDiff $diff - * - * @return string + * @return string HTML */ - private function visualizeEntityDiff( EntityDiff $diff ) { - $basicDiff = ( new BasicDiffView( + private function visualizeEntityDiff( LexemeDiff $diff ) { + $basicDiffView = new BasicDiffView( [], new Diff( [ @@ -90,8 +83,9 @@ ], true ) - ) )->getHtml(); - $formDiff = ( new FormDiffView( + ); + + $formDiffView = new FormDiffView( [], new Diff( [ @@ -103,8 +97,9 @@ $this->claimDiffer, $this->claimDiffVisualizer, $this->messageLocalizer - ) )->getHtml(); - return $basicDiff . $formDiff; + ); + + return $basicDiffView->getHtml() . $formDiffView->getHtml(); } } diff --git a/tests/phpunit/mediawiki/Diff/FormDiffViewTest.php b/tests/phpunit/mediawiki/Diff/FormDiffViewTest.php index e4e3634..13ee9c4 100644 --- a/tests/phpunit/mediawiki/Diff/FormDiffViewTest.php +++ b/tests/phpunit/mediawiki/Diff/FormDiffViewTest.php @@ -3,8 +3,8 @@ namespace Wikibase\Lexeme\Tests\Diff; use Diff\Comparer\ComparableComparer; +use Diff\Differ\OrderedListDiffer; use Diff\DiffOp\Diff\Diff; -use Diff\OrderedListDiffer; use MessageLocalizer; use RawMessage; use Wikibase\DataModel\Entity\PropertyId; @@ -36,18 +36,14 @@ /** * @param string $returnValue - * @param string $format * * @return SnakFormatter */ - public function newSnakFormatter( - $returnValue = '<i>SNAK</i>', - $format = SnakFormatter::FORMAT_HTML - ) { + public function newSnakFormatter( $returnValue = '<i>SNAK</i>' ) { $instance = $this->getMock( SnakFormatter::class ); $instance->expects( $this->any() ) ->method( 'getFormat' ) - ->will( $this->returnValue( $format ) ); + ->will( $this->returnValue( SnakFormatter::FORMAT_HTML ) ); $instance->expects( $this->any() ) ->method( 'canFormatSnak' ) ->will( $this->returnValue( true ) ); @@ -100,6 +96,8 @@ } /** + * @param ChangeFormDiffOp $diff + * * @return FormDiffView */ private function getDiffView( ChangeFormDiffOp $diff ) { @@ -225,7 +223,7 @@ } /** - * @return mixed + * @return Statement */ private function someStatement( $propertyId, $guid ) { $statement = new Statement( -- To view, visit https://gerrit.wikimedia.org/r/403969 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4e6b9531d56405e8d2c72d501c322ab71e6ead29 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/WikibaseLexeme Gerrit-Branch: master Gerrit-Owner: Thiemo Kreuz (WMDE) <thiemo.kr...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits