Mailing List Archive

[MediaWiki-commits] [Gerrit] mediawiki...Wikibase[master]: Move path array prepending logic to ClaimDifferenceVisualizer
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) <thiemo.kreuz@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] mediawiki...Wikibase[master]: Move path array prepending logic to ClaimDifferenceVisualizer [ In reply to ]
jenkins-bot has submitted this change and it was merged. ( 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/ClaimDifferenceVisualizerTest.php
M repo/tests/phpunit/includes/Diff/DifferencesSnakVisualizerTest.php
4 files changed, 81 insertions(+), 93 deletions(-)

Approvals:
Ladsgroup: Looks good to me, approved
jenkins-bot: Verified
Thiemo Kreuz (WMDE): Looks good to me, approved



diff --git a/repo/includes/Diff/ClaimDifferenceVisualizer.php b/repo/includes/Diff/ClaimDifferenceVisualizer.php
index d75f9a6..1828309 100644
--- a/repo/includes/Diff/ClaimDifferenceVisualizer.php
+++ b/repo/includes/Diff/ClaimDifferenceVisualizer.php
@@ -55,6 +55,7 @@
Statement $baseStatement,
array $path = []
) {
+ $headerPrefix = $path ? 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/ClaimDifferenceVisualizerTest.php b/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
index 57bcbb2..6a06c8e 100644
--- a/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
+++ b/repo/tests/phpunit/includes/Diff/ClaimDifferenceVisualizerTest.php
@@ -40,30 +40,29 @@

$instance->expects( $this->any() )
->method( 'getPropertyAndDetailedValue' )
- ->will( $this->returnCallback( function( PropertyValueSnak $snak ) {
+ ->willReturnCallback( function( PropertyValueSnak $snak ) {
return $snak->getPropertyId()->getSerialization() . ': ' . $snak->getDataValue()->getValue()
. ' (DETAILED)';
- } ) );
+ } );

$instance->expects( $this->any() )
->method( 'getDetailedValue' )
- ->will( $this->returnCallback( function( PropertyValueSnak $snak = null ) {
+ ->willReturnCallback( function( PropertyValueSnak $snak = null ) {
return $snak === null ? null : $snak->getDataValue()->getValue() . ' (DETAILED)';
- } ) );
+ } );

$instance->expects( $this->any() )
->method( 'getPropertyHeader' )
- ->will( $this->returnCallback( function( Snak $snak, array $path ) {
- return implode( ' / ', $path ) . 'property / ' . $snak->getPropertyId()
- ->getSerialization();
- } ) );
+ ->willReturnCallback( function( Snak $snak ) {
+ return 'property / ' . $snak->getPropertyId()->getSerialization();
+ } );

$instance->expects( $this->any() )
->method( 'getPropertyAndValueHeader' )
- ->will( $this->returnCallback( function( PropertyValueSnak $snak, array $path ) {
- return implode( ' / ', $path ) . 'property / ' . $snak->getPropertyId()->getSerialization()
- . ': ' .$snak->getDataValue()->getValue();
- } ) );
+ ->willReturnCallback( function( PropertyValueSnak $snak ) {
+ return 'property / ' . $snak->getPropertyId()->getSerialization() . ': ' .
+ $snak->getDataValue()->getValue();
+ } );

return $instance;
}
@@ -89,7 +88,6 @@
new ClaimDifference(),
new Statement( new PropertyValueSnak( new PropertyId( 'P1' ), new StringValue( 'foo' ) ) ),
'',
- []
],
'mainsnak' => [.
new ClaimDifference(
@@ -105,7 +103,6 @@
'<div><del class="diffchange diffchange-inline"><span>bar (DETAILED)</span></del></div></td>'.
'<td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>foo (DETAILED)</span></ins></div></td></tr>',
- []
],
'+qualifiers' => [.
new ClaimDifference(
@@ -119,7 +116,6 @@
'<td colspan="2" class="diff-lineno">property / P1: foo / qualifier</td></tr>' .
'<tr><td colspan="2">&nbsp;</td><td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>P44: v (DETAILED)</span></ins></div></td></tr>',
- []
],
'+references' =>
[.
@@ -138,7 +134,6 @@
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>P50: v (DETAILED)</span>' .
'</del></div></td><td colspan="2">&nbsp;</td></tr>',
- []
],
'ranks' => [.
new ClaimDifference(
@@ -160,7 +155,6 @@
. '<ins class="diffchange diffchange-inline"><span>Preferred rank</span></ins>'
. '</div></td>'
. '</tr>',
- []
],
'mainsnak and qualifiers' => [.
new ClaimDifference(
@@ -212,7 +206,6 @@
. '<span>P44: oldqualifiervalue (DETAILED)</span></del></div></td>'
. '<td colspan="2">&nbsp;</td>'
. '</tr>',
- []
],
'mainsnak and references' => [.
new ClaimDifference(
@@ -265,7 +258,6 @@
. '<span>P44: oldreferencevalue (DETAILED)</span></del></div></td>'
. '<td colspan="2">&nbsp;</td>'
. '</tr>',
- []
],
'mainsnak and rank' => [.
new ClaimDifference(
@@ -293,7 +285,6 @@
'<div><del class="diffchange diffchange-inline"><span>Normal rank</span></del></div></td>'.
'<td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>Preferred rank</span></ins></div></td></tr>',
- []
],
'mainsnak and rank with path' => [.
new ClaimDifference(
@@ -307,15 +298,15 @@
),
new Statement( new PropertyValueSnak( new PropertyId( 'P1' ), new StringValue( 'newmainsnakvalue' ) ) ),
// mainsnak change
- '<tr><td colspan="2" class="diff-lineno">foo / barproperty / P1</td><td colspan="2" class="diff-lineno">'.
- 'foo / barproperty / P1</td></tr><tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
+ '<tr><td colspan="2" class="diff-lineno">foo / bar / property / P1</td><td colspan="2" class="diff-lineno">'.
+ 'foo / bar / property / P1</td></tr><tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>oldmainsnakvalue (DETAILED)</span></del></div></td>'.
'<td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>newmainsnakvalue (DETAILED)</span></ins></div></td></tr>'.
// rank change
'<tr>' .
- '<td colspan="2" class="diff-lineno">foo / barproperty / P1: oldmainsnakvalue / rank</td>' .
- '<td colspan="2" class="diff-lineno">foo / barproperty / P1: newmainsnakvalue / rank</td>' .
+ '<td colspan="2" class="diff-lineno">foo / bar / property / P1: oldmainsnakvalue / rank</td>' .
+ '<td colspan="2" class="diff-lineno">foo / bar / property / P1: newmainsnakvalue / rank</td>' .
'</tr>' .
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>Normal rank</span></del></div></td>'.
@@ -330,13 +321,13 @@
* @dataProvider provideDifferenceAndClaim
*/
public function testVisualizeClaimChange(
- $difference,
- $baseClaim,
+ ClaimDifference $difference,
+ Statement $baseStatement,
$expectedHtml,
- array $path
+ array $path = []
) {
$visualizer = $this->newClaimDifferenceVisualizer();
- $html = $visualizer->visualizeClaimChange( $difference, $baseClaim, $path );
+ $html = $visualizer->visualizeClaimChange( $difference, $baseStatement, $path );
$this->assertHTMLEquals( $expectedHtml, $html );
}

@@ -344,25 +335,25 @@
$expect =
// main snak
'<tr><td colspan="2" class="diff-lineno"></td>'.
- '<td colspan="2" class="diff-lineno">foooproperty / P12</td></tr>'.
+ '<td colspan="2" class="diff-lineno">fooo / property / P12</td></tr>'.
'<tr><td colspan="2">&nbsp;</td><td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>foo (DETAILED)</span></ins></div></td></tr>'.

// rank
'<tr><td colspan="2" class="diff-lineno"></td>'.
- '<td colspan="2" class="diff-lineno">foooproperty / P12: foo / rank</td></tr>'.
+ '<td colspan="2" class="diff-lineno">fooo / property / P12: foo / rank</td></tr>'.
'<tr><td colspan="2">&nbsp;</td><td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>Normal rank</span></ins></div></td></tr>'.

// qualifier
'<tr><td colspan="2" class="diff-lineno"></td>'.
- '<td colspan="2" class="diff-lineno">foooproperty / P12: foo / qualifier</td></tr>'.
+ '<td colspan="2" class="diff-lineno">fooo / property / P12: foo / qualifier</td></tr>'.
'<tr><td colspan="2">&nbsp;</td><td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>P50: v (DETAILED)</span></ins></div></td></tr>'.

// reference
'<tr><td colspan="2" class="diff-lineno"></td>'.
- '<td colspan="2" class="diff-lineno">foooproperty / P12: foo / reference</td></tr>'.
+ '<td colspan="2" class="diff-lineno">fooo / property / P12: foo / reference</td></tr>'.
'<tr><td colspan="2">&nbsp;</td><td class="diff-marker">+</td><td class="diff-addedline">'.
'<div><ins class="diffchange diffchange-inline"><span>P44: referencevalue (DETAILED)</span></ins></div></td></tr>';

@@ -383,28 +374,28 @@
public function testVisualizeRemovedClaim() {
$expect =
// main snak
- '<tr><td colspan="2" class="diff-lineno">barproperty / P12</td>'.
+ '<tr><td colspan="2" class="diff-lineno">bar / property / P12</td>'.
'<td colspan="2" class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>foo (DETAILED)</span></del></div>'.
'</td><td colspan="2">&nbsp;</td></tr>'.

// rank
- '<tr><td colspan="2" class="diff-lineno">barproperty / P12: foo / rank</td>'.
+ '<tr><td colspan="2" class="diff-lineno">bar / property / P12: foo / rank</td>'.
'<td colspan="2" class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>Normal rank</span></del></div>'
.'</td><td colspan="2">&nbsp;</td></tr>'.

// qualifier
- '<tr><td colspan="2" class="diff-lineno">barproperty / P12: foo / qualifier</td>'.
+ '<tr><td colspan="2" class="diff-lineno">bar / property / P12: foo / qualifier</td>'.
'<td colspan="2" class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>P50: v (DETAILED)</span></del></div>'.
'</td><td colspan="2">&nbsp;</td></tr>'.

// reference
- '<tr><td colspan="2" class="diff-lineno">barproperty / P12: foo / reference</td>'.
+ '<tr><td colspan="2" class="diff-lineno">bar / property / P12: foo / reference</td>'.
'<td colspan="2" class="diff-lineno"></td></tr>'.
'<tr><td class="diff-marker">-</td><td class="diff-deletedline">'.
'<div><del class="diffchange diffchange-inline"><span>P44: referencevalue (DETAILED)</span></del></div>'.
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: merged
Gerrit-Change-Id: I0408672f42b9822f0fe380d8f692b15a7d64f73d
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <thiemo.kreuz@wikimedia.de>
Gerrit-Reviewer: Ladsgroup <Ladsgroup@gmail.com>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <thiemo.kreuz@wikimedia.de>
Gerrit-Reviewer: WMDE-leszek <leszek.manicki@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits