r96499 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r96498‎ | r96499 | r96500 >
Date:21:06, 7 September 2011
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
Follow-up r81074:
* Disable {{#convert}} function by default. This means that this change will not be live by default, so the fixme doesn't block WMF deployment, although it probably should still block the tarball.
* Documentation and type hinting
* Fix minor errors noted by Tim in CR.
Modified paths:
  • /trunk/extensions/ParserFunctions/Convert.php (modified) (history)
  • /trunk/extensions/ParserFunctions/ParserFunctions.php (modified) (history)

Diff [purge]

Index: trunk/extensions/ParserFunctions/Convert.php
@@ -24,8 +24,14 @@
2525 # A regex *FRAGMENT* which matches SI prefixes
2626 const PREFIX_REGEX = '[YZEPTGMkh(da)dcm\x{03BC}\x{00B5}npfazy]?';
2727
28 - # ConvertUnit objects
 28+ /**
 29+ * @var ConvertUnit
 30+ */
2931 protected $sourceUnit;
 32+
 33+ /**
 34+ * @var ConvertUnit
 35+ */
3036 protected $targetUnit;
3137
3238 # Whether to abbreviate the output unit
@@ -48,6 +54,9 @@
4955 # The last value converted, which will be used for PLURAL evaluation
5056 protected $lastValue;
5157
 58+ /**
 59+ * Reset the parser so it isn't contaminated by the results of previous parses
 60+ */
5261 public function clearState(){
5362 # Make sure we break any references set up in the parameter passing below
5463 unset( $this->sourceUnit );
@@ -176,6 +185,11 @@
177186 return $this->processString( $string );
178187 }
179188
 189+ /**
 190+ * Find the unit at the end of the string and load $this->sourceUnit with an appropriate
 191+ * ConvertUnit, or throw an exception if the unit is unrecognised.
 192+ * @param $string
 193+ */
180194 protected function deduceSourceUnit( $string ){
181195 # Get the unit from the end of the string
182196 $matches = array();
@@ -191,6 +205,7 @@
192206 /**
193207 * Identify the values to be converted, and convert them
194208 * @param $string String
 209+ * @return String
195210 */
196211 protected function processString( $string ){
197212 # Replace values
@@ -216,7 +231,7 @@
217232 * Express a value in the $sourceUnit in terms of the $targetUnit, preserving
218233 * an appropriate degree of accuracy.
219234 * @param $value String
220 - * @return void
 235+ * @return String
221236 */
222237 public function convert( $value ){
223238 global $wgContLang;
@@ -374,6 +389,11 @@
375390 public $value;
376391 protected $name;
377392
 393+ /**
 394+ * Constructor
 395+ * @param $var ConvertDimension|Int a dimension constant or existing unit
 396+ * @param $var2 ConvertDimension|Int optionally another dimension constant for a compound unit $var/$var2
 397+ */
378398 public function __construct( $var, $var2=null ){
379399 static $legalDimensionsFlip;
380400
@@ -418,8 +438,8 @@
419439 if( in_array( $this->value, array_keys( self::$legalDimensions ) ) ){
420440 $this->name = self::$legalDimensions[$this->value];
421441 $this->compoundName = array(
422 - self::$legalDimensions[$var],
423 - self::$legalDimensions[$var2],
 442+ self::$legalDimensions[$dim],
 443+ self::$legalDimensions[$dim2],
424444 );
425445 } else {
426446 # Some combinations of units are fine (carats per bushel is a perfectly good,
@@ -442,6 +462,8 @@
443463
444464 /**
445465 * Get the name, or names, of the dimension
 466+ * @param $expandCompound Bool Whether to return a string instead of an array of strings in
 467+ * case of a compound unit
446468 * @return String|Array of String
447469 */
448470 public function getName( $expandCompound = false ){
@@ -630,7 +652,9 @@
631653
632654 /***************** MEMBER VARIABLES *****************/
633655
634 - # @var ConvertDimension
 656+ /**
 657+ * @var ConvertDimension
 658+ */
635659 public $dimension;
636660
637661 # What number you need to multiply this unit by to get the equivalent
@@ -665,6 +689,10 @@
666690 $this->parseUnit( $rawUnit );
667691 }
668692
 693+ /**
 694+ * Parse a raw unit string, and populate member variables
 695+ * @param $rawUnit String
 696+ */
669697 protected function parseUnit( $rawUnit ){
670698
671699 # Do mappings like 'mph' --> 'mi/h'
@@ -724,12 +752,39 @@
725753 }
726754 }
727755
 756+ /**
 757+ * Get the mathematical factor which will convert a measurement in this unit into a
 758+ * measurement in the SI base unit for the dimension
 759+ * @return double
 760+ */
728761 public function getConversion(){
729 - return $this->prefix
730 - ? $this->conversion * self::$prefixes[$this->prefix][0]
731 - : $this->conversion;
 762+ return $this->conversion * $this->getPrefixConversion();
732763 }
733764
 765+ /**
 766+ * Get the conversion factor associated with the prefix(es) in the unit
 767+ * @return double
 768+ */
 769+ public function getPrefixConversion(){
 770+ if( !$this->prefix ){
 771+ return 1;
 772+ } elseif( is_array( $this->prefix ) ){
 773+ $x = $this->prefix[0] !== null
 774+ ? self::$prefixes[$this->prefix[0]][0]
 775+ : 1;
 776+ if( $this->prefix[1] !== null ){
 777+ $x *= self::$prefixes[$this->prefix[1]][0];
 778+ }
 779+ return $x;
 780+ } else {
 781+ return self::$prefixes[$this->prefix][0];
 782+ }
 783+ }
 784+
 785+ /**
 786+ * Get a regular expression which will match keywords for this unit
 787+ * @return String
 788+ */
734789 public function getRegex(){
735790 return $this->regex;
736791 }
@@ -739,6 +794,7 @@
740795 * @param $string String Original text, with the number converted
741796 * @param $value String number for PLURAL support
742797 * @param $link Bool
 798+ * @param $abbreviate Bool
743799 * @param $language Language
744800 * @return String
745801 */
@@ -787,6 +843,18 @@
788844 return trim( $msgText );
789845 }
790846
 847+ /**
 848+ * Retrieve the unit text from actual messages
 849+ * @param $dimension String
 850+ * @param $unit String
 851+ * @param $prefix String
 852+ * @param $string String
 853+ * @param $number String the actual value (for {{PLURAL}} etc)
 854+ * @param $link Bool
 855+ * @param $abbreviate Bool
 856+ * @param $language Language|Bool|null
 857+ * @return String
 858+ */
791859 protected function getTextFromMessage( $dimension, $unit, $prefix, $string, $number, $link, $abbreviate, $language ){
792860 $abbr = $abbreviate ? '-abbr' : '';
793861 $prefix = $prefix === null
Index: trunk/extensions/ParserFunctions/ParserFunctions.php
@@ -29,6 +29,11 @@
3030 */
3131 $wgPFEnableStringFunctions = false;
3232
 33+/**
 34+ * Enable Convert parser for converting between units of measurement
 35+ */
 36+$wgPFEnableConvert = false;
 37+
3338 /** REGISTRATION */
3439 $wgExtensionCredits['parserhook'][] = array(
3540 'path' => __FILE__,
@@ -53,7 +58,7 @@
5459 $wgHooks['ParserFirstCallInit'][] = 'wfRegisterParserFunctions';
5560
5661 function wfRegisterParserFunctions( $parser ) {
57 - global $wgPFEnableStringFunctions;
 62+ global $wgPFEnableStringFunctions, $wgPFEnableConvert;
5863
5964 if ( defined( get_class( $parser ) . '::SFH_OBJECT_ARGS' ) ) {
6065 // These functions accept DOM-style arguments
@@ -77,7 +82,6 @@
7883 $parser->setFunctionHook( 'timel', 'ExtParserFunctions::localTime' );
7984 $parser->setFunctionHook( 'rel2abs', 'ExtParserFunctions::rel2abs' );
8085 $parser->setFunctionHook( 'titleparts', 'ExtParserFunctions::titleparts' );
81 - $parser->setFunctionHook( 'convert', 'ExtParserFunctions::convert' );
8286
8387 // String Functions
8488 if ( $wgPFEnableStringFunctions ) {
@@ -91,5 +95,9 @@
9296 $parser->setFunctionHook( 'urldecode', 'ExtParserFunctions::runUrlDecode' );
9397 }
9498
 99+ if( $wgPFEnableConvert ) {
 100+ $parser->setFunctionHook( 'convert', 'ExtParserFunctions::convert' );
 101+ }
 102+
95103 return true;
96104 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r96505FU 96499, r81074: fix parser tests to reflect the change in default dialect, ...happy-melon21:39, 7 September 2011
r96933REL1_18: MFT r96499, r96502, r96505reedy00:36, 13 September 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 Catrope (talk | contribs)   21:17, 7 September 2011

This just broke 3 million parser tests :)

#Comment by Catrope (talk | contribs)   21:18, 7 September 2011

Hm, only 13 apparently. Anyway, the #convert parser tests need to not get run if #convert is disabled.

#Comment by Happy-melon (talk | contribs)   21:30, 7 September 2011

If we had three million parser tests to break, I'd be delighted!

#Comment by Happy-melon (talk | contribs)   21:40, 7 September 2011

Fixed in r96505.

Status & tagging log