r97793 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97792‎ | r97793 | r97794 >
Date:05:01, 22 September 2011
Author:santhosh
Status:resolved (Comments)
Tags:
Comment:
Add support for Number grouping(commafy) based on CLDR number grouping patterns like ##,##,###.
Testcases for Malayalam and Dutch
Number grouping Patterns added to Ml and Hi Message classes.
Reference: Bug 29495
Modified paths:
  • /trunk/phase3/includes/LocalisationCache.php (modified) (history)
  • /trunk/phase3/languages/Language.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesHi.php (modified) (history)
  • /trunk/phase3/languages/messages/MessagesMl.php (modified) (history)
  • /trunk/phase3/tests/phpunit/languages/LanguageMlTest.php (added) (history)
  • /trunk/phase3/tests/phpunit/languages/LanguageNlTest.php (added) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/languages/LanguageMlTest.php
@@ -0,0 +1,30 @@
 2+<?php
 3+/**
 4+ * @author Santhosh Thottingal
 5+ * @copyright Copyright © 2011, Santhosh Thottingal
 6+ * @file
 7+ */
 8+
 9+/** Tests for MediaWiki languages/LanguageMl.php */
 10+class LanguageMlTest extends MediaWikiTestCase {
 11+ private $lang;
 12+
 13+ function setUp() {
 14+ $this->lang = Language::factory( 'Ml' );
 15+ }
 16+ function tearDown() {
 17+ unset( $this->lang );
 18+ }
 19+
 20+ /** see bug 29495 */
 21+ function testFormatNum() {
 22+ $this->assertEquals( '12,34,567', $this->lang->formatNum( '1234567' ) );
 23+ $this->assertEquals( '12,345', $this->lang->formatNum( '12345' ) );
 24+ $this->assertEquals( '1', $this->lang->formatNum( '1' ) );
 25+ $this->assertEquals( '123', $this->lang->formatNum( '123' ) );
 26+ $this->assertEquals( '1,234', $this->lang->formatNum( '1234' ) );
 27+ $this->assertEquals( '12,345.56', $this->lang->formatNum( '12345.56' ) );
 28+ $this->assertEquals( '12,34,56,79,81,23,45,678', $this->lang->formatNum( '12345679812345678' ) );
 29+ $this->assertEquals( '.12345', $this->lang->formatNum( '.12345' ) );
 30+ }
 31+}
Property changes on: trunk/phase3/tests/phpunit/languages/LanguageMlTest.php
___________________________________________________________________
Added: svn:eol-style
132 + native
Index: trunk/phase3/tests/phpunit/languages/LanguageNlTest.php
@@ -0,0 +1,28 @@
 2+<?php
 3+/**
 4+ * @author Santhosh Thottingal
 5+ * @copyright Copyright © 2011, Santhosh Thottingal
 6+ * @file
 7+ */
 8+
 9+/** Tests for MediaWiki languages/LanguageNl.php */
 10+class LanguageNlTest extends MediaWikiTestCase {
 11+ private $lang;
 12+
 13+ function setUp() {
 14+ $this->lang = Language::factory( 'Nl' );
 15+ }
 16+ function tearDown() {
 17+ unset( $this->lang );
 18+ }
 19+
 20+ function testFormatNum() {
 21+ $this->assertEquals( '1.234.567', $this->lang->formatNum( '1234567' ) );
 22+ $this->assertEquals( '12.345', $this->lang->formatNum( '12345' ) );
 23+ $this->assertEquals( '1', $this->lang->formatNum( '1' ) );
 24+ $this->assertEquals( '123', $this->lang->formatNum( '123' ) );
 25+ $this->assertEquals( '1.234', $this->lang->formatNum( '1234' ) );
 26+ $this->assertEquals( '12.345,56', $this->lang->formatNum( '12345.56' ) );
 27+ $this->assertEquals( ',1234556', $this->lang->formatNum( '.1234556' ) );
 28+ }
 29+}
Property changes on: trunk/phase3/tests/phpunit/languages/LanguageNlTest.php
___________________________________________________________________
Added: svn:eol-style
130 + native
Index: trunk/phase3/includes/LocalisationCache.php
@@ -88,6 +88,7 @@
8989 'dateFormats', 'datePreferences', 'datePreferenceMigrationMap',
9090 'defaultDateFormat', 'extraUserToggles', 'specialPageAliases',
9191 'imageFiles', 'preloadedMessages', 'namespaceGenderAliases',
 92+ 'digitGroupingPattern'
9293 );
9394
9495 /**
Index: trunk/phase3/languages/messages/MessagesHi.php
@@ -68,6 +68,8 @@
6969 );
7070 $linkTrail = "/^([a-z]+)(.*)$/sD";
7171
 72+$digitGroupingPattern = "##,##,###";
 73+
7274 $messages = array(
7375 # User preference toggles
7476 'tog-underline' => 'कड़ियाँ अधोरेखन:',
Index: trunk/phase3/languages/messages/MessagesMl.php
@@ -315,6 +315,8 @@
316316 'url_query' => array( '0', 'ക്വറി', 'QUERY' ),
317317 );
318318
 319+$digitGroupingPattern = "##,##,###";
 320+
319321 $messages = array(
320322 # User preference toggles
321323 'tog-underline' => 'കണ്ണികൾക്ക് അടിവരയിടുക:',
Index: trunk/phase3/languages/Language.php
@@ -2606,13 +2606,57 @@
26072607
26082608 /**
26092609 * Adds commas to a given number
2610 - *
 2610+ * @since 1.19
26112611 * @param $_ mixed
26122612 * @return string
26132613 */
26142614 function commafy( $_ ) {
2615 - return strrev( (string)preg_replace( '/(\d{3})(?=\d)(?!\d*\.)/', '$1,', strrev( $_ ) ) );
 2615+ $digitGroupingPattern = $this->digitGroupingPattern();
 2616+
 2617+ if ( !$digitGroupingPattern || $digitGroupingPattern === "###,###,###" ) {
 2618+ //default grouping is at thousands, use the same for ###,###,### pattern too.
 2619+ return strrev( (string)preg_replace( '/(\d{3})(?=\d)(?!\d*\.)/', '$1,', strrev( $_ ) ) );
 2620+ }
 2621+ else {
 2622+ // Ref: http://cldr.unicode.org/translation/number-patterns
 2623+ $numberpart = array();
 2624+ $decimalpart = array();
 2625+ $numMatches = preg_match_all( "/(#+)/", $digitGroupingPattern, $matches );
 2626+ preg_match( "/\d+/", $_, $numberpart );
 2627+ preg_match( "/\.\d*/", $_, $decimalpart );
 2628+ $groupedNumber = ( count( $decimalpart ) > 0 ) ? $decimalpart[0]:"";
 2629+ if ( $groupedNumber === $_){
 2630+ //the string does not have any number part. Eg: .12345
 2631+ return $groupedNumber;
 2632+ }
 2633+ $start = $end = strlen( $numberpart[0] );
 2634+ while ( $start > 0 )
 2635+ {
 2636+ $match = $matches[0][$numMatches -1] ;
 2637+ $matchLen = strlen( $match );
 2638+ $start = $end - $matchLen;
 2639+ if ( $start < 0 ) {
 2640+ $start = 0;
 2641+ }
 2642+ $groupedNumber = substr( $_ , $start, $end -$start ) . $groupedNumber ;
 2643+ $end = $start;
 2644+ if ( $numMatches > 1 ) {
 2645+ // use the last pattern for the rest of the number
 2646+ $numMatches--;
 2647+ }
 2648+ if ( $start > 0 ) {
 2649+ $groupedNumber = "," . $groupedNumber;
 2650+ }
 2651+ }
 2652+ return $groupedNumber;
 2653+ }
26162654 }
 2655+ /**
 2656+ * @return String
 2657+ */
 2658+ function digitGroupingPattern() {
 2659+ return self::$dataCache->getItem( $this->mCode, 'digitGroupingPattern' );
 2660+ }
26172661
26182662 /**
26192663 * @return array

Follow-up revisions

RevisionCommit summaryAuthorDate
r97804Add number grouping pattern. Ref Bug 29495. Ml and Hi added in r97793santhosh09:21, 22 September 2011
r97807Fix mixed tabs and spaces from r97793...reedy11:06, 22 September 2011
r981441.17wmf1: MFT r97606, r97609, r97644, r97793, r97804, r97962, r98006, r98023catrope17:21, 26 September 2011
r102802Make commafy support negative numbers too,...santhosh17:23, 11 November 2011

Comments

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

please consider tagging for 1.18 as well.

#Comment by Catrope (talk | contribs)   22:07, 26 September 2011

We're not gonna backport every random new feature. Only non-minor bug fixes and regression fixes will be backported.

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

(since added to 1.18wmf, note that commit summary of r98144 is wrong, 1.17wmf1 should read 1.18wmf1)

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

(Sorry for long reply) This seems to fix a bug without which Wikipedia would not properly work (lack of number formatting; Dutch and German have same requirements). The present 1.18wmf test phase is the first time that 1.18 is tested: only now do bugs and omissions surface. It would be helpful to make the necessary changes to 1.18 also available to users outside wmf.

If release versions are not kept in sync with the wmf version (with 1.17wmf probably often being closer to trunk than to 1.17) serious problem occur. In my observations these are usually not core-internal, but affect the relation between extensions and core. Many extension developers seem to have have three possible targets: a) an arbitrary very old version they use on their home wiki b) the WMF version c) trunk. 1.17 was very frustrating, we actually were unable to use it to find a combination between working core and extensions. We were better off with trunk than with a "stable" version, but would like to end this.

Example: Semantic result formats might soon start using digitGroupingPattern. It works on WMF and trunk. If there is a bug in one of the semantic extensions - bad luck, nothing will ever work with 1.18 stable, you just have to forget stable again. This will not change until the version of dominating importance, 1.18wmf will be a 1.18 version plus wmf-specific adaptations.

#Comment by Catrope (talk | contribs)   22:35, 26 September 2011

I agree that lack of number formatting is a bug, but it's one that has existed since the beginning, so it's not something normally deserving of a backport. I do see the argument for keeping 1.18 and 1.18wmf1 close to each other.

#Comment by Liangent (talk | contribs)   14:55, 11 November 2011

The new commafy algorithm is buggy, doesn't work correctly for negative numbers and causes the last digit cropped, which was reported in bug 32359.

Status & tagging log