Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luamacro: implement custom events (POC) #244

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johnd0e
Copy link
Contributor

@johnd0e johnd0e commented May 29, 2020

Пример использования:

define event handler:

Event { description="test!";
  group="CustomEvent"; -- any name
  action=function(Event,...)
    far.Show(Event,...)
  end;
}

emit event:

local opts = {filename="example.txt"}
mf.ProcessEvent("CustomEvent",opts,1,2,3)

Аргументы mf.ProcessEvent:

  1. group
  2. options - сфера применение не вполне ясна. Параметр заведён ради filename
    • filename можно и напрямую передавать, а не в таблице
    • но в большинстве случаев filename не будет иметь значения...
  3. остальные аргументы передаются обработчику

@johnd0e
Copy link
Contributor Author

johnd0e commented May 30, 2020

Данный PR создан исключительно как proof-of-concept этой идеи: https://bugs.farmanager.com/view.php?id=3796,
и (в текущем виде) предназначен не для слияния, а для того чтобы начать предметное обсуждение.

Дискуссионные моменты реализации:

  • Любое нестандартное значения group теперь является валидным.
  • mf.ProcessEvent может инициировать в том числе и штатные события.

В настоящий момент меня не устраивают следующие ограничения:

  1. Нестандартные поля в определении обработчика отбрасываются
    Для обычных макросов вопрос решён так:

    shmuel 07.12.2019 23:34:24 +0200 - build 717
    1. Regular macros: functions condition() and action() receive an additional argument - a table.
    This table can be used for keeping and modifying data that are private to the macro.
    It is initialized by the content of a table-argument received by function Macro.
    2. Regular macros: arbitrary non-standard fields are allowed, they will be copied to the table
    described in the above paragraph.

    Применить это же (в исходном виде) для событий проблематично.

  2. Хотелось бы добавить дополнительный механизм фильтрации событий, указанием дополнительных полей.
    Предварительная идея такова: обобщить идею фильтров таким образом, что каждая группа событий будет иметь свой массив фильров, в котором по умолчанию уже будет проверка condition (плюс для редактора/вьювера - проверка filemask). И предоставить возможность пользователю добавлять собственные фильтры.
    Функция-фильтр будет иметь доступ ко всем полям определения (в том числе нестандартным). Возможно этого будет достаточно чтобы обойтись без пункта 1.

  3. Сейчас возвращаемое значение обработчиков некоторых групп обрабатывается специальным образом, позволяя досрочно прервать обработку событий (и вернуть значение):

    for _,i in ipairs(indexes) do
    if priorities[i] < 0 then break end
    local ret = macros[i].action(...)
    if ret then
    if macros==Events.dialogevent or macros==Events.editorinput or macros==Events.commandline then
    return ret
    elseif macros==Events.consoleinput then
    if ret ~= 0 then return ret end
    end
    end
    end
    end

    Потенциально такое могло бы быть полезно и для пользовательских событий, а значит и эту обработку стоит обобщить.

@shmuz
Copy link
Contributor

shmuz commented May 30, 2020

Мне кажется, что предлагаемую функциональность можно реализовать Lua-модулем, не трогая плагин LuaMacro. Модуль мог бы "экспортировать" функции Event и ProcessEvent (это предложенные в верхнем сообщении имена, но можно взять и другие).

@johnd0e
Copy link
Contributor Author

johnd0e commented May 30, 2020

Абсолютно согласен что можно.
Однако с одной стороны какой-то маргинальный модуль, во многом повторяющий код из луамакро, и требующий отдельной установки (а то и выбора из нескольких конкурирующих реализаций).

А с другой стороны тривиальное изменение луамакро, дающее то же самое из коробки всем и сразу.

Та часть, что не реализована здесь (фильтры), имеет самостоятельную ценность (на мантисе отдельный тикет: https://bugs.farmanager.com/view.php?id=2595).

@johnd0e
Copy link
Contributor Author

johnd0e commented May 30, 2020

Хотелось бы иметь эту функциональность именно из коробки, в данном случае считаю что это оправданно.

Как недавно обсуждалось на форуме, один из основных юзкейсов - предоставление интерфейса для плагинов. Опираясь на предлагаемый механизмпроизвольный плагин может генерировать собственные события для макросов.

Например, автор колорера так сможет легко реализовать оповещение о смене типа файла в редакторе.

Этого не случится, если ему надо будет реализовывать весь механизм самому, или переадресовывать пользователей к какому-то стороннему скрипту.

@alabuzhev
Copy link
Contributor

@johnd0e, is it still relevant?

@johnd0e
Copy link
Contributor Author

johnd0e commented Oct 22, 2020

It was neither implemented nor rejected, so yes, I suppose it is still relevant.

@dr-dba
Copy link

dr-dba commented Sep 8, 2022

Многоуважаемый Алабужев и многоуважаемые все.
А можно ли узнать, чем закончилась история, неужто ничем ?
Я честно хочу понять. (без малейших наездов, а наоборот люблю весь FarGroup)
Если ничем, то пожалуйста пожалуйста пожалуйста скажите в чем был провал ?
Я вот собираюсь подогнать вам хорошо написанные и насущные для многих АПИ.
(синхронизированный форк лежит у меня, собираюсь сделать бранч для слияния,
в котором не будет моей вкусувщины в стилистике кода,
а будут только нетто инкрементальные патчи достаточные для функционала)
Проверить, оформить все как следует, итд.
Но если даже JohnDoe не может вручить Алабужеву хорошо оформленный небольшой патч ?
То стоит ли мне стараться для сотрудничества,
или лучше забыть про сотрудничество,
чтобы всем было спокойнее ?

ВОПРОС ВАЖНЫЙ!

спасибо заранее за любой ответ
если Алабужеву даже отвечать лень,
то такой ответ тоже понятен и тоже приветствуется
Алабужев все равно останется героем в моих глазах.

@dr-dba
Copy link

dr-dba commented Sep 8, 2022

Ок, я чуть углубился, и понял что этот патч от @johndoe в лучшем случае черновик :)

@dr-dba
Copy link

dr-dba commented Sep 8, 2022

А с другой стороны тривиальное изменение луамакро, дающее то же самое из коробки всем и сразу.

вот это очень правильно я щитаю.
только непонятно как "тривиальное изменение" стыкуется с фразами самого же @johdoe, - "не применим в текущем виде", "черновик" ? :)
(за дословность цитаты не ручаюсь, но вроде так писалось)
Насчет "предметного обсуждения".
Думаю что надо просто самому @johdoe подумать как надо, и дописать то что начал самому.
даже буде тут сотни жаждущих предметных обсуждений, все равно кто-то должен быть главнее.
А тут еще одного обсуждателя найти хотя бы..

@shmuz
Copy link
Contributor

shmuz commented Sep 8, 2022

Вообще данная идея, предложенная johnd0e, мне сейчас видится более привлекательной, чем изначально.
Только я бы не стал расширять функцию Event, а ввёл бы для этого новую функцию, например CustomEvent.

@alabuzhev
Copy link
Contributor

@dr-dba if you look at the top right corner, you might notice the "Draft" label.
I hope its meaning is obvious, but, just in case if not, it means that the PR is a work in progress and is not ready to be merged yet, because its author thinks so.

To merge it, the author needs to finish it first and mark it accordingly. This could happen tomorrow or in 10 years, depending on personal circumstances.

On top of that, the author has already mentioned in the comments above that it is a POC that needs a further discussion, so you are welcome to participate and clarify any outstanding topics.

Having said that, please do not expect a lot of activity from me personally in this particular PR: lua is not my area of expertise and I am not confident enough to review it. When the time will come, please talk to Shmuel. I will happily merge it with his blessing.

@dr-dba
Copy link

dr-dba commented Sep 10, 2022

Только я бы не стал расширять функцию Event,
а ввёл бы для этого новую функцию, например CustomEvent.

@shmuz, Потому что так проще ?
А архитектурно не будет ли тогда параллельных дублирующих друг друга струтктур кодa ?
Это вот думаю что было бы не правильно.
Но я не знаю насколько я могу судить, я пока не очень вник в архитектуру предложенного в черновике от @johndoe даже, и не видел вашего черновика.
Я так понимаю что @johndoe не хочет умножать сущностей, что тоже справедливо

@dr-dba
Copy link

dr-dba commented Sep 10, 2022

Применить это же (в исходном виде) для событий проблематично.

Цитирую то что я думаю как можно было бы решить:

Что надо поставить эту таблицу EventData первой.
Конечно же все АПИ плагинов сломаются, но других проблем я не вижу
(есть ли еще какие проблемы кроме плагиновских АПИ ?)

Теперь как починить сломанные АПИ, есть такие варианты:

1.) Для плагинов написанных на "нормальных" типизированных языках как Си и Паскаль, есть
полиморфизм методов (функций).
"полиморфизм" означает вариативность "сигнатуры" метода (функции) без измения в самом методе (функции)
Т.е., совсем не проблема, поправьте если где ошибаюсь.
Старые плагины работают как раньше, новым рекомендуется использовать новую форму вызова, все счастливы.

2.) Для нетипизированных обращений, как например из плагина LuaMacro.
Тут проблема серьезнее, ведь lua macro script не знает что сейчас первый параметр это уже не то что было раньше.
Но есть такие варианты, для Луа:
2.1.) И/ИЛИ: При инициализации (создании) макроса (в LuaFar/LuaMacro) для функций condition()/action() задается upvalue - таблица названная EventData.
2.2.) И/ИЛИ: Создать "полиморфизм" уже на уровне LuaFar/LuaMacro, как condition2() и action2(),
которые и будут принимать (EventData, ...)
Кривовато, но легитимно, и частенько (если не всегда, когда есть всякие legacies) развитие кода именно в таком направлении.
Старые макросы работают, а у новых есть EventData, все счастливы

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

@shmuz
Copy link
Contributor

shmuz commented Sep 10, 2022

@dr-dba

  1. Что касается расширения функции Event по сравнению с введением новой функции - это не принципиально.
  2. Что касается вашей длинной цитаты - мне трудно разобраться в её хитросплетениях. Могу только заметить, что оригинальное предложение касается только макросистемы и не влияет на API плагинов (в смысле не ломает).
  3. Реализовать данное предложение для меня вопрос считанных часов, но поскольку на данный момент у меня нет применения для этой новой фичи, соответственно нет и стимула.

@dr-dba
Copy link

dr-dba commented Sep 10, 2022

@shmuz

Что касается расширения функции Event по сравнению с введением новой функции - это не принципиально.

Ну зачем-то вы же предложили, и если кто-то будет продолжать пилить в этом направлении, то ему было бы полезно ваше мнение

Что касается вашей длинной цитаты - мне трудно разобраться в её хитросплетениях.

Цитата была ответом на раздумья Джона как безопасно поставить таблицу EventData первым параметром в action()/condition() обьекта Event.
(Первым потому что он "generic" как бэ, общего пользования)
Если не ставить первым, то будет непонятно каким по очереди, потому что у разных евентов количество входных параметров в функции action()/condition() может отличаться.

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

А и точно ведь, у "нормальных" плагинов (не макро скриптов) ведь такого понятия как condition()/action() вапще нет, и означенной проблемы соответвенно, спасибо за замечание

@shmuz
Copy link
Contributor

shmuz commented Sep 10, 2022

Ну зачем-то вы же предложили, и если кто-то будет продолжать пилить в этом направлении, то ему было бы полезно ваше мнение

Если вместо расширения ф-ции Event ввести новую функцию, то и вопроса куда ставить параметр, который вы называете EventData, не будет. Функция Event остаётся как есть, а с новой функцией у разработчика полная свобода рук.

@dr-dba
Copy link

dr-dba commented Sep 10, 2022

@shmuz,

Если вместо расширения ф-ции Event ввести новую функцию,
то и вопроса куда ставить параметр, который вы называете EventData, не будет.
Функция Event остаётся как есть, а с новой функцией у разработчика полная свобода рук.

(Писалось до того как я увидел ваш последний камент, но тем не менее по теме как оказалось:)
Т.е., вышеупомянутый дискуссионный момент у Джона это передача ЕвентДата в condition()/action() Евента,
как на мой взгляд, решается на уровне ЛуаФар/ЛуаМакро одним из двух способов:

1.) добавления новых луа-функций condition2(EvtDat, ...)/action2(EvtDat, ...) для евента
(старые condition/action остаются, если есть новые condition2/action2, то механизм ЛуаМакро использует только их игнорируя те старые)

2.) И/ИЛИ Механизм ЛуаФар/ЛуаМакро задает функциям condition()/action() некое upvalue именем EventData
(или это не через upvalue а через setfenv делается? ну вопчем в этой области так или иначе)

вот это хотелось чтобы вы прокомментировали
Если я ничего не перепутал опять, это и есть тяжкие раздумья Джона.
Ну я так понял, что вы предлагаете не заморачиваться с передачей EventData в прежний механизм Евентов,
а просто создать новый механизм CustomEvent, в котором руки будут полностью развязаны, что разумно конечно.
Мне все же больше нравятся condition2(EvtDat, ...)/action2(EvtDat, ...) и/или upvalue EvtDat
Для макросов соответственно можно было бы сделать condition2(McrDat, key, ...) и/или upvalue McrDat.
action2(McrDat, ..) для макросов не нужно, так как уже есть action(McrDat, ..)

@shmuz
Copy link
Contributor

shmuz commented Sep 10, 2022

@dr-dba , к сожалению у меня нет времени на бесконечные обсуждения.
Если johnd0e вернётся к данному вопросу, то он не хуже меня сумеет его реализовать.

define event handler:
```lua
Event { description="test!";
  group="CustomEvent";
  action=function(Event,...)
    far.Show(Event,...)
  end;
}
```

emit event:
```lua
mf.ProcessEvent("CustomEvent",{},1,2,3)
```
@johnd0e johnd0e force-pushed the lm-custom-events branch from 8a04edf to 0162c3e Compare March 7, 2023 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants