Skip to content

Allow registering multiple hooks of the same type#695

Closed
casparvl wants to merge 3 commits intoTACC:mainfrom
casparvl:allow_multiple_hooks
Closed

Allow registering multiple hooks of the same type#695
casparvl wants to merge 3 commits intoTACC:mainfrom
casparvl:allow_multiple_hooks

Conversation

@casparvl
Copy link

@casparvl casparvl commented Mar 6, 2024

Note: there is an alternative approach in another PR that I consider cleaner than the one in this PR (#695).

This PR stores hooks in Hook.lua internally as tables. It also adds the additional possibility of appending hooks to this table: if the argument passed to hook.register is a table, it gets appended. If it is a single function, it overwrites what was there. @rtmclay I'm not 100% sure if this is what you meant yesterday in the call by providing an alternative interface, but this was my understanding - if not, feel free to correct me.

It can be tested with this simple SitePackage.lua:

require("strict")
local hook = require("Hook")

local function load_hook_a(t)
    local frameStk  = require("FrameStk"):singleton()
    local mt        = frameStk:mt()
    local simpleName = string.match(t.modFullName, "(.-)/")
    LmodMessage("Load hook A called on " .. simpleName)
end

local function load_hook_b(t)
    local frameStk  = require("FrameStk"):singleton()
    local mt        = frameStk:mt()
    local simpleName = string.match(t.modFullName, "(.-)/")
    LmodMessage("Load hook B called on " .. simpleName)
end

-- hook.register("load", combined_load_hook)
hook.register("load", load_hook_a)
hook.register("load", load_hook_b) -- overwrites previous hook, because the argument is not a table
hook.register("load", {load_hook_a}) -- appends the hook, as the argument is a table
hook.register("load", {load_hook_b}) -- appends the hook, as the argument is a table

I made this PR since I think it most closely matches what @rtmclay meant yesterday in the call. However, I feel the alternative PR is cleaner in terms of code, and I prefer that it is a more explicit approach.

Anyway, those are my 2 cents, curious to hear what you think.

@rtmclay
Copy link
Member

rtmclay commented Mar 11, 2024

Closing this pull request. Will use PR #696 instead

@rtmclay rtmclay closed this Mar 11, 2024
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.

2 participants