jenkins-bot has submitted this change and it was merged.
Change subject: Refactor and simplify time formatter classes
......................................................................
Refactor and simplify time formatter classes
This is pure refactoring that does not change any behavior. The goal
is to make following patches easier to review.
* Make $options optional in 2 constructors.
* Drop duplicate code from MwTimeIsoFormatter constructor.
* Drop not needed class variable from TimeDetailsFormatter.
* Simplify TimeDetailsFormatter constructor a lot (all this hasOption
code is exactly what defaultOption does).
* Move calendar model formatting to private method.
* Drop not needed options from tests. English is the default anyway.
* Use constants instead of "10" in tests.
* Split long lines.
Change-Id: Iab1782a7fe1193cef2c186a9d4a2a2d6a4fa3a6d
---
M lib/includes/formatters/MwTimeIsoFormatter.php
M lib/includes/formatters/TimeDetailsFormatter.php
M lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
3 files changed, 60 insertions(+), 52 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 449abe5..5e3524d 100644
--- a/lib/includes/formatters/MwTimeIsoFormatter.php
+++ b/lib/includes/formatters/MwTimeIsoFormatter.php
@@ -28,12 +28,10 @@
private $language;
/**
- * @param FormatterOptions $options
+ * @param FormatterOptions|null $options
*/
- public function __construct( FormatterOptions $options ) {
- $this->options = $options;
-
- $this->options->defaultOption( ValueFormatter::OPT_LANG, 'en' );
+ public function __construct( FormatterOptions $options = null ) {
+ parent::__construct( $options );
$this->language = Language::factory(
$this->options->getOption( ValueFormatter::OPT_LANG )
diff --git a/lib/includes/formatters/TimeDetailsFormatter.php
b/lib/includes/formatters/TimeDetailsFormatter.php
index 008d37b..20fb536 100644
--- a/lib/includes/formatters/TimeDetailsFormatter.php
+++ b/lib/includes/formatters/TimeDetailsFormatter.php
@@ -24,34 +24,27 @@
const OPT_CALENDARNAMES = 'calendars';
/**
- * @var MwTimeIsoFormatter
- */
- private $isoTimeFormatter;
-
- /**
* @var TimeFormatter
*/
private $timeFormatter;
/**
- * @param FormatterOptions $options
+ * @param FormatterOptions|null $options
*/
- public function __construct( FormatterOptions $options ) {
+ public function __construct( FormatterOptions $options = null ) {
parent::__construct( $options );
- if ( $options->hasOption( TimeFormatter::OPT_TIME_ISO_FORMATTER
) ) {
- $this->isoTimeFormatter = $options->getOption(
TimeFormatter::OPT_TIME_ISO_FORMATTER );
- } else {
- $this->isoTimeFormatter = new MwTimeIsoFormatter(
$options );
- $options->setOption(
TimeFormatter::OPT_TIME_ISO_FORMATTER, $this->isoTimeFormatter );
- }
-
- $this->timeFormatter = new TimeFormatter( $options );
+ $this->defaultOption(
+ TimeFormatter::OPT_TIME_ISO_FORMATTER,
+ new MwTimeIsoFormatter( $this->options )
+ );
$this->defaultOption( self::OPT_CALENDARNAMES, array(
TimeFormatter::CALENDAR_GREGORIAN => 'Gregorian',
TimeFormatter::CALENDAR_JULIAN => 'Julian',
) );
+
+ $this->timeFormatter = new TimeFormatter( $this->options );
}
/**
@@ -70,34 +63,39 @@
throw new InvalidArgumentException( 'Data value type
mismatch. Expected an TimeValue.' );
}
- $calendarModel = $value->getCalendarModel();
- $calendarNames = $this->getOption( self::OPT_CALENDARNAMES );
- if ( array_key_exists( $calendarModel, $calendarNames ) ) {
- $calendarModel = $calendarNames[$calendarModel];
- }
-
$html = '';
- $html .= Html::element( 'h4',
+ $html .= Html::element(
+ 'h4',
array( 'class' => 'wb-details wb-time-details
wb-time-rendered' ),
$this->timeFormatter->format( $value )
);
+ $html .= Html::openElement( 'table', array( 'class' =>
'wb-details wb-time-details' ) );
- $html .= Html::openElement( 'table',
- array( 'class' => 'wb-details wb-time-details' ) );
- $html .= $this->renderLabelValuePair( 'isotime',
htmlspecialchars( $value->getTime() ) );
-
- //TODO: provide "nice" rendering of timezone, calendar,
precision, etc.
+ $html .= $this->renderLabelValuePair(
+ 'isotime',
+ htmlspecialchars( $value->getTime() )
+ );
$html .= $this->renderLabelValuePair(
'timezone',
- $this->formatTimezone( $value->getTimezone() )
+ $this->getTimezoneHtml( $value->getTimezone() )
);
- $html .= $this->renderLabelValuePair( 'calendar',
- htmlspecialchars( $calendarModel ) );
- $html .= $this->renderLabelValuePair( 'precision',
- htmlspecialchars( $value->getPrecision() ) );
-
- $html .= $this->renderLabelValuePair( 'before',
htmlspecialchars( $value->getBefore() ) );
- $html .= $this->renderLabelValuePair( 'after',
htmlspecialchars( $value->getAfter() ) );
+ $html .= $this->renderLabelValuePair(
+ 'calendar',
+ $this->getCalendarModelHtml( $value->getCalendarModel()
)
+ );
+ // TODO: Provide "nice" rendering of precision, etc.
+ $html .= $this->renderLabelValuePair(
+ 'precision',
+ htmlspecialchars( $value->getPrecision() )
+ );
+ $html .= $this->renderLabelValuePair(
+ 'before',
+ htmlspecialchars( $value->getBefore() )
+ );
+ $html .= $this->renderLabelValuePair(
+ 'after',
+ htmlspecialchars( $value->getAfter() )
+ );
$html .= Html::closeElement( 'table' );
@@ -109,7 +107,7 @@
*
* @return string
*/
- private function formatTimezone( $timezone ) {
+ private function getTimezoneHtml( $timezone ) {
$sign = $timezone < 0 ? "\xE2\x88\x92" : '+';
$hour = floor( abs( $timezone ) / 60 );
$minute = abs( $timezone ) - $hour * 60;
@@ -117,6 +115,20 @@
}
/**
+ * @param string $calendarModel
+ *
+ * @return string
+ */
+ private function getCalendarModelHtml( $calendarModel ) {
+ $calendarNames = $this->getOption( self::OPT_CALENDARNAMES );
+ if ( array_key_exists( $calendarModel, $calendarNames ) ) {
+ $calendarModel = $calendarNames[$calendarModel];
+ }
+
+ return htmlspecialchars( $calendarModel );
+ }
+
+ /**
* @param string $fieldName
* @param string $valueHtml
*
diff --git a/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
b/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
index 9a350d5..144e39a 100644
--- a/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
+++ b/lib/tests/phpunit/formatters/TimeDetailsFormatterTest.php
@@ -24,23 +24,23 @@
/**
* @dataProvider quantityFormatProvider
+ * @param TimeValue $value
+ * @param string $pattern
*/
- public function testFormat( $value, $options, $pattern ) {
- $formatter = new TimeDetailsFormatter( $options );
+ public function testFormat( TimeValue $value, $pattern ) {
+ $formatter = new TimeDetailsFormatter( new FormatterOptions() );
$html = $formatter->format( $value );
$this->assertRegExp( $pattern, $html );
}
public function quantityFormatProvider() {
- $options = new FormatterOptions( array(
- ValueFormatter::OPT_LANG => 'en'
- ) );
+ $gregorian = TimeFormatter::CALENDAR_GREGORIAN;
+ $day = TimeValue::PRECISION_DAY;
return array(
array(
- new TimeValue( '+2001-01-01T00:00:00Z', 60, 0,
1, 10, TimeFormatter::CALENDAR_GREGORIAN ),
- $options,
+ new TimeValue( '+2001-01-01T00:00:00Z', 60, 0,
1, TimeValue::PRECISION_MONTH, $gregorian ),
'@' . implode( '.*',
array(
'<h4[^<>]*>[^<>]*2001[^<>]*</h4>',
@@ -54,13 +54,11 @@
) . '@s'
),
array(
- new TimeValue( '+2001-01-01T00:00:00Z', 60, 0,
1, 10, 'Stardate' ),
- $options,
+ new TimeValue( '+2001-01-01T00:00:00Z', 0, 0,
0, $day, 'Stardate' ),
'@.*<td
class="wb-time-calendar">Stardate</td>.*@s'
),
array(
- new TimeValue( '+2001-01-01T00:00:00Z', -179,
0, 1, 10, TimeFormatter::CALENDAR_GREGORIAN ),
- $options,
+ new TimeValue( '+2001-01-01T00:00:00Z', -179,
0, 0, $day, $gregorian ),
'@.*<td
class="wb-time-timezone">\xE2\x88\x9202:59</td>.*@s'
),
);
--
To view, visit https://gerrit.wikimedia.org/r/188541
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iab1782a7fe1193cef2c186a9d4a2a2d6a4fa3a6d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: Addshore <[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