r81517 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81516‎ | r81517 | r81518 >
Date:14:28, 4 February 2011
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
Follow-up r81074: turns out {{convert}} is (mostly) case-sensitive after all, so reclaim case-sensitivity here. Will work on abstracting out SI prefixes later.
Modified paths:
  • /trunk/extensions/ParserFunctions/Convert.php (modified) (history)
  • /trunk/extensions/ParserFunctions/convertTests.txt (modified) (history)

Diff [purge]

Index: trunk/extensions/ParserFunctions/convertTests.txt
——@@ -198,14 +198,30 @@
199199 *{{#convert: 10 m | km | #language=en-gb }}
200200 *{{#convert: 10m | km | #language = en-gb }}
201201 *{{#convert: 10 km | m |#language=en-gb}}
202 -*{{#convert: 10 pa | mmhg | #language = en-gb }}
203 -*{{#convert: 10 pa | mmhg | #language = /../evil/attack }}
 202+*{{#convert: 10 Pa | mmHg | #language = en-gb }}
204203 !! result
205204 <ul><li>0.01 kilometres
206205 </li><li>0.01kilometres
207206 </li><li>10,000 metres
208207 </li><li>0.1 milimetres of mercury
 208+</li></ul>
 209+
 210+!! end
 211+
 212+!! test
 213+Case sensitivity
 214+!! input
 215+*{{#convert: 10 mm | m }}
 216+*{{#convert: 10 Mm | m }}
 217+*{{#convert: 10 km | Mm }}
 218+*{{#convert: 10 Pa | mmHg }}
 219+*{{#convert: 10 pa | mmHg }}
 220+!! result
 221+<ul><li>0.01 meters
 222+</li><li>10,000,000 meters
 223+</li><li>0.01 megameters
209224 </li><li>0.1 milimeters of mercury
 225+</li><li><strong class="error">Error: unknown unit "pa"</strong>
210226 </li></ul>
211227
212228 !! end
Index: trunk/extensions/ParserFunctions/Convert.php
——@@ -472,14 +472,14 @@
473473 protected static $units = array(
474474 ConvertDimension::DIM_LENGTH => array(
475475 'gigametre' => array( 1000000000, 'Gm' ),
476 - 'megametre' => array( 1000000, '(?:(?-i)Mm)' ), # Case-sensitivity is forced
 476+ 'megametre' => array( 1000000, 'Mm' ), # Case-sensitivity is forced
477477 'kilometre' => array( 1000, 'km' ),
478478 'hectometre' => array( 100, 'hm' ),
479479 'decametre' => array( 10, 'dam' ),
480480 'metre' => array( 1, 'm' ),
481481 'decimetre' => array( 0.1, 'dm' ),
482482 'centimetre' => array( 0.01, 'cm' ),
483 - 'millimetre' => array( 0.001, '(?:(?-i)mm)' ), # Case-sensitivity is forced
 483+ 'millimetre' => array( 0.001, 'mm' ), # Case-sensitivity is forced
484484 'micrometre' => array( 0.0001, '\x03BCm|\x00B5m|um' ), # There are two similar mu characters
485485 'nanometre' => array( 0.0000001, 'nm' ),
486486 'angstrom' => array( 0.00000001, '\x00C5' ),
——@@ -495,8 +495,8 @@
496496 'inch' => array( 0.0254, 'inch|inches|in' ),
497497
498498 'nauticalmile' => array( 1852, 'nauticalmiles?|nmi' ),
499 - 'nauticalmileuk' => array( 1853.184, 'oldUKnmi|Brnmi|admi' ),
500 - 'nauticalmileus' => array( 1853.24496, 'oldUSnmi' ),
 499+ 'nauticalmileuk' => array( 1853.184, 'old[Uu][Kk]nmi|[Bb]rnmi|admi' ),
 500+ 'nauticalmileus' => array( 1853.24496, 'old[Uu][Ss]nmi' ),
501501
502502 'gigaparsec' => array( 3.0856775813057E25, 'gigaparsecs?|Gpc' ),
503503 'megaparsec' => array( 3.0856775813057E22, 'megaparsecs?|Mpc' ),
——@@ -506,7 +506,7 @@
507507 'mrgalightyear' => array( 9.4607304725808E21, 'megalightyears?|Mly' ),
508508 'kilolightyear' => array( 9.4607304725808E18, 'kilolightyears?|kly' ),
509509 'lightyear' => array( 9.4607304725808E15, 'lightyears?|ly' ),
510 - 'astronomicalunit' => array( 149597870700, 'astronomicalunits?|AU' ),
 510+ 'astronomicalunit' => array( 149597870700, 'astronomicalunits?|AU|au' ),
511511 ),
512512
513513 ConvertDimension::DIM_AREA => array(
——@@ -575,11 +575,11 @@
576576
577577 ConvertDimension::DIM_PRESSURE => array(
578578 'gigapascal' => array( 1000000000, 'GPa' ),
579 - 'megapascal' => array( 1000000, '(?:(?-i)M[Pp]a)' ), # Case-sensitivity is forced
 579+ 'megapascal' => array( 1000000, 'MPa' ), # Case-sensitivity is forced
580580 'kilopascal' => array( 1000, 'kPa' ),
581581 'hectopascal' => array( 100, 'hPa' ),
582582 'pascal' => array( 1, 'Pa' ),
583 - 'millipascal' => array( 0.001, '(?:(?-i)m[Pp]a)' ), # Case-sensitivity is forced
 583+ 'millipascal' => array( 0.001, 'mPa' ), # Case-sensitivity is forced
584584
585585 'bar' => array( 100000, 'bar' ),
586586 'decibar' => array( 10000, 'dbar' ),
——@@ -609,7 +609,7 @@
610610
611611 # An array of preprocessing conversions to apply to units
612612 protected static $unitConversions = array(
613 - '/^mph$/ui' => 'mi/h',
 613+ '/^mph$/u' => 'mi/h',
614614 );
615615
616616 # Map of UNIT => DIMENSION, created on construct
——@@ -664,7 +664,7 @@
665665 # Single unit
666666 foreach( self::$units as $dimension => $units ){
667667 foreach( $units as $unit => $data ){
668 - if( $rawUnit == $unit || preg_match( "/^({$data[1]})$/ui", $parts[0] ) ){
 668+ if( $rawUnit == $unit || preg_match( "/^({$data[1]})$/u", $parts[0] ) ){
669669 $this->dimension = new ConvertDimension( self::$dimensionMap[$unit] );
670670 $this->conversion = self::$units[$this->dimension->value][$unit][0];
671671 $this->regex = $data[1];

Follow-up revisions

RevisionCommit summaryAuthorDate
r82230Follow-up r81074, r81517: implement SI prefixes as a separate structure. You...happy-melon10:27, 16 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r81074(bug 235) parser function for conversion of units of measurement....happy-melon00:13, 27 January 2011

Comments

#Comment by The Evil IP address (talk | contribs)   14:51, 4 February 2011

If you ask me, that's {{Convert}}'s fault. Case-insensivity is definitely a good thing for MediaWiki, IMO, and should be kept.

#Comment by Happy-melon (talk | contribs)   15:02, 4 February 2011

But when case-sensitivity means we can't introduce new functionality, is that a good thing? If we keep it case-insensitive we couldn't do conversions between units of magnetic field strength (Teslas vs Gauss, a standard SI/Imperial conversion) because 'T' for Tesla would conflict with 't' for ton. We couldn't put SI prefixes on a whole range of SI units, because "kN" (kilonewton) conflicts with "kn" (knot), "pC" (picocoulomb) with "pc" (parsec), etc etc.

#Comment by Aaron Schulz (talk | contribs)   20:16, 15 August 2011

I'd agree with the case-sensitivity added in this commit.

Status & tagging log