Aude has submitted this change and it was merged.
Change subject: (bug 46075) diff view support for multiline references
......................................................................
(bug 46075) diff view support for multiline references
- when we have multiline references the whole snaklist will be
shown in diff view
patchset 4: added @since tag
patchset 5: changed seperator in property-value pairs from "/" to ":"
Change-Id: I6c25d2aa6761bb3f6ffe0107c41c643617ced979
---
M lib/includes/ClaimDifferenceVisualizer.php
M lib/includes/DiffOpValueFormatter.php
2 files changed, 63 insertions(+), 55 deletions(-)
Approvals:
Aude: Verified; Looks good to me, approved
jenkins-bot: Checked
diff --git a/lib/includes/ClaimDifferenceVisualizer.php
b/lib/includes/ClaimDifferenceVisualizer.php
index 68a8553..1efbf93 100644
--- a/lib/includes/ClaimDifferenceVisualizer.php
+++ b/lib/includes/ClaimDifferenceVisualizer.php
@@ -212,27 +212,27 @@
}
/**
- * Get formatted SnakList
+ * Get formatted values of SnakList in an array
*
* @since 0.4
*
* @param SnakList $snakList
*
- * @return string
+ * @return string[]
*/
- protected function getSnakListHtml( $snakList ) {
- $html = '';
+ protected function getSnakListValues( $snakList ) {
+ $values = array();
foreach ( $snakList as $snak ) {
-
- if ( $html !== '' ) {
- $html .= Html::rawElement( 'br', array(), '' );
- }
- // @fixme
- $html .= $this->getMainSnakValue( $snak );
+ // TODO: change hardcoded ": " so something like
wfMessage( 'colon-separator' ),
+ // but this will require further refactoring as it
would add HTML which gets escaped
+ $values[] =
+ $this->getEntityLabel( $snak->getPropertyId() )
.
+ ': '.
+ $this->getMainSnakValue( $snak );
}
- return $html;
+ return $values;
}
/**
@@ -276,7 +276,6 @@
return $diffValueString;
} else {
- // TODO: should be differenced visually (e.g. italic)
return $snakType;
}
}
@@ -330,22 +329,19 @@
// @todo assert that both reference changes refer to the same
reference
foreach( $referenceChanges as $referenceChange ) {
if ( $referenceChange instanceof \Diff\DiffOpAdd ) {
- $header = $this->getRefHeader(
$referenceChange->getNewValue() );
- $newRef = $this->getRefHtml(
$referenceChange->getNewValue() );
+ $newRef = $this->getSnakListValues(
$referenceChange->getNewValue()->getSnaks() );
} else if ( $referenceChange instanceof
\Diff\DiffOpRemove ) {
- $header = $this->getRefHeader(
$referenceChange->getOldValue() );
- $oldRef = $this->getRefHtml(
$referenceChange->getOldValue() );
+ $oldRef = $this->getSnakListValues(
$referenceChange->getOldValue()->getSnaks() );
} else if ( $referenceChange instanceof
\Diff\DiffOpChange ) {
- $header = $this->getRefHeader(
$referenceChange->getNewValue() );
- $oldRef = $this->getRefHtml(
$referenceChange->getOldValue() );
- $newRef = $this->getRefHtml(
$referenceChange->getNewValue() );
+ $oldRef = $this->getSnakListValues(
$referenceChange->getOldValue()->getSnaks() );
+ $newRef = $this->getSnakListValues(
$referenceChange->getNewValue()->getSnaks() );
} else {
// something went wrong, never should happen
throw new \MWException( 'There are no
references in the reference change.' );
}
$valueFormatter = new DiffOpValueFormatter(
- $claimHeader . ' / ' . wfMessage(
'wikibase-diffview-reference' ) . ' / ' . $header,
+ $claimHeader . ' / ' . wfMessage(
'wikibase-diffview-reference' ),
$oldRef,
$newRef
);
@@ -357,19 +353,6 @@
}
return $html;
- }
-
- /**
- * Format reference change
- *
- * @since 0.4
- *
- * @param $ref Reference
- *
- * @return string
- */
- protected function getRefHtml( $ref ) {
- return $this->getSnakListHtml( $ref->getSnaks() );
}
/**
diff --git a/lib/includes/DiffOpValueFormatter.php
b/lib/includes/DiffOpValueFormatter.php
index 1b6cd62..0f8dcb8 100644
--- a/lib/includes/DiffOpValueFormatter.php
+++ b/lib/includes/DiffOpValueFormatter.php
@@ -44,16 +44,16 @@
/**
* @since 0.4
*
- * @var string
+ * @var string|string[]
*/
- protected $oldValue;
+ protected $oldValues;
/**
* @since 0.4
*
- * @var string
+ * @var string|string[]
*/
- protected $newValue;
+ protected $newValues;
/**
* Constructor.
@@ -61,13 +61,13 @@
* @since 0.4
*
* @param string $name
- * @param string $oldValue
- * @param string $newValue
+ * @param string|string[] $oldValues
+ * @param string|string[] $newValues
*/
- public function __construct( $name, $oldValue, $newValue ) {
+ public function __construct( $name, $oldValues, $newValues ) {
$this->name = $name;
- $this->oldValue = $oldValue;
- $this->newValue = $newValue;
+ $this->oldValues = $oldValues;
+ $this->newValues = $newValues;
}
/**
@@ -78,8 +78,8 @@
* @return string
*/
protected function generateHeaderHtml() {
- $oldHeader = is_string( $this->oldValue ) ? $this->name : '';
- $newHeader = is_string( $this->newValue ) ? $this->name : '';
+ $oldHeader = is_array( $this->oldValues ) || is_string(
$this->oldValues ) ? $this->name : '';
+ $newHeader = is_array( $this->newValues ) || is_string(
$this->newValues ) ? $this->name : '';
$html = Html::openElement( 'tr' );
$html .= Html::element( 'td', array( 'colspan'=>'2', 'class' =>
'diff-lineno' ), $oldHeader );
@@ -101,13 +101,13 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-marker' ), '-' );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-deletedline' ),
Html::rawElement( 'div', array(),
- Html::element( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->oldValue ) ) );
+ Html::rawElement( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
+ $this->generateSafeValueHtml(
$this->oldValues ) ) ) );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-marker' ), '+' );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-addedline' ),
Html::rawElement( 'div', array(),
- Html::element( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->newValue ) ) );
+ Html::rawElement( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
+ $this->generateSafeValueHtml(
$this->newValues ) ) ) );
$html .= Html::closeElement( 'tr' );
$html .= Html::closeElement( 'tr' );
@@ -127,8 +127,8 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-marker' ), '+' );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-addedline' ),
Html::rawElement( 'div', array(),
- Html::element( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->newValue )
+ Html::rawElement( 'ins', array( 'class' =>
'diffchange diffchange-inline' ),
+ $this->generateSafeValueHtml(
$this->newValues ) )
)
);
$html .= Html::closeElement( 'tr' );
@@ -148,10 +148,34 @@
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-marker' ), '-' );
$html .= Html::rawElement( 'td', array( 'class' =>
'diff-deletedline' ),
Html::rawElement( 'div', array(),
- Html::element( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
- $this->oldValue ) ) );
+ Html::rawElement( 'del', array( 'class' =>
'diffchange diffchange-inline' ),
+ $this->generateSafeValueHtml(
$this->oldValues ) ) ) );
$html .= Html::rawElement( 'td', array( 'colspan'=>'2' ),
' ' );
$html .= Html::closeElement( 'tr' );
+
+ return $html;
+ }
+
+ /**
+ * Generates safe HTML from a given value or array of values
+ *
+ * @since 0.4
+ *
+ * @param string|string[] $values
+ *
+ * @return string
+ */
+ protected function generateSafeValueHtml( $values ) {
+ if ( is_string( $values ) ) {
+ return Html::Element( 'span', array(), $values );
+ }
+ $html = '';
+ foreach ( $values as $value ) {
+ if ( $html !== '' ) {
+ $html .= Html::rawElement( 'br', array(), '' );
+ }
+ $html .= Html::Element( 'span', array(), $value );
+ }
return $html;
}
@@ -170,11 +194,12 @@
$html .= $this->generateHeaderHtml();
}
- if ( is_string( $this->oldValue ) && is_string( $this->newValue
) ) {
+ if ( is_array( $this->oldValues ) && is_array( $this->newValues
)
+ || is_string( $this->oldValues ) && is_string(
$this->newValues ) ) {
$html .= $this->generateChangeOpHtml();
- } else if ( is_string( $this->newValue ) ) {
+ } else if ( is_array( $this->newValues ) || is_string(
$this->newValues ) ) {
$html .= $this->generateAddOpHtml();
- } else if ( is_string( $this->oldValue ) ) {
+ } else if ( is_array( $this->oldValues ) || is_string(
$this->oldValues ) ) {
$html .= $this->generateRemoveOpHtml();
}
--
To view, visit https://gerrit.wikimedia.org/r/53974
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6c25d2aa6761bb3f6ffe0107c41c643617ced979
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Daniel Werner <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
