far2l icon indicating copy to clipboard operation
far2l copied to clipboard

"Use case sensitive sort" in panels strange logic

Open akruphi opened this issue 1 year ago • 7 comments

В тг-чате обратили внимание на странное поведение в far2l "Сортировка с учётом регистра" на панелях:

в mc вроде логично, без учёта: X x Y y Z z с учётом: X Y Z x y z

в far2l без учёта: X x Y y Z z с учётом: x X y Y z Z

Экспресс осмотр кода выявил забавную матрёшку, выносящую мозг:

В коде CaseSensitiveSort вроде учитывается в filelist.cpp, но внутри это выбор вызова StrCmpI() и StrCmp() из locale.hpp. Они вызывают одну и ту же CompareString() лишь добавляя к флаг NORM_IGNORECASE. Сама CompareString() из APIStringMap.cpp вызывает wine_compare_string() из sortkey.c, которая внутри себя в случае результата с UNICODE_WEIGHT похоже не доходит до учёта обработки NORM_IGNORECASE. 🤔

Теперь уже стало просто интересно - работает ли этот вариант сортировки так как был задуман?

akruphi avatar Feb 11 '24 20:02 akruphi

простенький тест

fprintf(stderr, "%d %d %d\n", StrCmpI(L"Z", L"z"), StrCmpI(L"z", L"Z"), StrCmp(L"z", L"Z"));
fprintf(stderr, "%d %d %d\n", StrCmpI(L"X", L"x"), StrCmpI(L"x", L"X"), StrCmp(L"x", L"X"));

выдает

0 0 -1
0 0 -1

что вроде адекватно, надо посмотреть..

elfmz avatar Feb 11 '24 20:02 elfmz

Хотя что тут посмотреть, z и Z получается одинаковые строки, и оно их располагает в зависимости от фазы Луны. А точнее в зависимости от порядка, в котором выводит список readdir, его можно посмотреть командой ls -U Если в конце SortList вместо

	if (!NameCmp)
		NameCmp = (SPtr1->Position > SPtr2->Position) ? 1 : -1;

сделать так

	if (!NameCmp) {
		if (!ListCaseSensitiveSort) {
			NameCmp = StrCmp(Name1, Name2);
		}
		if (!NameCmp) {
			NameCmp = (SPtr1->Position > SPtr2->Position) ? 1 : -1;
		}
	}

то результат сравнения становится более определенным

elfmz avatar Feb 11 '24 20:02 elfmz

ща норм?

elfmz avatar Feb 11 '24 21:02 elfmz

ща норм?

Стало лучше, но всё-равно с опцией не соответствует человеческим ожиданиям - сначала заглавные, а потом строчные, а получается ровно наоборот и при этом влияет состояние Numeric sort 🤔

Например для каталога far2l/far2l/src

  • без опции Use case sensitive sort как без, так и при Numeric sort более-менее ожидаемо изображение

  • с опцией Use case sensitive sort без Numeric sort так же, что наверное тут ожидаемо изображение

  • с опцией Use case sensitive sort при Numeric sort совсем странный порядок изображение

Для тестовых имен файлов совсем уже странность:

  • без опции Use case sensitive sort как без, так и при Numeric sort более-менее ожидаемо изображение

  • с опцией Use case sensitive sort без Numeric sort вопреки ожиданиям строчные раньше заглавных изображение

  • с опцией Use case sensitive sort при Numeric sort совсем странный порядок изображение

В общем это опция не первоочередная задача, но в ней явно непорядок. И возможно эта странность с StrCmpI и StrCmp также причина https://github.com/elfmz/far2l/issues/1903

akruphi avatar Feb 12 '24 15:02 akruphi

ну, то что a идет раньше чем A это походу by design у CompareString. А неконсистности вроде дофиксил тока что

elfmz avatar Feb 12 '24 18:02 elfmz

Вроде работает. Но ощущение странности не пропадает - может только из-за строчных перед прописными...

akruphi avatar Feb 12 '24 19:02 akruphi

Всё же интуитивно ожидаешь чего-то такого, как в Midnight Commander и Double Commander. midnight double

Впрочем, даже как и в FAR на Windows: far

spnethw avatar Feb 28 '24 16:02 spnethw

я там еще кое что подфиксил..

elfmz avatar Mar 10 '24 18:03 elfmz

я там еще кое что подфиксил..

Стало поинтересней:

BraveSoftware
bleachbit
btop
Softdeluxe
session
smplayer
VeraCrypt
VirtualBox
VSCodium
vlc

Но всё равно не тот результат, который ожидаешь:

BraveSoftware
Softdeluxe
VeraCrypt
VirtualBox
VSCodium
bleachbit
btop
session
smplayer
vlc

Цитаты с тг-чата за разным авторством, может быть, натолкнет на какие мысли:

В коде CaseSensitiveSort вроде учитывается в filelist.cpp, но внутри это выбор вызова StrCmpI() и StrCmp() из locale.hpp. А далее забавная матрёшка: они вызывают одну и ту же CompareString() лишь добавляя к флаг NORM_IGNORECASE. А CompareString() из APIStringMap.cpp вызывает wine_compare_string() из sortkey.c, которая внутри себя в случае unicode похоже не доходит до учёта обработки NORM_IGNORECASE.

С wine_compare_string() отсортировать строки с учётом регистра "сначала заглавные A-B-C, потом строчные a-b-c" не выходит. Похоже, регистр просто игнорируется. Но если использовать wcscmp, то всё сортируется "как надо". Возможно, стоит переделать на использование wcscmp? Вопрос в том, какие подводные камни.

Например, в случае текстов на русском, буквы ё и Ё будут при сортировке обрабатываться неправильно.

Вызовы разных элементов этой матрёшки StrCmpI(), StrCmpN() и т.п. много где разбросаны по far2l и прежде чем менять лежащее под ними хорошо бы наготовить тестов на их работу. Без тестов тут легко прошляпить логику.

Я бы предложил пойти другим путем, и добавить еще один ряд сортировок в меню. Которые бы сортировали через wcs* вместо унаследованного кода. И посмотреть на фидбек - понравится людям или нет... А то libc-шный collate он странный, и зависит от LANG/LC_*. На многих системах может оказаться кодировка, отличная от utf-8, на таких сортировка знатно может удивить :-)

я сходил проверить в mc. да, они сделали и на первый взгляд выглядит вполне адекватно. https://github.com/MidnightCommander/mc/blob/d792f707fc486f6239715f216d5d59c1799d4274/lib/strutil/strutilutf8.c#L1348 (в keygen передается g_utf8_collate_key из glib).

spnethw avatar Mar 10 '24 21:03 spnethw

Но всё равно не тот результат, который ожидаешь:

На самом деле сделать так - изменение в 10 строк, но я даже не знаю, я например уже привык что файлы начинающиеся на одну букву рядом расположены...

elfmz avatar Mar 10 '24 21:03 elfmz

Вот изменение если что:

diff --git a/WinPort/wineguts/sortkey.c b/WinPort/wineguts/sortkey.c
index 79ef86a7..21057c2f 100644
--- a/WinPort/wineguts/sortkey.c
+++ b/WinPort/wineguts/sortkey.c
@@ -184,15 +184,7 @@ static void eval_weights(WCHAR ch1, WCHAR ch2, unsigned int types, unsigned int
         w2 = ch2;
     *r1 = 0;
     *r2 = 0;
-    if (types & UNICODE_WEIGHT) {
-        *r1 = w1 >> 16;
-        *r2 = w2 >> 16;
-    }
-    if ((*r1 == *r2) && (types & DIACRITIC_WEIGHT) != 0) {
-        *r1 = (w1 >> 8) & 0xff;
-        *r2 = (w2 >> 8) & 0xff;
-    }
-    if ((*r1 == *r2) && (types & CASE_WEIGHT) != 0) {
+    if ((types & CASE_WEIGHT) != 0) {
         *r1 = (w1 >> 4) & 0x0f;
         *r2 = (w2 >> 4) & 0x0f;
         if (*r1 && *r2 && *r1 != *r2) {
@@ -202,6 +194,14 @@ static void eval_weights(WCHAR ch1, WCHAR ch2, unsigned int types, unsigned int
             *r2 = t;
         }
     }
+    if ((*r1 == *r2) && (types & DIACRITIC_WEIGHT) != 0) {
+        *r1 = (w1 >> 8) & 0xff;
+        *r2 = (w2 >> 8) & 0xff;
+    }
+    if ((*r1 == *r2) && (types & UNICODE_WEIGHT) != 0) {
+        *r1 = w1 >> 16;
+        *r2 = w2 >> 16;
+    }
 }

elfmz avatar Mar 10 '24 21:03 elfmz

Когда-то давно ещё со времён DOS я был очень привычен к такой сортировке: сначала верхний регистр от A до Z, затем нижний от a до z. Но Far3 постепенно отучил меня от этого, и сейчас лично мне такая сортировка абсолютно без надобности, она мне представляется экзотической. Понимаю, что о вкусах не спорят.

shmuz avatar Mar 10 '24 22:03 shmuz

я например уже привык что файлы начинающиеся на одну букву рядом расположены...

ну так это по идее "сортировка без учёта регистра"... 🤔 с учётом-то смысл как раз в том, чтобы сначала прописные, потом строчные (как делает mc). впрочем, у Double Commander, например, такая настройка есть:

dc

spnethw avatar Mar 10 '24 22:03 spnethw

Когда-то давно ещё со времён DOS я был очень привычен к такой сортировке: сначала верхний регистр от A до Z, затем нижний от a до z. Но Far3 постепенно отучил меня от этого, и сейчас лично мне такая сортировка абсолютно без надобности, она мне представляется экзотической. Понимаю, что о вкусах не спорят.

Да ну тут не столько дело вкуса, сколько за работоспособность самой опции. Или выкинуть тогда нафиг её из режимов сортировки, чтоб не смущала пользователей, и код, с ней связанный. 🤷‍♂️ Впрочем, получившийся сейчас вариант уже неплох.

spnethw avatar Mar 10 '24 22:03 spnethw

На самом деле сделать так - изменение в 10 строк, но я даже не знаю, я например уже привык что файлы начинающиеся на одну букву рядом расположены...

Я за то, чтобы добавить в Ctrl+F12 второй вариант сортировки и дать пользователям выбор что использовать.

akruphi avatar Mar 11 '24 09:03 akruphi

ну ща уже нормально?

elfmz avatar Mar 11 '24 11:03 elfmz

ну ща уже нормально?

Почти норм. Начинающиеся точки можно и совсем наверх отсортировывать, последним фиксом точка лишь выше алфавитных поднята. изображение

akruphi avatar Mar 11 '24 11:03 akruphi

При включенном "Use case sensitive sort" точка теперь первая :+1: изображение а при выключенном пропускает небуквенные символы вперёд изображение

Дофиксить бы.

akruphi avatar Mar 11 '24 11:03 akruphi

Цифры влезают между строчными и заглавными. Плюс сбилась сортировка кириллицы (в предыдущем коммите было норм).

Ä
Ernährung
Ö
Ü
ÜFhler
Ügrossschreiben
1
150
2
999
ä
book
ernährung
kind
ö
ü
üFhler
ükleingeschrieben
zoom
А
а
Б
б
В
в
Поросенок
пингвин
Тест
тест
чертенок

spnethw avatar Mar 11 '24 12:03 spnethw

...замечания к текущему варианту?

elfmz avatar Mar 11 '24 13:03 elfmz

...замечания к текущему варианту?

При беглом взгляде идеально.

akruphi avatar Mar 11 '24 13:03 akruphi

В связи с последними изменениями автоматическое включение Use case sensitive sort даёт не всегда удобные последствия, например быстрый поиск Alt+буква сначала к заглавным, а потом долго ниже искать строчные. Может перевести в ConfigOpt.cpp параметры NSecPanelLeft, "CaseSensitiveSortNix" и NSecPanelRight, "CaseSensitiveSortNix" по умолчанию в 0 ? Если кому надо докопаются и включат, а остальных новичков не будет сходу удивлять.

akruphi avatar Mar 11 '24 13:03 akruphi

Вот а я о чем писал, но - можно, почему нет...

elfmz avatar Mar 11 '24 14:03 elfmz

Вот а я о чем писал, но - можно, почему нет...

А можно в Ctrl+F12 прописать 2 варианта регистрочувствительной сортировки и на CaseSensitiveSortNix=1 оставить старую, а на CaseSensitiveSortNix=2 повесить новую.

akruphi avatar Mar 11 '24 14:03 akruphi

...замечания к текущему варианту?

При беглом взгляде идеально.

Угу. За исключением того, что кириллица опять не. 👀

Алиса
анкета
Боб
бриллиант
Виктор
восторг

В общем, задачка сложная оказалась. 😕

spnethw avatar Mar 11 '24 14:03 spnethw

Угу. За исключением того, что кириллица опять не. 👀

Алиса
анкета
Боб
бриллиант
Виктор
восторг

Мда. В регистрочувствительной выдаёт как в нечувствительной :cry:

akruphi avatar Mar 11 '24 14:03 akruphi

Ну сейчас то можно закрыть уже баг?) Замечу что по дефолту сейчас case sensitive отключен, чтоб минимизировать изменения UX. Кроме того в виндовом фаре такой же дефолт

elfmz avatar Mar 11 '24 20:03 elfmz

Ну сейчас то можно закрыть уже баг?) Замечу что по дефолту сейчас case sensitive отключен, чтоб минимизировать изменения UX. Кроме того в виндовом фаре такой же дефолт

Наверное, можно закрыть. Скорее всего, ещё что-то вылезет, но сейчас хоть опция небесполезная, показывает что-то похожее на правду, да. 🤪

spnethw avatar Mar 11 '24 20:03 spnethw

Закрываю.

Спасибо @elfmz - такая огромная работа проделана.

akruphi avatar Mar 12 '24 05:03 akruphi