TACC / Lmod

Lmod: An Environment Module System based on Lua, Reads TCL Modules, Supports a Software Hierarchy
http://lmod.readthedocs.org
Other
489 stars 126 forks source link

Allow registering multiple hooks of the same type #695

Closed casparvl closed 6 months ago

casparvl commented 6 months ago

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 commented 6 months ago

Closing this pull request. Will use PR #696 instead