Page 2 of 5

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:00
by NightRoman

Latest post of the previous page:

http://bugs.farmanager.com/view.php?id=1087
Еще про диалоги при ошибках в SetPanelDirectory. Не убедил. В итоге приходится самому до вызова проверять, что директория есть. В случае с хитрыми и длинными путями это еще и не так просто в Фарнет сделать. Нелепо это делать, а приходится, чтобы избегать диалога. (Авто-тесты не должны вставать из-за диалога, но это не единственная причина).

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:09
by Shmuel
DrKnS wrote:Мне не сильно нравится новый код возврата тем, что ProcessName как бы BOOL, и если кто-то в коде делал так: [...]
- то условие тихо и незаметно сработает и для PNR_BADMASK тоже.
Но мы же ещё не закончили слом API, поэтому можно просто задокументировать ф-цию как возвращающую int, а в changelog'е упомянуть о несовместимости. Нельзя разрулить эту ситуацию, ничего не ломая (ну не добавлять же новую функцию).

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:27
by NightRoman
А я разделяю опасения DrKnS. Если просто поменять код возврата, то все будет продолжать работать, но неправильно. Это опасно, пользователи ченжлоги не читают, плагины не обновляют, ибо "вроде работает". А когда заметят, что не те файлы удалялись по маскам, будет поздно.

Давайте добавим новую функцию, а старая пусть возращает отлуп всегда. Через разумное время старую из апи убрать.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:50
by Alexey Samlyukov
не годится. оставить одну, но в аргументах структура и код возврата int.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:50
by NightRoman
DrKnS wrote:Мне не сильно нравится новый код возврата тем, что ProcessName как бы BOOL, и если кто-то в коде делал так:

Code: Select all

if(fsf.processname(blabla))
{
   MyHadronCollider->Collapse();
}
- то условие тихо и незаметно сработает и для PNR_BADMASK тоже.

Можете считать паранойей, да.
Тогда так (в новой! функциии)

Code: Select all

enum ProcessNameReturn {
    PNR_MATCH=0,
    PNR_NOMATCH=1,
    PNR_BADMASK=2
};
Это не гарантия от краха коллайдера по ошибке, но гораздо спокойней.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:52
by NightRoman
Alexey Samlyukov wrote:не годится. оставить одну, но в аргументах структура и код возврата int.
А что будет вызываться у тех, кто не обновит плагины?

Re: Маски файлов

Posted: Fri 02 Mar, 2012 09:57
by Denis
Сейчас самый слом апи, так что если ломать, то лучше чем сейчас не будет :)

Re: Маски файлов

Posted: Fri 02 Mar, 2012 10:04
by Shmuel
Ну зачем же две функции для одного и того же? Я всё надеюсь, что слом API скоро закончится, но сейчас-то ведь можно ломать. А насчёт того, что "пользователи ченджлоги не читают" - это проблема только этих пользователей.

Есть компромиссный вариант (выше уже упоминалось) - оставить функцию как возвращающую BOOL, но добавить флаг: PN_DISABLEMESSAGEBOX или PN_ENABLEMESSAGEBOX, в зависимости от выбора поведения по умолчанию. (Лично мне этот компромисс не нравится, но он существует).

Re: Маски файлов

Posted: Fri 02 Mar, 2012 11:17
by Shmuel
Как мне кажется, нашёл решение, которое ничего не ломает и всех должно устроить.
Просто добавить операцию PN_CHECKMASK (или PN_TESTMASK), первый параметр - маска, второй - игнорируется. Проверка маски на валидность.
Это логично - прежде, чем запускать поиск файла по маске на дереве, содержащем тысячи файлов, нужно сначала проверить маску.

Мнения?

Re: Маски файлов

Posted: Fri 02 Mar, 2012 11:29
by NightRoman
Shmuel wrote:Мнения?
Вариант, да. Может и не перфектный, но может сойти.

Минус один - парсинг два раза будет делаться (сначала плуг зовет проверку, потом работу).

UPD - Точнее, на один вызов будет больше, это может и неважно, если файлов много. А если мало, то невелика потеря от лишней проверки. Так что нормальный вариант, а минус небольшой.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 11:34
by Shmuel
Кстати, последнее решение может обойтись даже без дополнительного кода операции: если второй параметр передан как NULL, считать это операцией проверки маски.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 11:56
by DrKnS
Shmuel wrote:Мнения?
Флаги.

PN_SHOWERRORMESSAGE - показать сообщение, если маска не взлетела.
PN_VERBOSERESULT - вернуть подробный результат - <совпало/не совпало/кривая маска/тут_что_нибудь_еще_потом>

По умолчанию сообщения нет и результат булевый.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 12:08
by Shmuel
DrKnS wrote:Флаги.

PN_SHOWERRORMESSAGE - показать сообщение, если маска не взлетела.
PN_VERBOSERESULT - вернуть подробный результат - <совпало/не совпало/кривая маска/тут_что_нибудь_еще_потом>

По умолчанию сообщения нет и результат булевый.
Не очень-то понял. Вам не нравится моё последнее предложение? Почему?

Это, конечно, тоже годится, типа подсовывать какое-то имя "abc" вторым параметром, указывая PN_VERBOSERESULT только для того, чтобы проверить маску, но выглядит несколько криво. Но да ладно, красота - не главное, важно, что плагин получает полный контроль над ситуацией.

Re: Маски файлов

Posted: Fri 02 Mar, 2012 12:21
by DrKnS
Shmuel wrote: Не очень-то понял. Вам не нравится моё последнее предложение? Почему?
Перечитал еще раз - нравится.
При проверке только результат возвращать и дальше плагин сам, или пусть фар опционально сообщение показывает?

Re: Маски файлов

Posted: Fri 02 Mar, 2012 12:28
by Shmuel
Можно и так, и так (если сообщение опционально).

Re: Маски файлов

Posted: Fri 02 Mar, 2012 12:51
by NightRoman
Но все-таки на один парсинг будет больше. Может, все же одним вызовом все и решать - и работу (match|not-match), и проверку на валидость?