Thiemo Mättig (WMDE) has uploaded a new change for review. https://gerrit.wikimedia.org/r/234298
Change subject: Partly revert "alphabetic" InterwikiSorter order ...................................................................... Partly revert "alphabetic" InterwikiSorter order This partly reverts patch I1b415cb. The problem is: What if the sort order array says [x, k, a] and you need to add the unknown value "b"? Where should it go? There is no alphabetic order the code can follow. I could pick a "random" position that looks like it follows an alphabetic order, which is what patch I1b415cb did. That implementation even made it *worse*: The example results in [a, b, x, k], so "b" is in an acceptable place, but "a" is misplaced now. This patch adds more tests and reimplements "unknown goes to the end, in an alphabetic order". Bug: T103044 Change-Id: Ib9add179a6f22fddd5a3a3b68d660588e56aa157 --- M client/includes/InterwikiSorter.php M client/tests/phpunit/includes/InterwikiSorterTest.php M docs/options.wiki 3 files changed, 47 insertions(+), 24 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/98/234298/1 diff --git a/client/includes/InterwikiSorter.php b/client/includes/InterwikiSorter.php index f0cb320..0497ab9 100644 --- a/client/includes/InterwikiSorter.php +++ b/client/includes/InterwikiSorter.php @@ -2,7 +2,7 @@ namespace Wikibase; -use MWException; +use InvalidArgumentException; /** * Language sorting utility functions. @@ -12,6 +12,7 @@ * @licence GNU GPL v2+ * @author Nikola Smolenski <[email protected]> * @author Katie Filbert < [email protected] > + * @author Thiemo Mättig */ class InterwikiSorter { @@ -31,9 +32,9 @@ private $sortPrepend; /** - * @var int[] + * @var int[]|null */ - private $sortOrder; + private $sortOrder = null; /** * @since 0.4 @@ -41,8 +42,16 @@ * @param string $sort * @param array[] $sortOrders * @param string[] $sortPrepend + * + * @throws InvalidArgumentException */ public function __construct( $sort, array $sortOrders, array $sortPrepend ) { + if ( !array_key_exists( 'alphabetic', $sortOrders ) ) { + throw new InvalidArgumentException( + 'alphabetic interwiki sorting order is missing from Wikibase Client settings.' + ); + } + $this->sort = $sort; $this->sortOrders = $sortOrders; $this->sortPrepend = $sortPrepend; @@ -59,12 +68,9 @@ * @return string[] */ public function sortLinks( array $links ) { - // Prepare the sorting array. - $this->sortOrder = $this->buildSortOrder( - $this->sort, - $this->sortOrders, - $this->sortPrepend - ); + if ( $this->sortOrder === null ) { + $this->sortOrder = $this->buildSortOrder( $this->sort, $this->sortOrders ); + } // Prepare the array for sorting. foreach ( $links as $k => $langLink ) { @@ -93,16 +99,25 @@ $a = $a[0]; $b = $b[0]; - if ( $a == $b ) { + if ( $a === $b ) { return 0; } $aIndex = array_key_exists( $a, $this->sortOrder ) ? $this->sortOrder[$a] : null; $bIndex = array_key_exists( $b, $this->sortOrder ) ? $this->sortOrder[$b] : null; - // If we encounter an unknown language, which may happen if the sort table is not updated, - // we list it alphabetically. - return $aIndex === null || $bIndex === null ? strcmp( $a, $b ) : $aIndex - $bIndex; + if ( $aIndex === $bIndex ) { + // If we encounter multiple unknown languages, which may happen if the sort table is not + // updated, we list them alphabetically. + return strcmp( $a, $b ); + } elseif ( $aIndex === null ) { + // Unknown languages must go under the known languages. + return 1; + } elseif ( $bIndex === null ) { + return -1; + } else { + return $aIndex - $bIndex; + } } /** @@ -110,16 +125,10 @@ * * @param string $sort * @param array[] $sortOrders - * @param string[] $sortPrepend * - * @throws MWException * @return int[] */ - private function buildSortOrder( $sort, array $sortOrders, array $sortPrepend ) { - if ( !array_key_exists( 'alphabetic', $sortOrders ) ) { - throw new MWException( 'alphabetic interwiki sorting order is missing from Wikibase Client settings.' ); - } - + private function buildSortOrder( $sort, array $sortOrders ) { $sortOrder = $sortOrders['alphabetic']; if ( $sort === 'alphabetic' ) { @@ -137,8 +146,8 @@ } } - if ( $sortPrepend !== array() ) { - $sortOrder = array_unique( array_merge( $sortPrepend, $sortOrder ) ); + if ( $this->sortPrepend !== array() ) { + $sortOrder = array_unique( array_merge( $this->sortPrepend, $sortOrder ) ); } return array_flip( $sortOrder ); diff --git a/client/tests/phpunit/includes/InterwikiSorterTest.php b/client/tests/phpunit/includes/InterwikiSorterTest.php index 03ac2f1..78b5db7 100644 --- a/client/tests/phpunit/includes/InterwikiSorterTest.php +++ b/client/tests/phpunit/includes/InterwikiSorterTest.php @@ -85,7 +85,21 @@ 'alphabetic', $sortOrders, array(), - array( 'a1', 'a2', 'de', 'en', 'x1', 'x2' ) + array( 'de', 'en', 'a1', 'a2', 'x1', 'x2' ) + ), + array( + array( 'f', 'd', 'b', 'a', 'c', 'e' ), + 'alphabetic', + array( 'alphabetic' => array( 'c', 'a' ) ), + array( 'e' ), + array( 'e', 'c', 'a', 'b', 'd', 'f' ) + ), + array( + array( 'a', 'b', 'k', 'x' ), + 'alphabetic', + array( 'alphabetic' => array( 'x', 'k', 'a' ) ), + array(), + array( 'x', 'k', 'a', 'b' ) ), ); } diff --git a/docs/options.wiki b/docs/options.wiki index a4834c2..9d63cb2 100644 --- a/docs/options.wiki +++ b/docs/options.wiki @@ -82,7 +82,7 @@ :;<code>'none'</code>: Don't sort. Basically, the order of the links is not guaranteed. '''Deprecated''' and dysfunctional. : Default is <code>'code'</code>. ;sortPrepend: List of language codes to put on top of the language links in the side bar. Default: <code>array()</code>. '''Note''': this may change to use global wiki IDs instead of language codes in the future. -;interwikiSortOrder: Specify a custom sort order for interwiki links; default options provided include alphabetic, code, alphabetic revised, alphabetic_sr, and alphabetic_fy. +;interwikiSortOrders: Array of arrays of language codes, specifying custom sort orders for interwiki links; default options provided include <code>'code'</code>, <code>'alphabetic'</code>, <code>'alphabetic_revised'</code>, <code>'alphabetic_sr'</code>, and <code>'alphabetic_fy'</code>. ;alwaysSort: Sort links from wikitext even if <code><nowiki>{{noexternallanglinks:*}}</nowiki></code> is used. Default: <code>true</code>. ;siteGlobalID: This site's global ID (e.g. <code>'itwiki'</code>), as used in the sites table. Default: <code>$wgDBname</code>. ;siteLocalID: This site's local ID resp. language code (e.g. <code>'it'</code>). Default: <code>$wgLanguageCode</code>. '''Note:''' this setting will be removed once we can take this information from the sites table. -- To view, visit https://gerrit.wikimedia.org/r/234298 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ib9add179a6f22fddd5a3a3b68d660588e56aa157 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
