Thiemo Kreuz (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/403963 )
Change subject: Move path array prepending logic to ClaimDifferenceVisualizer
......................................................................
Move path array prepending logic to ClaimDifferenceVisualizer
With this patch I am making the prepended path a private implementation
detail of the ClaimDifferenceVisualizer class, and remove it entirely
from DifferencesSnakVisualizer where it was before. That solves all
concerns I had with the original patch Ia4bebd8:
* The $path is not passed so deeply into code that does not really need
to know anything about it.
* Many of the optional $path parameters are gone.
I had multiple reason to do it this way and would like to explain them,
if needed.
One reason is that a method called "getPropertyHeader" should only do
exactly that: return the substring that describes a property.
Note this patch does not change anything that is needed for I50eb048.
The code I am touching in this patch is not used outside of the
Wikibase code base.
Bug: T182424
Change-Id: I0408672f42b9822f0fe380d8f692b15a7d64f73d
---
M repo/includes/Diff/ClaimDifferenceVisualizer.php
M repo/includes/Diff/DifferencesSnakVisualizer.php
M repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
3 files changed, 54 insertions(+), 57 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/63/403963/1
diff --git a/repo/includes/Diff/ClaimDifferenceVisualizer.php
b/repo/includes/Diff/ClaimDifferenceVisualizer.php
index d75f9a6..7777ab9 100644
--- a/repo/includes/Diff/ClaimDifferenceVisualizer.php
+++ b/repo/includes/Diff/ClaimDifferenceVisualizer.php
@@ -55,6 +55,7 @@
Statement $baseStatement,
array $path = []
) {
+ $headerPrefix = implode( ' / ', $path ) . ' / ';
$newestMainSnak = $baseStatement->getMainSnak();
$oldestMainSnak = $newestMainSnak;
$html = '';
@@ -63,40 +64,40 @@
if ( $mainSnakChange !== null ) {
$oldestMainSnak = $mainSnakChange->getOldValue() ?:
$newestMainSnak;
$html .= $this->visualizeMainSnakChange(
+ $headerPrefix,
$mainSnakChange,
$oldestMainSnak,
- $newestMainSnak,
- $path
+ $newestMainSnak
);
}
$rankChange = $claimDifference->getRankChange();
if ( $rankChange !== null ) {
$html .= $this->visualizeRankChange(
+ $headerPrefix,
$rankChange,
$oldestMainSnak,
- $newestMainSnak,
- $path
+ $newestMainSnak
);
}
$qualifierChanges = $claimDifference->getQualifierChanges();
if ( $qualifierChanges !== null ) {
$html .= $this->visualizeQualifierChanges(
+ $headerPrefix,
$qualifierChanges,
$oldestMainSnak,
- $newestMainSnak,
- $path
+ $newestMainSnak
);
}
$referenceChanges = $claimDifference->getReferenceChanges();
if ( $referenceChanges !== null ) {
$html .= $this->visualizeReferenceChanges(
+ $headerPrefix,
$referenceChanges,
$oldestMainSnak,
- $newestMainSnak,
- $path
+ $newestMainSnak
);
}
@@ -132,22 +133,22 @@
}
/**
+ * @param string $headerPrefix
* @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
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
private function visualizeMainSnakChange(
+ $headerPrefix,
DiffOpChange $mainSnakChange,
Snak $oldestMainSnak,
- Snak $newestMainSnak,
- array $path
+ Snak $newestMainSnak
) {
$valueFormatter = new DiffOpValueFormatter(
- $this->snakVisualizer->getPropertyHeader(
$oldestMainSnak, $path ),
- $this->snakVisualizer->getPropertyHeader(
$newestMainSnak, $path ),
+ $headerPrefix .
$this->snakVisualizer->getPropertyHeader( $oldestMainSnak ),
+ $headerPrefix .
$this->snakVisualizer->getPropertyHeader( $newestMainSnak ),
// TODO: How to highlight the actual changes inside the
snak?
$this->snakVisualizer->getDetailedValue(
$mainSnakChange->getOldValue() ),
$this->snakVisualizer->getDetailedValue(
$mainSnakChange->getNewValue() )
@@ -157,27 +158,27 @@
}
/**
+ * @param string $headerPrefix
* @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
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
private function visualizeRankChange(
+ $headerPrefix,
DiffOpChange $rankChange,
Snak $oldestMainSnak,
- Snak $newestMainSnak,
- array $path
+ Snak $newestMainSnak
) {
$msg = wfMessage( 'wikibase-diffview-rank' )->inLanguage(
$this->languageCode );
$header = $msg->parse();
$valueFormatter = new DiffOpValueFormatter(
- $this->snakVisualizer->getPropertyAndValueHeader(
$oldestMainSnak, $path ) . ' / ' .
- $header,
- $this->snakVisualizer->getPropertyAndValueHeader(
$newestMainSnak, $path ) . ' / ' .
- $header,
+ $headerPrefix .
$this->snakVisualizer->getPropertyAndValueHeader( $oldestMainSnak )
+ . ' / ' . $header,
+ $headerPrefix .
$this->snakVisualizer->getPropertyAndValueHeader( $newestMainSnak )
+ . ' / ' . $header,
$this->getRankHtml( $rankChange->getOldValue() ),
$this->getRankHtml( $rankChange->getNewValue() )
);
@@ -209,27 +210,29 @@
}
/**
+ * @param string $headerPrefix
* @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
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
private function visualizeReferenceChanges(
+ $headerPrefix,
Diff $changes,
Snak $oldestMainSnak,
- Snak $newestMainSnak,
- array $path
+ Snak $newestMainSnak
) {
$html = '';
$msg = wfMessage( 'wikibase-diffview-reference' )->inLanguage(
$this->languageCode );
$header = $msg->parse();
- $oldClaimHeader =
$this->snakVisualizer->getPropertyAndValueHeader( $oldestMainSnak, $path )
+ $oldClaimHeader = $headerPrefix
+ . $this->snakVisualizer->getPropertyAndValueHeader(
$oldestMainSnak )
. ' / ' . $header;
- $newClaimHeader =
$this->snakVisualizer->getPropertyAndValueHeader( $newestMainSnak, $path )
+ $newClaimHeader = $headerPrefix
+ . $this->snakVisualizer->getPropertyAndValueHeader(
$newestMainSnak )
. ' / ' . $header;
foreach ( $changes as $change ) {
@@ -281,27 +284,29 @@
}
/**
+ * @param string $headerPrefix
* @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
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
private function visualizeQualifierChanges(
+ $headerPrefix,
Diff $changes,
Snak $oldestMainSnak,
- Snak $newestMainSnak,
- array $path
+ Snak $newestMainSnak
) {
$html = '';
$msg = wfMessage( 'wikibase-diffview-qualifier' )->inLanguage(
$this->languageCode );
$header = $msg->parse();
- $oldClaimHeader =
$this->snakVisualizer->getPropertyAndValueHeader( $oldestMainSnak, $path )
+ $oldClaimHeader = $headerPrefix
+ . $this->snakVisualizer->getPropertyAndValueHeader(
$oldestMainSnak )
. ' / ' . $header;
- $newClaimHeader =
$this->snakVisualizer->getPropertyAndValueHeader( $newestMainSnak, $path )
+ $newClaimHeader = $headerPrefix
+ . $this->snakVisualizer->getPropertyAndValueHeader(
$newestMainSnak )
. ' / ' . $header;
foreach ( $changes as $change ) {
diff --git a/repo/includes/Diff/DifferencesSnakVisualizer.php
b/repo/includes/Diff/DifferencesSnakVisualizer.php
index e537181..22aad73 100644
--- a/repo/includes/Diff/DifferencesSnakVisualizer.php
+++ b/repo/includes/Diff/DifferencesSnakVisualizer.php
@@ -110,16 +110,11 @@
* Get formatted header for a snak, including the snak's property
label, but not the snak's value.
*
* @param Snak|null $snak
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
- public function getPropertyHeader( Snak $snak = null, array $path = []
) {
- $headerText = '';
- if ( $path !== [] ) {
- $headerText = implode( ' / ', $path ) . ' / ';
- }
- $headerText .= wfMessage( 'wikibase-entity-property' )
+ public function getPropertyHeader( Snak $snak = null ) {
+ $headerText = wfMessage( 'wikibase-entity-property' )
->inLanguage( $this->languageCode )->escaped();
if ( $snak !== null ) {
@@ -134,12 +129,11 @@
* Get formatted header for a snak, including the snak's property label
and value.
*
* @param Snak $snak
- * @param string[] $path The path to prepend in the header
*
* @return string HTML
*/
- public function getPropertyAndValueHeader( Snak $snak, array $path = []
) {
- $before = $this->getPropertyHeader( $snak, $path );
+ public function getPropertyAndValueHeader( Snak $snak ) {
+ $before = $this->getPropertyHeader( $snak );
try {
$after = $this->snakBreadCrumbFormatter->formatSnak(
$snak );
diff --git a/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
b/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
index 60813b9..9f6de3f 100644
--- a/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
+++ b/repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
@@ -10,6 +10,7 @@
use Wikibase\DataModel\Snak\PropertyNoValueSnak;
use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use Wikibase\DataModel\Snak\PropertyValueSnak;
+use Wikibase\DataModel\Snak\Snak;
use Wikibase\Lib\SnakFormatter;
use Wikibase\Repo\Diff\DifferencesSnakVisualizer;
@@ -105,7 +106,6 @@
[ new PropertySomeValueSnak( new PropertyId( 'P1' ) ),
$expected ],
[ new PropertyNoValueSnak( new PropertyId( 'P1' ) ),
$expected ],
[ new PropertyValueSnak( new PropertyId( 'P1' ), new
StringValue( '' ) ), $expected ],
- //array( null, '' ),
];
}
@@ -131,40 +131,38 @@
/**
* @dataProvider provideGetPropertyAndValueHeader
*/
- public function testGetPropertyAndValueHeader( $snak, $expected, array
$path ) {
+ public function testGetPropertyAndValueHeader( $snak, $expected ) {
$snakVisualizer = $this->newDifferencesSnakVisualizer();
- $result = $snakVisualizer->getPropertyAndValueHeader( $snak,
$path );
+ $result = $snakVisualizer->getPropertyAndValueHeader( $snak );
$this->assertEquals( $expected, $result );
}
public function provideGetPropertyAndValueHeader() {
- $expected = 'foo / bar / property / <a>PID</a>: <i>SNAK</i>';
- $path = [ 'foo', 'bar' ];
+ $expected = 'property / <a>PID</a>: <i>SNAK</i>';
return [
- [ new PropertySomeValueSnak( new PropertyId( 'P1' ) ),
$expected, $path ],
- [ new PropertyNoValueSnak( new PropertyId( 'P1' ) ),
$expected, $path ],
- [ new PropertyValueSnak( new PropertyId( 'P1' ), new
StringValue( '' ) ), $expected, $path ],
+ [ new PropertySomeValueSnak( new PropertyId( 'P1' ) ),
$expected ],
+ [ new PropertyNoValueSnak( new PropertyId( 'P1' ) ),
$expected ],
+ [ new PropertyValueSnak( new PropertyId( 'P1' ), new
StringValue( '' ) ), $expected ],
];
}
/**
* @dataProvider provideGetPropertyHeader
*/
- public function testGetPropertyHeader( $snak, $expected, array $path ) {
+ public function testGetPropertyHeader( Snak $snak = null, $expected ) {
$snakVisualizer = $this->newDifferencesSnakVisualizer();
- $result = $snakVisualizer->getPropertyHeader( $snak, $path );
+ $result = $snakVisualizer->getPropertyHeader( $snak );
$this->assertEquals( $expected, $result );
}
public function provideGetPropertyHeader() {
- $expected = 'foo / bar / property / <a>PID</a>';
- $path = [ 'foo', 'bar' ];
+ $expected = 'property / <a>PID</a>';
return [
- [ new PropertySomeValueSnak( new PropertyId( 'P1' ) ),
$expected, $path ],
- [ new PropertyNoValueSnak( new PropertyId( 'P1' ) ),
$expected, $path ],
- [ new PropertyValueSnak( new PropertyId( 'P1' ), new
StringValue( '' ) ), $expected, $path ],
- [ null, 'foo / bar / property', $path ],
- [ null, 'property', [] ],
+ [ new PropertySomeValueSnak( new PropertyId( 'P1' ) ),
$expected ],
+ [ new PropertyNoValueSnak( new PropertyId( 'P1' ) ),
$expected ],
+ [ new PropertyValueSnak( new PropertyId( 'P1' ), new
StringValue( '' ) ), $expected ],
+ [ null, 'property' ],
+ [ null, 'property' ],
];
}
--
To view, visit https://gerrit.wikimedia.org/r/403963
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0408672f42b9822f0fe380d8f692b15a7d64f73d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
