From 458e3ef20ac3f8b815f1087675b9bf820ea8c3f5 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Tue, 14 Feb 2023 21:31:29 +1100 Subject: [PATCH] Immediately execute ModuleLoaded hooks for already loaded modules This is trickier than expected because ModuleLoaded hooks can (as any other hooks) use arbitrary regular expressions for their filter. Fixes #4841 --- src/command_manager.cc | 7 +++ src/command_manager.hh | 1 + src/commands.cc | 2 +- src/hook_manager.cc | 83 ++++++++++++++++++++++++------------ src/hook_manager.hh | 6 +-- test/hooks/module-loaded/cmd | 1 + test/hooks/module-loaded/in | 1 + test/hooks/module-loaded/out | 1 + test/hooks/module-loaded/rc | 10 +++++ 9 files changed, 80 insertions(+), 32 deletions(-) create mode 100644 test/hooks/module-loaded/cmd create mode 100644 test/hooks/module-loaded/in create mode 100644 test/hooks/module-loaded/out create mode 100644 test/hooks/module-loaded/rc diff --git a/src/command_manager.cc b/src/command_manager.cc index ff7a143b..13d79b2d 100644 --- a/src/command_manager.cc +++ b/src/command_manager.cc @@ -90,6 +90,13 @@ void CommandManager::load_module(StringView module_name, Context& context) context.hooks().run_hook(Hook::ModuleLoaded, module_name, context); } +HashSet CommandManager::loaded_modules() const +{ + return m_modules | filter([](auto&& elem) { return elem.value.state == Module::State::Loaded; }) + | transform([](auto&& elem) { return elem.key; }) + | gather(); +} + struct parse_error : runtime_error { parse_error(StringView error) diff --git a/src/command_manager.hh b/src/command_manager.hh index d41c1dcf..503e1977 100644 --- a/src/command_manager.hh +++ b/src/command_manager.hh @@ -123,6 +123,7 @@ public: void register_module(String module_name, String commands); void load_module(StringView module_name, Context& context); + HashSet loaded_modules() const; Completions complete_module_name(StringView query) const; diff --git a/src/commands.cc b/src/commands.cc index bb067699..5dc5f3f7 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -1127,7 +1127,7 @@ const CommandDesc add_hook_cmd = { const auto flags = (parser.get_switch("always") ? HookFlags::Always : HookFlags::None) | (parser.get_switch("once") ? HookFlags::Once : HookFlags::None); get_scope(parser[0], context).hooks().add_hook(it->value, group.str(), flags, - std::move(regex), command); + std::move(regex), command, context); } }; diff --git a/src/hook_manager.cc b/src/hook_manager.cc index 6e3ae178..406b803f 100644 --- a/src/hook_manager.cc +++ b/src/hook_manager.cc @@ -20,16 +20,62 @@ struct HookManager::HookData HookFlags flags; Regex filter; String commands; + + bool should_run(bool only_always, const Regex& disabled_hooks, StringView param, + MatchResults& captures) const + { + return (not only_always or (flags & HookFlags::Always)) and + (group.empty() or disabled_hooks.empty() or + not regex_match(group.begin(), group.end(), disabled_hooks)) + and regex_match(param.begin(), param.end(), captures, filter); + } + + void exec(Hook hook, StringView param, Context& context, const MatchResults& captures) + { + if (context.options()["debug"].get() & DebugFlags::Hooks) + write_to_debug_buffer(format("hook {}({})/{}", + enum_desc(Meta::Type{})[to_underlying(hook)].name, + param, group)); + + ScopedSetBool disable_history{context.history_disabled()}; + + EnvVarMap env_vars{ {"hook_param", param.str()} }; + for (size_t i = 0; i < captures.size(); ++i) + env_vars.insert({format("hook_param_capture_{}", i), + {captures[i].first, captures[i].second}}); + for (auto& c : filter.impl()->named_captures) + env_vars.insert({format("hook_param_capture_{}", c.name), + {captures[c.index].first, captures[c.index].second}}); + + CommandManager::instance().execute(commands, context, {{}, std::move(env_vars)}); + } + }; HookManager::HookManager() : m_parent(nullptr) {} HookManager::HookManager(HookManager& parent) : SafeCountable{}, m_parent(&parent) {} HookManager::~HookManager() = default; -void HookManager::add_hook(Hook hook, String group, HookFlags flags, Regex filter, String commands) +void HookManager::add_hook(Hook hook, String group, HookFlags flags, Regex filter, String commands, Context& context) { - auto& hooks = m_hooks[to_underlying(hook)]; - hooks.emplace_back(new HookData{std::move(group), flags, std::move(filter), std::move(commands)}); + std::unique_ptr hook_data{new HookData{std::move(group), flags, std::move(filter), std::move(commands)}}; + if (hook == Hook::ModuleLoaded) + { + const bool only_always = context.hooks_disabled(); + auto& disabled_hooks = context.options()["disabled_hooks"].get(); + + for (auto&& name : CommandManager::instance().loaded_modules()) + { + MatchResults captures; + if (hook_data->should_run(only_always, disabled_hooks, name, captures)) + { + hook_data->exec(hook, name, context, captures); + if (hook_data->flags & HookFlags::Once) + return; + } + } + } + m_hooks[to_underlying(hook)].push_back(std::move(hook_data)); } void HookManager::remove_hooks(const Regex& regex) @@ -62,21 +108,16 @@ CandidateList HookManager::complete_hook_group(StringView prefix, ByteCount pos_ void HookManager::run_hook(Hook hook, StringView param, Context& context) { - auto& hook_list = m_hooks[to_underlying(hook)]; - const bool only_always = context.hooks_disabled(); auto& disabled_hooks = context.options()["disabled_hooks"].get(); struct ToRun { HookData* hook; MatchResults captures; }; Vector hooks_to_run; // The m_hooks_trash vector ensure hooks wont die during this method - for (auto& hook : hook_list) + for (auto& hook : m_hooks[to_underlying(hook)]) { MatchResults captures; - if ((not only_always or (hook->flags & HookFlags::Always)) and - (hook->group.empty() or disabled_hooks.empty() or - not regex_match(hook->group.begin(), hook->group.end(), disabled_hooks)) - and regex_match(param.begin(), param.end(), captures, hook->filter)) - hooks_to_run.push_back({ hook.get(), std::move(captures) }); + if (hook->should_run(only_always, disabled_hooks, param, captures)) + hooks_to_run.push_back({hook.get(), std::move(captures)}); } if (m_parent) @@ -106,26 +147,12 @@ void HookManager::run_hook(Hook hook, StringView param, Context& context) { try { - if (debug_flags & DebugFlags::Hooks) - write_to_debug_buffer(format("hook {}({})/{}", hook_name, param, to_run.hook->group)); - - ScopedSetBool disable_history{context.history_disabled()}; - - EnvVarMap env_vars{ {"hook_param", param.str()} }; - for (size_t i = 0; i < to_run.captures.size(); ++i) - env_vars.insert({format("hook_param_capture_{}", i), - {to_run.captures[i].first, to_run.captures[i].second}}); - for (auto& c : to_run.hook->filter.impl()->named_captures) - env_vars.insert({format("hook_param_capture_{}", c.name), - {to_run.captures[c.index].first, to_run.captures[c.index].second}}); - - CommandManager::instance().execute(to_run.hook->commands, context, - { {}, std::move(env_vars) }); + to_run.hook->exec(hook, param, context, to_run.captures); if (to_run.hook->flags & HookFlags::Once) { - auto it = find(hook_list, to_run.hook); - if (it != hook_list.end()) + auto& hook_list = m_hooks[to_underlying(hook)]; + if (auto it = find(hook_list, to_run.hook); it != hook_list.end()) { m_hooks_trash.push_back(std::move(*it)); hook_list.erase(it); diff --git a/src/hook_manager.hh b/src/hook_manager.hh index 4b8a8913..24e88e1a 100644 --- a/src/hook_manager.hh +++ b/src/hook_manager.hh @@ -120,18 +120,18 @@ public: ~HookManager(); void add_hook(Hook hook, String group, HookFlags flags, - Regex filter, String commands); + Regex filter, String commands, Context& context); void remove_hooks(const Regex& regex); CandidateList complete_hook_group(StringView prefix, ByteCount pos_in_token); void run_hook(Hook hook, StringView param, Context& context); private: + struct HookData; + HookManager(); // the only one allowed to construct a root hook manager friend class Scope; - struct HookData; - SafePtr m_parent; Array, MemoryDomain::Hooks>, enum_desc(Meta::Type{}).size()> m_hooks; diff --git a/test/hooks/module-loaded/cmd b/test/hooks/module-loaded/cmd new file mode 100644 index 00000000..80947651 --- /dev/null +++ b/test/hooks/module-loaded/cmd @@ -0,0 +1 @@ +"ai diff --git a/test/hooks/module-loaded/in b/test/hooks/module-loaded/in new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/hooks/module-loaded/in @@ -0,0 +1 @@ + diff --git a/test/hooks/module-loaded/out b/test/hooks/module-loaded/out new file mode 100644 index 00000000..df4f2f61 --- /dev/null +++ b/test/hooks/module-loaded/out @@ -0,0 +1 @@ + literal regex regex-once regex literal-late late-regex late-regex late-regex-once diff --git a/test/hooks/module-loaded/rc b/test/hooks/module-loaded/rc new file mode 100644 index 00000000..47ea4b50 --- /dev/null +++ b/test/hooks/module-loaded/rc @@ -0,0 +1,10 @@ +provide-module foo %{ } +provide-module foobar %{ } +hook global ModuleLoaded foo %{ set-register a %reg{a} literal } +hook global ModuleLoaded f.* %{ set-register a %reg{a} regex } +hook -once global ModuleLoaded f.* %{ set-register a %reg{a} regex-once } +require-module foo +require-module foobar +hook global ModuleLoaded foo %{ set-register a %reg{a} literal-late } +hook global ModuleLoaded f.* %{ set-register a %reg{a} late-regex } +hook -once global ModuleLoaded f.* %{ set-register a %reg{a} late-regex-once }