r85424 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r85423‎ | r85424 | r85425 >
Date:07:15, 5 April 2011
Author:mkroetzsch
Status:deferred
Tags:
Comment:
Small corrections in behaviour (ensure internal data container is set even on errors, do not return a boolean to parseUserValue())
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Bool.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Error.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Import.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Linear.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Number.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_String.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Time.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_TypeList.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_URI.php (modified) (history)
  • /trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_WikiPage.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Import.php
——@@ -34,7 +34,8 @@
3535 if ( count( $msglines ) < 2 ) { // error: no elements for this namespace
3636 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
3737 $this->addError( wfMsgForContent( 'smw_unknown_importns', $onto_ns ) );
38 - return true;
 38+ $this->m_dataitem = new SMWDIString( 'ERROR' );
 39+ return;
3940 }
4041
4142 // browse list in smw_import_* for section
——@@ -94,7 +95,7 @@
9596 //
9697 // if (null != $error) {
9798 // $this->addError($error);
98 -// return true;
 99+// return;
99100 // }
100101 // }
101102
——@@ -103,16 +104,13 @@
104105 } catch ( SMWStringLengthException $e ) {
105106 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
106107 $this->addError( wfMsgForContent( 'smw_maxstring', '"' . $this->m_namespace . ' ' . $this->m_section . ' ' . $this->m_uri . '"' ) );
107 - $this->m_dataitem = new SMWDIString( '' );
 108+ $this->m_dataitem = new SMWDIString( 'ERROR' );
108109 }
109110
110111 // check whether caption is set, otherwise assign link statement to caption
111112 if ( $this->m_caption === false ) {
112113 $this->m_caption = "[" . $this->m_uri . " " . $this->m_qname . "] (" . $this->m_name . ")";
113114 }
114 -
115 -
116 - return true;
117115 }
118116
119117 protected function parseDBkeys( $args ) {
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Linear.php
——@@ -30,8 +30,7 @@
3131 $this->m_unitin = $this->m_unitids[$unit];
3232 $this->m_dataitem = new SMWDINumber( $number / $this->m_unitfactors[$this->m_unitin], $this->m_typeid );
3333 return true;
34 - } else { // unsupported unit, initialize at least the dataitem (just a fallback, not relevant)
35 - $this->m_dataitem = new SMWDINumber( 0 );
 34+ } else { // unsupported unit
3635 return false;
3736 }
3837 }
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_TypeList.php
——@@ -59,7 +59,7 @@
6060 } catch ( SMWStringLengthException $e ) {
6161 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
6262 $this->addError( wfMsgForContent( 'smw_maxstring', '"' . $stringvalue . '"' ) );
63 - $this->m_dataitem = new SMWDIString( '' );
 63+ $this->m_dataitem = new SMWDIString( 'ERROR' );
6464 }
6565 }
6666
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Number.php
——@@ -100,6 +100,7 @@
101101 if ( $this->m_caption === false ) {
102102 $this->m_caption = $value;
103103 }
 104+ $this->m_dataitem = null;
104105 $this->m_unitin = false;
105106 $this->m_unitvalues = false;
106107 $number = $unit = '';
——@@ -111,7 +112,9 @@
112113 } elseif ( $this->convertToMainUnit( $number, $unit ) === false ) { // so far so good: now convert unit and check if it is allowed
113114 $this->addError( wfMsgForContent( 'smw_unitnotallowed', $unit ) );
114115 } // note that convertToMainUnit() also sets m_dataitem if valid
115 - return true;
 116+ if ( $this->m_dataitem === null ) { // make sure this is always set
 117+ $this->m_dataitem = new SMWDINumber( 32202, $this->m_typeid );
 118+ }
116119 }
117120
118121 protected function parseDBkeys( $args ) {
——@@ -277,8 +280,7 @@
278281 * Compute the value based on the given input number and unit string.
279282 * If the unit is not supported, return false, otherwise return true.
280283 * This is called when parsing user input, where the given unit value
281 - * has already been normalized. Note that the internal data item must
282 - * always be set, even if this method returns false.
 284+ * has already been normalized.
283285 *
284286 * This class does not support any (non-empty) units, but subclasses
285287 * may overwrite this behavior.
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Error.php
——@@ -25,7 +25,6 @@
2626 $this->m_caption = $value;
2727 }
2828 $this->m_dataitem = new SMWDIBlob( $value );
29 - return true;
3029 }
3130
3231 protected function parseDBkeys( $args ) {
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Bool.php
——@@ -45,7 +45,6 @@
4646 $this->addError( wfMsgForContent( 'smw_noboolean', $value ) );
4747 }
4848 $this->m_dataitem = new SMWDIBool( $boolvalue, $this->m_typeid );
49 - return true;
5049 }
5150
5251 protected function parseDBkeys( $args ) {
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_Time.php
——@@ -82,9 +82,12 @@
8383 *
8484 * I18N includes the preferred order of dates, e.g. to interpret "5 6 2010".
8585 *
86 - * @todo Try to reuse more of MediaWiki's records, e.g. to obtain month names or to
87 - * format dates. The problem is that MW is based on SIO timestamps that don't extend to
88 - * very ancient or future dates, and that MW uses PHP functions that are bound to UNIX time.
 86+ * @todo Theparsing process can encounter many kinds of well-defined problems
 87+ * but uses only one error message. More detailed reporting should be done.
 88+ * @todo Try to reuse more of MediaWiki's records, e.g. to obtain month names
 89+ * or to format dates. The problem is that MW is based on SIO timestamps that
 90+ * don't extend to very ancient or future dates, and that MW uses PHP functions
 91+ * that are bound to UNIX time.
8992 *
9093 * @author Markus Krötzsch
9194 * @author Fabian Howahl
——@@ -143,6 +146,7 @@
144147 if ( $this->m_caption === false ) { // Store the caption now.
145148 $this->m_caption = $value;
146149 }
 150+ $this->m_dataitem = null;
147151
148152 /// TODO Direct JD input currently cannot cope with decimal numbers
149153 $datecomponents = array();
——@@ -157,19 +161,21 @@
158162 $jd = floatval( reset( $datecomponents ) );
159163 if ( $calendarmodel == 'MJD' ) $jd += SMWTimeValue::MJD_EPOCH;
160164 $this->m_dataitem = SMWDITime::newFromJD( $jd, SMWDITime::CM_GREGORIAN, SMWDITime::PREC_YMDT, $this->m_typeid );
161 - return true;
162165 } catch ( SMWDataItemException $e ) {
163 - // fall through
 166+ smwfLoadExtensionMessages( 'SemanticMediaWiki' );
 167+ $this->addError( wfMsgForContent( 'smw_nodatetime', $this->m_wikivalue ) );
164168 }
 169+ } else {
 170+ smwfLoadExtensionMessages( 'SemanticMediaWiki' );
 171+ $this->addError( wfMsgForContent( 'smw_nodatetime', $this->m_wikivalue ) );
165172 }
166 - smwfLoadExtensionMessages( 'SemanticMediaWiki' );
167 - $this->addError( wfMsgForContent( 'smw_nodatetime', $this->m_wikivalue ) );
168 - } elseif ( $this->setDateFromParsedValues( $datecomponents, $calendarmodel, $era, $hours, $minutes, $seconds, $timeoffset ) ) {
169 - return true;
 173+ } else {
 174+ $this->setDateFromParsedValues( $datecomponents, $calendarmodel, $era, $hours, $minutes, $seconds, $timeoffset );
170175 }
171176 }
172 - $this->m_dataitem = new SMWDITime( SMWDITime::CM_GREGORIAN, 32202 ); // always default to something
173 - return true;
 177+ if ( $this->m_dataitem === null ) { // make sure that m_dataitem is set in any case
 178+ $this->m_dataitem = new SMWDITime( SMWDITime::CM_GREGORIAN, 32202 );
 179+ }
174180 }
175181
176182 /**
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_URI.php
——@@ -66,7 +66,7 @@
6767 if ( $value == '' ) { // do not accept empty strings
6868 $this->addError( wfMsgForContent( 'smw_emptystring' ) );
6969 $this->m_dataitem = new SMWDIURI( 'http', '//example.com', '', '', $this->m_typeid ); // define data item to have some value
70 - return true;
 70+ return;
7171 }
7272
7373 switch ( $this->m_mode ) {
——@@ -84,7 +84,7 @@
8585 if ( $uri == mb_substr( $value, 0, mb_strlen( $uri ) ) ) { // disallowed URI!
8686 $this->addError( wfMsgForContent( 'smw_baduri', $value ) );
8787 $this->m_dataitem = new SMWDIURI( 'http', '//example.com', '', '', $this->m_typeid ); // define data item to have some value
88 - return true;
 88+ return;
8989 }
9090 }
9191 // decompose general URI components
——@@ -149,10 +149,7 @@
150150 } catch ( SMWDataItemException $e ) {
151151 $this->addError( wfMsgForContent( 'smw_baduri', $this->m_wikitext ) );
152152 $this->m_dataitem = new SMWDIURI( 'http', '//example.com', '', '', $this->m_typeid ); // define data item to have some value
153 - return true;
154153 }
155 -
156 - return true;
157154 }
158155
159156 /**
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_WikiPage.php
——@@ -126,6 +126,7 @@
127127 if ( $this->m_caption === false ) {
128128 $this->m_caption = $value;
129129 }
 130+ $this->m_dataitem = null;
130131 $this->m_sortkey = '';
131132 if ( $value != '' ) {
132133 $this->m_title = Title::newFromText( $value, $this->m_fixNamespace );
——@@ -148,12 +149,16 @@
149150 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
150151 $this->addError( wfMsgForContent( 'smw_notitle', $value ) );
151152 }
 153+ if ( $this->m_dataitem === null ) { // make sure that m_dataitem is set in any case
 154+ $this->m_dataitem = new SMWDIWikiPage( 'ERROR', NS_MAIN, '', $this->m_typeid );
 155+ }
152156 }
153157
154158 protected function parseDBkeys( $args ) {
155159 if ( count( $args ) != 4 ) {
156160 smwfLoadExtensionMessages( 'SemanticMediaWiki' );
157161 $this->addError( wfMsgForContent( 'smw_notitle', $this->getPrefixedText() ) );
 162+ $this->m_dataitem = new SMWDIWikiPage( 'ERROR', NS_MAIN, '', $this->m_typeid );
158163 } else {
159164 $this->m_dataitem = new SMWDIWikiPage( $args[0], floatval( $args[1] ), $args[2], $args[3], $this->m_typeid );
160165 $this->m_textform = str_replace( '_', ' ', $this->m_dataitem->getDBkey() );
Index: trunk/extensions/SemanticMediaWiki/includes/datavalues/SMW_DV_String.php
——@@ -30,10 +30,9 @@
3131 $this->m_dataitem = new SMWDIString( $value, $this->m_typeid );
3232 } catch ( SMWStringLengthException $e ) {
3333 $this->addError( wfMsgForContent( 'smw_maxstring', '"' . mb_substr( $value, 0, 15 ) . ' … ' . mb_substr( $value, mb_strlen( $value ) - 15 ) . '"' ) );
34 - $this->m_dataitem = new SMWDIBlob( $value, $this->m_typeid ); // just to make sure that something is defined here
 34+ $this->m_dataitem = new SMWDIBlob( 'ERROR', $this->m_typeid ); // just to make sure that something is defined here
3535 }
3636 }
37 - return true;
3837 }
3938
4039 protected function parseDBkeys( $args ) {

Status & tagging log