r98006 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98005‎ | r98006 | r98007 >
Date:15:44, 24 September 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Per CR on r97962, introduce an array parameter for formatTimePeriod() rather than adding more boolean params.
Modified paths:
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/tests/phpunit/languages/LanguageTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/languages/LanguageTest.php
——@@ -21,42 +21,42 @@
2222 }
2323
2424 /** @dataProvider provideFormattableTimes */
25 - function testFormatTimePeriod( $seconds, $avoid, $noAbbrevs, $expected, $desc ) {
26 - $this->assertEquals( $expected, $this->lang->formatTimePeriod( $seconds, $avoid, $noAbbrevs ), $desc );
 25+ function testFormatTimePeriod( $seconds, $format, $expected, $desc ) {
 26+ $this->assertEquals( $expected, $this->lang->formatTimePeriod( $seconds, $format ), $desc );
2727 }
2828
2929 function provideFormattableTimes() {
3030 return array(
31 - array( 9.45, false, false, '9.5s', 'formatTimePeriod() rounding (<10s)' ),
32 - array( 9.45, false, 'noabbrevs', '9.5 seconds', 'formatTimePeriod() rounding (<10s)' ),
33 - array( 9.95, false, false, '10s', 'formatTimePeriod() rounding (<10s)' ),
34 - array( 9.95, false, 'noabbrevs', '10 seconds', 'formatTimePeriod() rounding (<10s)' ),
35 - array( 59.55, false, false, '1m 0s', 'formatTimePeriod() rounding (<60s)' ),
36 - array( 59.55, false, 'noabbrevs', '1 minute 0 seconds', 'formatTimePeriod() rounding (<60s)' ),
37 - array( 119.55, false, false, '2m 0s', 'formatTimePeriod() rounding (<1h)' ),
38 - array( 119.55, false, 'noabbrevs', '2 minutes 0 seconds', 'formatTimePeriod() rounding (<1h)' ),
39 - array( 3599.55, false, false, '1h 0m 0s', 'formatTimePeriod() rounding (<1h)' ),
40 - array( 3599.55, false, 'noabbrevs', '1 hour 0 minutes 0 seconds', 'formatTimePeriod() rounding (<1h)' ),
41 - array( 7199.55, false, false, '2h 0m 0s', 'formatTimePeriod() rounding (>=1h)' ),
42 - array( 7199.55, false, 'noabbrevs', '2 hours 0 minutes 0 seconds', 'formatTimePeriod() rounding (>=1h)' ),
43 - array( 7199.55, 'avoidseconds', false, '2h 0m', 'formatTimePeriod() rounding (>=1h), avoidseconds' ),
44 - array( 7199.55, 'avoidseconds', 'noabbrevs', '2 hours 0 minutes', 'formatTimePeriod() rounding (>=1h), avoidseconds' ),
45 - array( 7199.55, 'avoidminutes', false, '2h 0m', 'formatTimePeriod() rounding (>=1h), avoidminutes' ),
46 - array( 7199.55, 'avoidminutes', 'noabbrevs', '2 hours 0 minutes', 'formatTimePeriod() rounding (>=1h), avoidminutes' ),
47 - array( 172799.55, 'avoidseconds', false, '48h 0m', 'formatTimePeriod() rounding (=48h), avoidseconds' ),
48 - array( 172799.55, 'avoidseconds', 'noabbrevs', '48 hours 0 minutes', 'formatTimePeriod() rounding (=48h), avoidseconds' ),
49 - array( 259199.55, 'avoidminutes', false, '3d 0h', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
50 - array( 259199.55, 'avoidminutes', 'noabbrevs', '3 days 0 hours', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
51 - array( 176399.55, 'avoidseconds', false, '2d 1h 0m', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
52 - array( 176399.55, 'avoidseconds', 'noabbrevs', '2 days 1 hour 0 minutes', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
53 - array( 176399.55, 'avoidminutes', false, '2d 1h', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
54 - array( 176399.55, 'avoidminutes', 'noabbrevs', '2 days 1 hour', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
55 - array( 259199.55, 'avoidseconds', false, '3d 0h 0m', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
56 - array( 259199.55, 'avoidseconds', 'noabbrevs', '3 days 0 hours 0 minutes', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
57 - array( 172801.55, 'avoidseconds', false, '2d 0h 0m', 'formatTimePeriod() rounding, (>48h), avoidseconds' ),
58 - array( 172801.55, 'avoidseconds', 'noabbrevs', '2 days 0 hours 0 minutes', 'formatTimePeriod() rounding, (>48h), avoidseconds' ),
59 - array( 176460.55, false, false, '2d 1h 1m 1s', 'formatTimePeriod() rounding, recursion, (>48h)' ),
60 - array( 176460.55, false, 'noabbrevs', '2 days 1 hour 1 minute 1 second', 'formatTimePeriod() rounding, recursion, (>48h)' ),
 31+ array( 9.45, array(), '9.5s', 'formatTimePeriod() rounding (<10s)' ),
 32+ array( 9.45, array( 'noabbrevs' => true ), '9.5 seconds', 'formatTimePeriod() rounding (<10s)' ),
 33+ array( 9.95, array(), '10s', 'formatTimePeriod() rounding (<10s)' ),
 34+ array( 9.95, array( 'noabbrevs' => true ), '10 seconds', 'formatTimePeriod() rounding (<10s)' ),
 35+ array( 59.55, array(), '1m 0s', 'formatTimePeriod() rounding (<60s)' ),
 36+ array( 59.55, array( 'noabbrevs' => true ), '1 minute 0 seconds', 'formatTimePeriod() rounding (<60s)' ),
 37+ array( 119.55, array(), '2m 0s', 'formatTimePeriod() rounding (<1h)' ),
 38+ array( 119.55, array( 'noabbrevs' => true ), '2 minutes 0 seconds', 'formatTimePeriod() rounding (<1h)' ),
 39+ array( 3599.55, array(), '1h 0m 0s', 'formatTimePeriod() rounding (<1h)' ),
 40+ array( 3599.55, array( 'noabbrevs' => true ), '1 hour 0 minutes 0 seconds', 'formatTimePeriod() rounding (<1h)' ),
 41+ array( 7199.55, array(), '2h 0m 0s', 'formatTimePeriod() rounding (>=1h)' ),
 42+ array( 7199.55, array( 'noabbrevs' => true ), '2 hours 0 minutes 0 seconds', 'formatTimePeriod() rounding (>=1h)' ),
 43+ array( 7199.55, 'avoidseconds', '2h 0m', 'formatTimePeriod() rounding (>=1h), avoidseconds' ),
 44+ array( 7199.55, array( 'avoid' => 'avoidseconds', 'noabbrevs' => true ), '2 hours 0 minutes', 'formatTimePeriod() rounding (>=1h), avoidseconds' ),
 45+ array( 7199.55, 'avoidminutes', '2h 0m', 'formatTimePeriod() rounding (>=1h), avoidminutes' ),
 46+ array( 7199.55, array( 'avoid' => 'avoidminutes', 'noabbrevs' => true ), '2 hours 0 minutes', 'formatTimePeriod() rounding (>=1h), avoidminutes' ),
 47+ array( 172799.55, 'avoidseconds', '48h 0m', 'formatTimePeriod() rounding (=48h), avoidseconds' ),
 48+ array( 172799.55, array( 'avoid' => 'avoidseconds', 'noabbrevs' => true ), '48 hours 0 minutes', 'formatTimePeriod() rounding (=48h), avoidseconds' ),
 49+ array( 259199.55, 'avoidminutes', '3d 0h', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
 50+ array( 259199.55, array( 'avoid' => 'avoidminutes', 'noabbrevs' => true ), '3 days 0 hours', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
 51+ array( 176399.55, 'avoidseconds', '2d 1h 0m', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
 52+ array( 176399.55, array( 'avoid' => 'avoidseconds', 'noabbrevs' => true ), '2 days 1 hour 0 minutes', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
 53+ array( 176399.55, 'avoidminutes', '2d 1h', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
 54+ array( 176399.55, array( 'avoid' => 'avoidminutes', 'noabbrevs' => true ), '2 days 1 hour', 'formatTimePeriod() rounding (>48h), avoidminutes' ),
 55+ array( 259199.55, 'avoidseconds', '3d 0h 0m', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
 56+ array( 259199.55, array( 'avoid' => 'avoidseconds', 'noabbrevs' => true ), '3 days 0 hours 0 minutes', 'formatTimePeriod() rounding (>48h), avoidseconds' ),
 57+ array( 172801.55, 'avoidseconds', '2d 0h 0m', 'formatTimePeriod() rounding, (>48h), avoidseconds' ),
 58+ array( 172801.55, array( 'avoid' => 'avoidseconds', 'noabbrevs' => true ), '2 days 0 hours 0 minutes', 'formatTimePeriod() rounding, (>48h), avoidseconds' ),
 59+ array( 176460.55, array(), '2d 1h 1m 1s', 'formatTimePeriod() rounding, recursion, (>48h)' ),
 60+ array( 176460.55, array( 'noabbrevs' => true ), '2 days 1 hour 1 minute 1 second', 'formatTimePeriod() rounding, recursion, (>48h)' ),
6161 );
6262
6363 }
Index: trunk/phase3/languages/Language.php
——@@ -3549,17 +3549,28 @@
35503550 /**
35513551 * @todo Document
35523552 * @param $seconds int|float
3553 - * @param $format String Optional, one of ("avoidseconds","avoidminutes"):
3554 - * "avoidseconds" - don't mention seconds if $seconds >= 1 hour
3555 - * "avoidminutes" - don't mention seconds/minutes if $seconds > 48 hours
3556 - * @param $noAbbrevs Bool If true (or true-ish, recommend using 'noabbrevs' for clarity), use 'seconds' and friends instead of 'seconds-abbrev' and friends
 3553+ * @param $format Array Optional
 3554+ * If $format['avoid'] == 'avoidseconds' - don't mention seconds if $seconds >= 1 hour
 3555+ * If $format['avoid'] == 'avoidminutes' - don't mention seconds/minutes if $seconds > 48 hours
 3556+ * If $format['noabbrevs'] is true - use 'seconds' and friends instead of 'seconds-abbrev' and friends
 3557+ * For backwards compatibility, $format may also be one of the strings 'avoidseconds' or 'avoidminutes'
35573558 * @return string
35583559 */
3559 - function formatTimePeriod( $seconds, $format = false, $noAbbrevs = false ) {
3560 - $secondsMsg = wfMessage( $noAbbrevs ? 'seconds' : 'seconds-abbrev' )->inLanguage( $this );
3561 - $minutesMsg = wfMessage( $noAbbrevs ? 'minutes' : 'minutes-abbrev' )->inLanguage( $this );
3562 - $hoursMsg = wfMessage( $noAbbrevs ? 'hours' : 'hours-abbrev' )->inLanguage( $this );
3563 - $daysMsg = wfMessage( $noAbbrevs ? 'days' : 'days-abbrev' )->inLanguage( $this );
 3560+ function formatTimePeriod( $seconds, $format = array() ) {
 3561+ if ( !is_array( $format ) ) {
 3562+ $format = array( 'avoid' => $format, 'noabbrevs' => false ); // For backwards compatibility
 3563+ }
 3564+ if ( !isset( $format['avoid'] ) ) {
 3565+ $format['avoid'] = false;
 3566+ }
 3567+ if ( !isset( $format['noabbrevs' ] ) ) {
 3568+ $format['noabbrevs'] = false;
 3569+ }
 3570+ $secondsMsg = wfMessage( $format['noabbrevs'] ? 'seconds' : 'seconds-abbrev' )->inLanguage( $this );
 3571+ $minutesMsg = wfMessage( $format['noabbrevs'] ? 'minutes' : 'minutes-abbrev' )->inLanguage( $this );
 3572+ $hoursMsg = wfMessage( $format['noabbrevs'] ? 'hours' : 'hours-abbrev' )->inLanguage( $this );
 3573+ $daysMsg = wfMessage( $format['noabbrevs'] ? 'days' : 'days-abbrev' )->inLanguage( $this );
 3574+
35643575 if ( round( $seconds * 10 ) < 100 ) {
35653576 $s = $this->formatNum( sprintf( "%.1f", round( $seconds * 10 ) / 10 ) );
35663577 $s = $secondsMsg->params( $s )->text();
——@@ -3591,12 +3602,12 @@
35923603 $s = $hoursMsg->params( $this->formatNum( $hours ) )->text();
35933604 $s .= ' ';
35943605 $s .= $minutesMsg->params( $this->formatNum( $minutes ) )->text();
3595 - if ( !in_array( $format, array( 'avoidseconds', 'avoidminutes' ) ) ) {
 3606+ if ( !in_array( $format['avoid'], array( 'avoidseconds', 'avoidminutes' ) ) ) {
35963607 $s .= ' ' . $secondsMsg->params( $this->formatNum( $secondsPart ) )->text();
35973608 }
35983609 } else {
35993610 $days = floor( $seconds / 86400 );
3600 - if ( $format === 'avoidminutes' ) {
 3611+ if ( $format['avoid'] === 'avoidminutes' ) {
36013612 $hours = round( ( $seconds - $days * 86400 ) / 3600 );
36023613 if ( $hours == 24 ) {
36033614 $hours = 0;
——@@ -3605,7 +3616,7 @@
36063617 $s = $daysMsg->params( $this->formatNum( $days ) )->text();
36073618 $s .= ' ';
36083619 $s .= $hoursMsg->params( $this->formatNum( $hours ) )->text();
3609 - } elseif ( $format === 'avoidseconds' ) {
 3620+ } elseif ( $format['avoid'] === 'avoidseconds' ) {
36103621 $hours = floor( ( $seconds - $days * 86400 ) / 3600 );
36113622 $minutes = round( ( $seconds - $days * 86400 - $hours * 3600 ) / 60 );
36123623 if ( $minutes == 60 ) {
——@@ -3624,7 +3635,7 @@
36253636 } else {
36263637 $s = $daysMsg->params( $this->formatNum( $days ) )->text();
36273638 $s .= ' ';
3628 - $s .= $this->formatTimePeriod( $seconds - $days * 86400, $format, $noAbbrevs );
 3639+ $s .= $this->formatTimePeriod( $seconds - $days * 86400, $format );
36293640 }
36303641 }
36313642 return $s;

Follow-up revisions

RevisionCommit summaryAuthorDate
r98007Update MoodBar for r98006catrope15:45, 24 September 2011
r981381.17wmf1: MFT r97962, r98006, basically copies formatTimePeriod() from trunkcatrope15:22, 26 September 2011
r981441.17wmf1: MFT r97606, r97609, r97644, r97793, r97804, r97962, r98006, r98023catrope17:21, 26 September 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r97962Make Language::formatTimePeriod() more flexible so it can produce stuff like ...catrope22:17, 23 September 2011

Comments

#Comment by G.Hagedorn (talk | contribs)   20:21, 26 September 2011

since added to 1.18wmf (r98144 is updating 1.18wmf1, not 1.17wmf1), please consider tagging for 1.18 as well

Status & tagging log