jenkins-bot has submitted this change and it was merged.
Change subject: Split code and fix precision edge case in MwTimeIsoFormatter
......................................................................
Split code and fix precision edge case in MwTimeIsoFormatter
Meant as a clean up of existing code. I started this patch with
splitting the big function.
I found an issue where this formatter incorrectly formats values
with hour, minute or second precision as a day. The issue with this
is that you can not even see the hour, minute or second. It's
supressed. This patch introduces a fallback to the full timestamp.
Change-Id: If5974e808f78a5fbac82f0eeb8d825509c9495a9
---
M lib/includes/formatters/MwTimeIsoFormatter.php
M lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
2 files changed, 50 insertions(+), 31 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/lib/includes/formatters/MwTimeIsoFormatter.php
b/lib/includes/formatters/MwTimeIsoFormatter.php
index bc969a2..f89e3b6 100644
--- a/lib/includes/formatters/MwTimeIsoFormatter.php
+++ b/lib/includes/formatters/MwTimeIsoFormatter.php
@@ -76,13 +76,13 @@
* @return string Formatted date
*/
private function getLocalizedDate( $isoTimestamp, $precision ) {
- $dateFormat = $this->getDateFormat( $precision );
$localizedYear = $this->getLocalizedYear( $isoTimestamp,
$precision );
- if ( $dateFormat === 'Y' ) {
+ if ( $precision <= TimeValue::PRECISION_YEAR ) {
return $localizedYear;
}
+ $dateFormat = $this->getDateFormat( $precision );
$mwTimestamp = $this->getMwTimestamp( $isoTimestamp, $precision
);
$mwYear = $this->language->sprintfDate( 'Y', $mwTimestamp );
$localizedDate = $this->language->sprintfDate( $dateFormat,
$mwTimestamp );
@@ -103,17 +103,18 @@
/**
* @param int $precision
*
+ * @throws InvalidArgumentException
* @return string Date format string to be used by Language::sprintfDate
*/
private function getDateFormat( $precision ) {
- if ( $precision <= TimeValue::PRECISION_YEAR ) {
- return 'Y';
- } elseif ( $precision === TimeValue::PRECISION_MONTH ) {
+ if ( $precision === TimeValue::PRECISION_MONTH ) {
$format = $this->language->getDateFormatString(
'monthonly', 'dmy' );
return sprintf( '%s Y', $this->getMonthFormat( $format
) );
- } else {
+ } elseif ( $precision === TimeValue::PRECISION_DAY ) {
$format = $this->language->getDateFormatString( 'date',
'dmy' );
return sprintf( '%s %s Y', $this->getDayFormat( $format
), $this->getMonthFormat( $format ) );
+ } else {
+ throw new InvalidArgumentException( 'Unsupported
precision' );
}
}
@@ -198,7 +199,6 @@
* @param string $isoTimestamp
* @param int $precision
*
- * @throws InvalidArgumentException
* @return string
*/
private function getLocalizedYear( $isoTimestamp, $precision ) {
@@ -206,8 +206,8 @@
list( , $sign, $year ) = $matches;
$isBCE = $sign === '-';
- $shift = 1e+0;
- $unshift = 1e+0;
+ $shift = 1;
+ $unshift = 1;
$func = 'round';
switch ( $precision ) {
@@ -257,28 +257,16 @@
break;
}
- if ( $shift !== 1e+0 || $unshift !== 1e+0 ) {
- switch ( $func ) {
- case 'ceil':
- $shifted = ceil( $year / $shift ) *
$unshift;
- break;
- case 'floor':
- $shifted = floor( $year / $shift ) *
$unshift;
- break;
- default:
- $shifted = round( $year / $shift ) *
$unshift;
- }
-
- // Year to small for precision, fall back to year
- if ( $shifted == 0
- && ( $precision < TimeValue::PRECISION_YEAR
- || ( $isBCE && $precision ===
TimeValue::PRECISION_YEAR )
- )
- ) {
- $msg = null;
- } else {
- $year = sprintf( '%.0f', $shifted );
- }
+ $shifted = $this->shiftNumber( $year, $func, $shift, $unshift );
+ if ( $shifted == 0
+ && ( $precision < TimeValue::PRECISION_YEAR
+ || ( $isBCE && $precision ===
TimeValue::PRECISION_YEAR )
+ )
+ ) {
+ // Year to small for precision, fall back to year.
+ $msg = null;
+ } else {
+ $year = $shifted;
}
$year = str_pad( ltrim( $year, '0' ), 1, '0', STR_PAD_LEFT );
@@ -295,6 +283,33 @@
}
/**
+ * @param string $number
+ * @param string $function
+ * @param float $shift
+ * @param float $unshift
+ *
+ * @return string
+ */
+ private function shiftNumber( $number, $function, $shift, $unshift ) {
+ if ( $shift == 1 && $unshift == 1 ) {
+ return $number;
+ }
+
+ switch ( $function ) {
+ case 'ceil':
+ $shifted = ceil( $number / $shift ) * $unshift;
+ break;
+ case 'floor':
+ $shifted = floor( $number / $shift ) * $unshift;
+ break;
+ default:
+ $shifted = round( $number / $shift ) * $unshift;
+ }
+
+ return sprintf( '%.0f', $shifted );
+ }
+
+ /**
* @param string $key
* @param string $param
*
diff --git a/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
b/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
index 0277353..a20ad81 100644
--- a/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
+++ b/lib/tests/phpunit/formatters/MwTimeIsoFormatterTest.php
@@ -439,6 +439,10 @@
'-1-00-00T00:00:00Z',
TimeValue::PRECISION_YEAR10,
'1 BCE',
),
+ array(
+ '+2015-01-01T01:00:00Z',
TimeValue::PRECISION_HOUR,
+ '+2015-01-01T01:00:00Z',
+ ),
// Better than the raw ISO string
array(
--
To view, visit https://gerrit.wikimedia.org/r/248900
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If5974e808f78a5fbac82f0eeb8d825509c9495a9
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits