Skip to content

Commit

Permalink
Merge pull request #7254 from owncloud/core-sortalgo
Browse files Browse the repository at this point in the history
Fixed JS sort comparator to be consistent between JS and PHP
  • Loading branch information
LukasReschke committed Sep 16, 2014
2 parents 4669ea3 + f2001a4 commit d2743e6
Show file tree
Hide file tree
Showing 9 changed files with 527 additions and 4 deletions.
2 changes: 1 addition & 1 deletion apps/files/js/filelist.js
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,7 @@
if (fileInfo1.type !== 'dir' && fileInfo2.type === 'dir') {
return 1;
}
return fileInfo1.name.localeCompare(fileInfo2.name);
return OC.Util.naturalSortCompare(fileInfo1.name, fileInfo2.name);
},
/**
* Compares two file infos by size.
Expand Down
2 changes: 1 addition & 1 deletion apps/files/lib/helper.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static function compareFileNames($a, $b) {
} elseif ($aType !== 'dir' and $bType === 'dir') {
return 1;
} else {
return strnatcasecmp($a->getName(), $b->getName());
return \OCP\Util::naturalSortCompare($a->getName(), $b->getName());
}
}

Expand Down
52 changes: 51 additions & 1 deletion core/js/js.js
Original file line number Diff line number Diff line change
Expand Up @@ -1367,8 +1367,58 @@ OC.Util = {
// FIXME: likely to break when crossing DST
// would be better to use a library like momentJS
return new Date(date.getFullYear(), date.getMonth(), date.getDate());
},

_chunkify: function(t) {
// Adapted from http://my.opera.com/GreyWyvern/blog/show.dml/1671288
var tz = [], x = 0, y = -1, n = 0, code, c;

while (x < t.length) {
c = t.charAt(x);
// only include the dot in strings
var m = ((!n && c === '.') || (c >= '0' && c <= '9'));
if (m !== n) {
// next chunk
y++;
tz[y] = '';
n = m;
}
tz[y] += c;
x++;
}
return tz;
},
/**
* Compare two strings to provide a natural sort
* @param a first string to compare
* @param b second string to compare
* @return -1 if b comes before a, 1 if a comes before b
* or 0 if the strings are identical
*/
naturalSortCompare: function(a, b) {
var x;
var aa = OC.Util._chunkify(a);
var bb = OC.Util._chunkify(b);

for (x = 0; aa[x] && bb[x]; x++) {
if (aa[x] !== bb[x]) {
var aNum = Number(aa[x]), bNum = Number(bb[x]);
// note: == is correct here
if (aNum == aa[x] && bNum == bb[x]) {
return aNum - bNum;
} else {
// Forcing 'en' locale to match the server-side locale which is
// always 'en'.
//
// Note: This setting isn't supported by all browsers but for the ones
// that do there will be more consistency between client-server sorting
return aa[x].localeCompare(bb[x], 'en');
}
}
}
return aa.length - bb.length;
}
};
}

/**
* Utility class for the history API,
Expand Down
2 changes: 2 additions & 0 deletions core/js/tests/specHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ window.Snap.prototype = {
close: function() {}
};

window.isPhantom = /phantom/i.test(navigator.userAgent);

// global setup for all tests
(function setupTests() {
var fakeServer = null,
Expand Down
159 changes: 159 additions & 0 deletions core/js/tests/specs/coreSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -496,5 +496,164 @@ describe('Core base tests', function() {
});
});
});
describe('naturalSortCompare', function() {
// cit() will skip tests if running on PhantomJS because it has issues with
// localeCompare(). See https://github.com/ariya/phantomjs/issues/11063
//
// Please make sure to run these tests in Chrome/Firefox manually
// to make sure they run.
var cit = window.isPhantom?xit:it;

// must provide the same results as \OC_Util::naturalSortCompare
it('sorts alphabetically', function() {
var a = [
'def',
'xyz',
'abc'
];
a.sort(OC.Util.naturalSortCompare);
expect(a).toEqual([
'abc',
'def',
'xyz'
]);
});
cit('sorts with different casing', function() {
var a = [
'aaa',
'bbb',
'BBB',
'AAA'
];
a.sort(OC.Util.naturalSortCompare);
expect(a).toEqual([
'aaa',
'AAA',
'bbb',
'BBB'
]);
});
it('sorts with numbers', function() {
var a = [
'124.txt',
'abc1',
'123.txt',
'abc',
'abc2',
'def (2).txt',
'ghi 10.txt',
'abc12',
'def.txt',
'def (1).txt',
'ghi 2.txt',
'def (10).txt',
'abc10',
'def (12).txt',
'z',
'ghi.txt',
'za',
'ghi 1.txt',
'ghi 12.txt',
'zz',
'15.txt',
'15b.txt'
];
a.sort(OC.Util.naturalSortCompare);
expect(a).toEqual([
'15.txt',
'15b.txt',
'123.txt',
'124.txt',
'abc',
'abc1',
'abc2',
'abc10',
'abc12',
'def.txt',
'def (1).txt',
'def (2).txt',
'def (10).txt',
'def (12).txt',
'ghi.txt',
'ghi 1.txt',
'ghi 2.txt',
'ghi 10.txt',
'ghi 12.txt',
'z',
'za',
'zz'
]);
});
it('sorts with chinese characters', function() {
var a = [
'十.txt',
'一.txt',
'二.txt',
'十 2.txt',
'三.txt',
'四.txt',
'abc.txt',
'五.txt',
'七.txt',
'八.txt',
'九.txt',
'六.txt',
'十一.txt',
'波.txt',
'破.txt',
'莫.txt',
'啊.txt',
'123.txt'
];
a.sort(OC.Util.naturalSortCompare);
expect(a).toEqual([
'123.txt',
'abc.txt',
'一.txt',
'七.txt',
'三.txt',
'九.txt',
'二.txt',
'五.txt',
'八.txt',
'六.txt',
'十.txt',
'十 2.txt',
'十一.txt',
'啊.txt',
'四.txt',
'波.txt',
'破.txt',
'莫.txt'
]);
});
cit('sorts with umlauts', function() {
var a = [
'öh.txt',
'Äh.txt',
'oh.txt',
'Üh 2.txt',
'Üh.txt',
'ah.txt',
'Öh.txt',
'uh.txt',
'üh.txt',
'äh.txt'
];
a.sort(OC.Util.naturalSortCompare);
expect(a).toEqual([
'ah.txt',
'äh.txt',
'Äh.txt',
'oh.txt',
'öh.txt',
'Öh.txt',
'uh.txt',
'üh.txt',
'Üh.txt',
'Üh 2.txt'
]);
});
});
});

117 changes: 117 additions & 0 deletions lib/private/naturalsort.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
<?php
/**
* Copyright (c) 2014 Vincent Petry <PVince81@owncloud.com>
* This file is licensed under the Affero General Public License version 3 or
* later.
* See the COPYING-README file.
*
*/

namespace OC;

class NaturalSort_DefaultCollator {

public function compare($a, $b) {
if ($a === $b) {
return 0;
}
return ($a < $b) ? -1 : 1;
}
}

class NaturalSort {
private static $instance;
private $collator;

/**
* Split the given string in chunks of numbers and strings
* @param string $t string
* @return array of strings and number chunks
*/
private function naturalSortChunkify($t) {
// Adapted and ported to PHP from
// http://my.opera.com/GreyWyvern/blog/show.dml/1671288
$tz = array();
$x = 0;
$y = -1;
$n = null;
$length = strlen($t);

while ($x < $length) {
$c = $t[$x];
// only include the dot in strings
$m = ((!$n && $c === '.') || ($c >= '0' && $c <= '9'));
if ($m !== $n) {
// next chunk
$y++;
$tz[$y] = '';
$n = $m;
}
$tz[$y] .= $c;
$x++;
}
return $tz;
}

/**
* Returns the string collator
* @return Collator string collator
*/
private function getCollator() {
if (!isset($this->collator)) {
// looks like the default is en_US_POSIX which yields wrong sorting with
// German umlauts, so using en_US instead
if (class_exists('Collator')) {
$this->collator = new \Collator('en_US');
}
else {
$this->collator = new \OC\NaturalSort_DefaultCollator();
}
}
return $this->collator;
}

/**
* Compare two strings to provide a natural sort
* @param $a first string to compare
* @param $b second string to compare
* @return -1 if $b comes before $a, 1 if $a comes before $b
* or 0 if the strings are identical
*/
public function compare($a, $b) {
// Needed because PHP doesn't sort correctly when numbers are enclosed in
// parenthesis, even with NUMERIC_COLLATION enabled.
// For example it gave ["test (2).txt", "test.txt"]
// instead of ["test.txt", "test (2).txt"]
$aa = self::naturalSortChunkify($a);
$bb = self::naturalSortChunkify($b);
$alen = count($aa);
$blen = count($bb);

for ($x = 0; $x < $alen && $x < $blen; $x++) {
$aChunk = $aa[$x];
$bChunk = $bb[$x];
if ($aChunk !== $bChunk) {
if (is_numeric($aChunk) && is_numeric($bChunk)) {
$aNum = (int)$aChunk;
$bNum = (int)$bChunk;
return $aNum - $bNum;
}
return self::getCollator()->compare($aChunk, $bChunk);
}
}
return $alen - $blen;
}

/**
* Returns a singleton
* @return \OC\NaturalSort instance
*/
public static function getInstance() {
if (!isset(self::$instance)) {
self::$instance = new \OC\NaturalSort();
}
return self::$instance;
}
}

11 changes: 11 additions & 0 deletions lib/public/util.php
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,17 @@ public static function generateRandomBytes($length = 30) {
return \OC_Util::generateRandomBytes($length);
}

/**
* Compare two strings to provide a natural sort
* @param $a first string to compare
* @param $b second string to compare
* @return -1 if $b comes before $a, 1 if $a comes before $b
* or 0 if the strings are identical
*/
public static function naturalSortCompare($a, $b) {
return \OC\NaturalSort::getInstance()->compare($a, $b);
}

/**
* check if a password is required for each public link
* @return boolean
Expand Down
Loading

0 comments on commit d2743e6

Please sign in to comment.