Re: [PATCH v7 09/17] hook: replace find_hook() with hook_exists()
From: Jonathan Tan <hidden>
Date: 2021-01-31 04:41:25
Add a helper to easily determine whether any hooks exist for a given hook event. Many callers want to check whether some state could be modified by a hook; that check should include the config-based hooks as well. Optimize by checking the config directly. Since commands which execute hooks might want to take args to replace 'hook.runHookDir', let 'hook_exists()' mirror the behavior of 'hook.runHookDir'.
The text makes sense, but the title might better be "introduce hook_exists()" instead of "replace", since find_hook() is still around. Also maybe briefly mention the future plans - e.g. in the future, no code will use find_hook() except <whatever the hook-internal functions are>, because all of them will use hook_exists() and run_hook().
+/* + * Returns 1 if any hooks are specified in the config or if a hook exists in the + * hookdir. Typically, invoke hook_exsts() like: + * hook_exists(hookname, configured_hookdir_opt()); + * Like with run_hooks, if you take a --run-hookdir flag, reflect that + * user-specified behavior here instead. + */ +int hook_exists(const char *hookname, enum hookdir_opt should_run_hookdir);
I wonder if enum hookdir_opt should support a "unspecified" instead, in which case hook_exists() will automatically read the config (instead of relying on the caller to call configured_hookdir_opt()), but I see that this patch set is version 7 and perhaps this design point has already been discussed.