Thiemo Mättig (WMDE) has uploaded a new change for review.
https://gerrit.wikimedia.org/r/248900
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(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/00/248900/1
diff --git a/lib/includes/formatters/MwTimeIsoFormatter.php
b/lib/includes/formatters/MwTimeIsoFormatter.php
index ba2fb57..9eaaafd 100644
--- a/lib/includes/formatters/MwTimeIsoFormatter.php
+++ b/lib/includes/formatters/MwTimeIsoFormatter.php
@@ -75,13 +75,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 );
@@ -102,17 +102,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' );
}
}
@@ -197,7 +198,6 @@
* @param string $isoTimestamp
* @param int $precision
*
- * @throws InvalidArgumentException
* @return string
*/
private function getLocalizedYear( $isoTimestamp, $precision ) {
@@ -205,8 +205,8 @@
list( , $sign, $year ) = $matches;
$isBCE = $sign === '-';
- $shift = 1e+0;
- $unshift = 1e+0;
+ $shift = 1;
+ $unshift = 1;
$func = 'round';
switch ( $precision ) {
@@ -256,28 +256,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;
}
if ( $precision >= TimeValue::PRECISION_YEAR ) {
@@ -296,6 +284,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 9985061..46ef180 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,
'0001 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: newchange
Gerrit-Change-Id: If5974e808f78a5fbac82f0eeb8d825509c9495a9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
