r66872 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r66871‎ | r66872 | r66873 >
Date:13:33, 25 May 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
bug 23524 - &url for ApiQueryIWLinks to expand full URL
Modified paths:
  • /trunk/phase3/includes/api/ApiQueryIWLinks.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiQueryIWLinks.php
@@ -54,6 +54,14 @@
5555
5656 $this->addTables( 'iwlinks' );
5757 $this->addWhereFld( 'iwl_from', array_keys( $this->getPageSet()->getGoodTitles() ) );
 58+
 59+ $url = !is_null( $params['url'] );
 60+ if ( $url ) {
 61+ $this->addTables( 'interwiki' );
 62+ $this->addJoinConds( array( 'interwiki' => array( 'INNER JOIN', 'iw_prefix=iwl_prefix' ) ) );
 63+ $this->addFields( 'iw_url' );
 64+ }
 65+
5866 if ( !is_null( $params['continue'] ) ) {
5967 $cont = explode( '|', $params['continue'] );
6068 if ( count( $cont ) != 3 ) {
@@ -91,6 +99,12 @@
92100 break;
93101 }
94102 $entry = array( 'prefix' => $row->iwl_prefix );
 103+
 104+ if ( $url ) {
 105+ $rowUrl = str_replace( '$1', $row->iwl_title, $row->iw_url );
 106+ $entry = array_merge( $entry, array( 'url' => $rowUrl ) );
 107+ }
 108+
95109 ApiResult::setContent( $entry, $row->iwl_title );
96110 $fit = $this->addPageSubItem( $row->iwl_from, $entry );
97111 if ( !$fit ) {
@@ -103,6 +117,7 @@
104118
105119 public function getAllowedParams() {
106120 return array(
 121+ 'url' => null,
107122 'limit' => array(
108123 ApiBase::PARAM_DFLT => 10,
109124 ApiBase::PARAM_TYPE => 'limit',
@@ -116,6 +131,7 @@
117132
118133 public function getParamDescription() {
119134 return array(
 135+ 'url' => 'Whether to get the full URL',
120136 'limit' => 'How many interwiki links to return',
121137 'continue' => 'When more results are available, use this to continue',
122138 );

Follow-up revisions

RevisionCommit summaryAuthorDate
r66885Followup r66872...reedy18:34, 25 May 2010
r66891* (bug 23524) Api Modules as followup to bug 14473 (Add iwlinks table to trac...reedy19:50, 25 May 2010
r77080Per r77072 CR, followup r66872 (seemingly my crappy code from 5 months ago) l...reedy17:27, 21 November 2010

Comments

#Comment by Duplicatebug (talk | contribs)   19:47, 24 March 2011

Why is this not an bool?

#Comment by Reedy (talk | contribs)   19:48, 24 March 2011

Cause it really doesn't matter?

You can do &iwurl=true and it'll work fine

#Comment by Duplicatebug (talk | contribs)   19:56, 24 March 2011

No, I can also use &iwurl=false and it is true because the string is not null (that is also true for bool, I know).

prop=langlinks&llurl is a bool, and it is nice to get the right type with action=paraminfo also for this module.

#Comment by Reedy (talk | contribs)   20:14, 24 March 2011

It's the same for our boolean outputs

blah="" is true, nothing at all is false. Should we be changing those aswell?

#Comment by Duplicatebug (talk | contribs)   20:23, 24 March 2011

No, that output is ok.

But setting the right type for action=paraminfo is nice to get the right input. I was looking at the paraminfo and I wondered why it is a string. I cannot input a value like the other params.

#Comment by Reedy (talk | contribs)   20:40, 24 March 2011

Even if we set it to ApiBase::PARAM_TYPE => 'boolean', and then use &iwurl=false, it'll still register as true

So it's a lie anyway....

#Comment by Duplicatebug (talk | contribs)   18:06, 25 March 2011

'bool' is only for documentation, other than 'integer'.

I was only wondered why it is a string and why prop=langlinks&llurl is defined other than prop=iwlinks&iwurl but has the same text.

But leave it, it not wrong.

#Comment by Catrope (talk | contribs)   11:49, 27 March 2011

Yes, it is wrong. Boolean parameters are set with 'foo' => false, not 'foo' => null,

#Comment by Duplicatebug (talk | contribs)   10:25, 14 June 2011

It there any action to change this? Or it is better to open a bug, because it is easy to track (for you)? Thanks.

#Comment by Duplicatebug (talk | contribs)   19:32, 2 July 2011

Status & tagging log