r92142 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92141‎ | r92142 | r92143 >
Date:06:45, 14 July 2011
Author:devayon
Status:deferred
Tags:
Comment:
Fixed some bugs and todo's, follow-up to r92056
Modified paths:
  • /trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SemanticMediaWiki/specials/AskSpecial/SMW_QueryUIHelper.php
——@@ -107,6 +107,26 @@
108108 }
109109
110110 /**
 111+ * A helper function to enable JQuery
 112+ *
 113+ * @global OutputPage $wgOut
 114+ * @global boolean $smwgJQueryIncluded
 115+ */
 116+ private function enableJQuery(){
 117+ global $wgOut, $smwgJQueryIncluded;
 118+ if ( !$smwgJQueryIncluded ) {
 119+ $realFunction = array( 'OutputPage', 'includeJQuery' );
 120+ if ( is_callable( $realFunction ) ) {
 121+ $wgOut->includeJQuery();
 122+ } else {
 123+ $wgOut->addScriptFile( "$smwgScriptPath/libs/jquery-1.4.2.min.js" );
 124+ }
 125+
 126+ $smwgJQueryIncluded = true;
 127+ }
 128+ }
 129+
 130+ /**
111131 * Adds common JS and CSS required for Autocompletion.
112132 * @global OutputPage $wgOut
113133 * @global string $smwgScriptPath
——@@ -118,19 +138,10 @@
119139 if ( $this->autocompleteenabled == false ) {
120140 $wgOut->addExtensionStyle( "$smwgScriptPath/skins/jquery-ui/base/jquery.ui.all.css" );
121141
 142+ $this-> enableJQuery();
 143+
122144 $scripts = array();
123145
124 - if ( !$smwgJQueryIncluded ) {
125 - $realFunction = array( 'OutputPage', 'includeJQuery' );
126 - if ( is_callable( $realFunction ) ) {
127 - $wgOut->includeJQuery();
128 - } else {
129 - $scripts[] = "$smwgScriptPath/libs/jquery-1.4.2.min.js";
130 - }
131 -
132 - $smwgJQueryIncluded = true;
133 - }
134 -
135146 if ( !$smwgJQueryUIIncluded ) {
136147 $scripts[] = "$smwgScriptPath/libs/jquery-ui/jquery.ui.core.min.js";
137148 $scripts[] = "$smwgScriptPath/libs/jquery-ui/jquery.ui.widget.min.js";
——@@ -538,7 +549,7 @@
539550 $urltail = '&q=' . urlencode( $this->uiCore->getQuerystring() );
540551
541552 $tmp_parray = array();
542 - $params = $this->uiCore->getParams();
 553+ $params = $this->uiCore->getParameters();
543554 foreach ( $params as $key => $value ) {
544555 if ( !in_array( $key, array( 'sort', 'order', 'limit', 'offset', 'title' ) ) ) {
545556 $tmp_parray[$key] = $value;
——@@ -594,7 +605,7 @@
595606 'style' => 'width: 30%; padding: 5px; float: left;'
596607 ),
597608 htmlspecialchars( $param->getName() ) . ': ' .
598 - $this->showFormatOption( $param, $currentValue ) .
 609+ $this->getFormatOption( $param, $currentValue ) .
599610 '<br />' .
600611 Html::element( 'em', array(), $param->getDescription() )
601612 );
——@@ -610,21 +621,17 @@
611622 $rowHtml = '';
612623 $resultHtml = '';
613624
614 - /// @todo Check if this code works if the number of options is not a multiple of 3!
615625 while ( $option = array_shift( $optionsHtml ) ) {
616626 $rowHtml .= $option;
617627 $i += 1;
618 -
619 - if ( $i % 3 == 0 ) {
620 - $resultHtml .= Html::rawElement(
621 - 'div',
622 - array(
623 - 'style' => 'background: ' . ( $i % 6 == 0 ? 'white' : '#dddddd' ) . ';'
624 - ),
625 - $rowHtml
626 - );
627 - $rowHtml = '';
628 - }
 628+ $resultHtml .= Html::rawElement(
 629+ 'div',
 630+ array(
 631+ 'style' => 'background: ' . ( $i % 6 == 0 ? 'white' : '#dddddd' ) . ';'
 632+ ),
 633+ $rowHtml
 634+ );
 635+ $rowHtml = '';
629636 }
630637
631638 return $resultHtml;
——@@ -671,15 +678,14 @@
672679
673680 /**
674681 * Get the HTML for a single parameter input.
 682+ * A helper method for showFormatOptions()
675683 *
676684 * @param Parameter $parameter
677 - * @param mixed $currentValue
 685+ * @param mixed $currentValue curretly set value of the parameter, or false if unknown.
678686 *
679 - * @return string
680 - * @todo Change method name to reflect behaviour (it does not "show" anything).
681 - * @todo Document what kind of types are expected for $currentValue, or at least say what its meaning is.
 687+ * @return string generated HTML
682688 */
683 - private function showFormatOption( Parameter $parameter, $currentValue ) {
 689+ private function getFormatOption( Parameter $parameter, $currentValue ) {
684690 $input = new ParameterInput( $parameter );
685691 $input->setInputName( 'p[' . $parameter->getName() . ']' );
686692
——@@ -702,18 +708,9 @@
703709 * @return string
704710 */
705711 protected function getFormatSelectBox( $defaultformat = 'broadtable' ) {
706 - global $smwgResultFormats, $smwgJQueryIncluded, $wgOut;
 712+ global $smwgResultFormats, $wgOut;
707713
708 - /// @todo The very same code for JQuery inclusion occurs multiple times. Should be a helper function.
709 - if ( !$smwgJQueryIncluded ) {
710 - $realFunction = array( 'OutputPage', 'includeJQuery' );
711 - if ( is_callable( $realFunction ) ) {
712 - $wgOut->includeJQuery();
713 - } else { ///@bug $scripts is undefined and not used later on
714 - $scripts[] = "$smwgScriptPath/libs/jquery-1.4.2.min.js";
715 - }
716 - $smwgJQueryIncluded = true;
717 - }
 714+ $this->enableJQuery();
718715
719716 // checking argument
720717 $default_format = 'broadtable';
——@@ -725,7 +722,7 @@
726723 $printer = SMWQueryProcessor::getResultPrinter( $default_format, SMWQueryProcessor::SPECIAL_PAGE );
727724 $url = $this->getTitle()->getLocalURL( "showformatoptions=' + this.value + '" );
728725
729 - foreach ( $this->uiCore->getParams() as $param => $value ) {
 726+ foreach ( $this->uiCore->getParameters() as $param => $value ) {
730727 if ( $param !== 'format' ) {
731728 $url .= '&params[' . Xml::escapeJsString( $param ) . ']=' . Xml::escapeJsString( $value );
732729 }
——@@ -746,7 +743,7 @@
747744 }
748745 natcasesort( $formats );
749746
750 - $params = $this->uiCore->getParams();
 747+ $params = $this->uiCore->getParameters();
751748 foreach ( $formats as $format => $name ) {
752749 $result .= '<option value="' . $format . '"' . ( $params['format'] == $format ? ' selected' : '' ) . '>' . $name . "</option>\n";
753750 }
——@@ -872,7 +869,6 @@
873870 *
874871 * @author Devayon Das
875872 *
876 - * @todo The is_a function is deprecated in PHP and instanceof should be used instead. In many cases as simple check for "is_null" would be even better.
877873 */
878874 class SMWQueryUIHelper {
879875
——@@ -1002,7 +998,7 @@
1003999 * @return boolean
10041000 */
10051001 public function hasFurtherResults() {
1006 - if ( is_a( $this->queryResult, 'SMWQueryResult' ) ) { // The queryResult may not be set
 1002+ if ( !is_null( $this->queryResult ) ) { // The queryResult may not be set
10071003 return $this->queryResult->hasFurtherResults();
10081004 } else {
10091005 return false;
——@@ -1180,6 +1176,8 @@
11811177 * Processes the QueryString, Params, and PrintOuts.
11821178 *
11831179 * @todo Combine this method with execute() or remove it altogether.
 1180+ * @todo for wikilink context, try to avoid computation if no query is set,
 1181+ * also check for pagination problems, if any.
11841182 */
11851183 public function extractParameters( $p ) {
11861184 if ( $this->context == self::SPECIAL_PAGE ) {
——@@ -1312,7 +1310,7 @@
13131311 * @return int
13141312 */
13151313 public function getResultCount() {
1316 - if ( is_a( $this->queryResult, 'SMWQueryResult' ) ) {
 1314+ if ( !is_null( $this->queryResult ) ) {
13171315 return $this->queryResult->getCount();
13181316 } else {
13191317 return 0;
——@@ -1323,9 +1321,8 @@
13241322 * Returns the parameter array.
13251323 *
13261324 * @return array
1327 - * @todo Always avoid abbreviations (unless they are a special technical term used everywhere). Call this getParameters().
13281325 */
1329 - public function getParams() {
 1326+ public function getParameters() {
13301327 return $this->parameters;
13311328 }
13321329
——@@ -1336,26 +1333,29 @@
13371334 */
13381335 public function getPrintOuts() {
13391336 if ( !empty( $this->printOuts ) &&
1340 - is_a( $this->printOuts[0], 'SMWPrintRequest' ) ) {
 1337+ ( $this->printOuts[0] instanceof SMWPrintRequest ) ) {
13411338 return $this->printOuts;
13421339 }
13431340 return array();
13441341 }
13451342
13461343 /**
1347 - * Constructs a new SMWQueryUIHelper when parameters are passed in the
1348 - * InfoLink style.
 1344+ * Constructs a new SMWQueryUIHelper object when the query is passed to
 1345+ * the UI in the Info-link format. This constructor should be used for
 1346+ * handling the "further results" links in wiki-pages that use #ask. If
 1347+ * your search UI handles form parameters only, then consider using
 1348+ * makeForUI().
 1349+ *
 1350+ * If any errors do occur while parsing parameters, they may be accessed
 1351+ * from hasError() and getErrors().
13491352 *
1350 - * Errors, if any can be accessed from hasError() and getErrors()
1351 - *
1352 - * @param string $p parameters
 1353+ * @param string $p parameters of the query.
13531354 * @param boolean $enable_validation
13541355 * @return SMWQueryUIHelper
13551356 *
1356 - * @todo The above documentation contains an unclear sequence of words that do not form a sentence.
13571357 * @todo Handle validation for infolink parameters
13581358 */
1359 - public static function makeForInfoLink( $p, $enable_validation = true ) {
 1359+ public static function makeForInfoLink( $p, $enable_validation = false ) {
13601360 $result = new SMWQueryUIHelper( self::WIKI_LINK );
13611361 $result->extractParameters( $p );
13621362 $result->execute();
——@@ -1363,10 +1363,14 @@
13641364 }
13651365
13661366 /**
1367 - * Constructs a new SMWQueryUIHelper when arguments are extracted from
1368 - * the UI.
 1367+ * Constructs a new SMWQueryUIHelper when the query is passed to the UI
 1368+ * from a web form. This constructor should be used to handle form
 1369+ * parameters sent from the UI itself. If your search UI must also handle
 1370+ * "further results" links from a wiki page, consider using
 1371+ * makeForInfoLink().
13691372 *
1370 - * Errors, if any can be accessed from hasError() and getErrors()
 1373+ * If any errors do occur while parsing parameters, they may be accessed
 1374+ * from hasError() and getErrors().
13711375 *
13721376 * @param string $query
13731377 * @param array $params of key=>value pairs
——@@ -1374,9 +1378,8 @@
13751379 * @param boolean $enable_validation
13761380 * @return SMWQueryUIHelper
13771381 *
1378 - * @todo The above documentation contains an unclear sequence of words that do not form a sentence.
13791382 */
1380 - public static function makeForUI( $query, array $params, array $printouts, $enable_validation = true ) {
 1383+ public static function makeForUI( $query, array $params, array $printouts, $enable_validation = false ) {
13811384 $result = new SMWQueryUIHelper( self::SPECIAL_PAGE );
13821385 $result->setParams( $params, $enable_validation );
13831386 $result->setPrintOuts( $printouts, $enable_validation );
——@@ -1389,8 +1392,8 @@
13901393 /**
13911394 * Checks if $property exists in the wiki or not.
13921395 *
1393 - * @return bool
1394 - * @todo Document parameter type and format.
 1396+ * @param string $property a property name in "?property" format
 1397+ * @return boolean
13951398 */
13961399 protected static function validateProperty( $property ) {
13971400 /*
——@@ -1399,7 +1402,7 @@
14001403 */
14011404 $prop = substr( $property, 1 ); // removing the leading '?' while checking.
14021405 $propertypage = Title::newFromText( $prop, SMW_NS_PROPERTY );
1403 - if ( is_a( $propertypage, 'Title' ) ) {
 1406+ if ( $propertypage instanceof Title ) {
14041407 return( $propertypage->exists() );
14051408 } else {
14061409 return false;

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r92056reviewed code, smaller improvements in style and logic, some comments/todos f...mkroetzsch11:32, 13 July 2011