From b9099f52879c75d1292b1fa3b13616b890c1b164 Mon Sep 17 00:00:00 2001 From: Philipp Wolfer Date: Sun, 29 Dec 2024 13:03:16 +0100 Subject: [PATCH] PICARD-3014: Fix macOS not sorting empty values before other entries --- picard/i18n.py | 18 ++++++++++++++---- test/test_i18n.py | 29 ++++++++++++++++------------- 2 files changed, 30 insertions(+), 17 deletions(-) diff --git a/picard/i18n.py b/picard/i18n.py index fe4d26810d..c2d1f62461 100644 --- a/picard/i18n.py +++ b/picard/i18n.py @@ -259,15 +259,25 @@ def _digits_replace(matchobj): def _sort_key_qt(string, numeric=False): collator = _qcollator_numeric if numeric else _qcollator + + # Null bytes can cause crashes in OS collation functions. + string = string.replace('\0', '') + # On macOS / Windows the numeric sorting does not work reliable with non-latin # scripts. Replace numbers in the sort string with their latin equivalent. if numeric and (IS_MACOS or IS_WIN): string = RE_NUMBER.sub(_digits_replace, string) - # On macOS numeric sorting of strings entirely consisting of numeric characters fails - # and always sorts alphabetically (002 < 1). Always prefix with an alphabetic character - # to work around that. - return collator.sortKey('a' + string.replace('\0', '')) + if IS_MACOS: + # macOS does not sort the empty string before other values correctly + if not string: + string = ' ' + # On macOS numeric sorting of strings entirely consisting of numeric + # characters fails and always sorts alphabetically (002 < 1). Always + # prefix with an alphabetic character to work around that. + string = 'a' + string + + return collator.sortKey(string) def _sort_key_strxfrm(string, numeric=False): diff --git a/test/test_i18n.py b/test/test_i18n.py index ac06d46e5b..7f6d7aad60 100644 --- a/test/test_i18n.py +++ b/test/test_i18n.py @@ -91,25 +91,28 @@ def test_gettext_handles_empty_string(self): def test_sort_key(self): setup_gettext(localedir, 'de') - self.assertTrue(sort_key('äb') < sort_key('ac')) - self.assertTrue(sort_key('foo002') < sort_key('foo1')) - self.assertTrue(sort_key('002 foo') < sort_key('1 foo')) - self.assertTrue(sort_key('1') < sort_key('C')) - self.assertTrue(sort_key('foo1', numeric=True) < sort_key('foo002', numeric=True)) - self.assertTrue(sort_key('004', numeric=True) < sort_key('5', numeric=True)) - self.assertTrue(sort_key('0042', numeric=True) < sort_key('50', numeric=True)) - self.assertTrue(sort_key('5', numeric=True) < sort_key('0042', numeric=True)) - self.assertTrue(sort_key('99', numeric=True) < sort_key('100', numeric=True)) + self.assertLess(sort_key('äb'), sort_key('ac')) + self.assertLess(sort_key('foo002'), sort_key('foo1')) + self.assertLess(sort_key('002 foo'), sort_key('1 foo')) + self.assertLess(sort_key('1'), sort_key('C')) + self.assertLess(sort_key(''), sort_key('0')) + self.assertLess(sort_key('\0'), sort_key('0')) + self.assertLess(sort_key('0'), sort_key('00')) + self.assertLess(sort_key('foo1', numeric=True), sort_key('foo002', numeric=True)) + self.assertLess(sort_key('004', numeric=True), sort_key('5', numeric=True)) + self.assertLess(sort_key('0042', numeric=True), sort_key('50', numeric=True)) + self.assertLess(sort_key('5', numeric=True), sort_key('0042', numeric=True)) + self.assertLess(sort_key('99', numeric=True), sort_key('100', numeric=True)) def test_sort_key_numbers_different_scripts(self): setup_gettext(localedir, 'en') for four in ('4', '𝟜', '٤', '๔'): - self.assertTrue( - sort_key('3', numeric=True) < sort_key(four, numeric=True), + self.assertLess( + sort_key('3', numeric=True), sort_key(four, numeric=True), msg=f'3 < {four}' ) - self.assertTrue( - sort_key(four, numeric=True) < sort_key('5', numeric=True), + self.assertLess( + sort_key(four, numeric=True), sort_key('5', numeric=True), msg=f'{four} < 5' )