Re: [PATCH v2 4/6] hook: allow running non-native hooks
From: Junio C Hamano <hidden>
Date: 2021-08-12 19:08:14
Emily Shaffer [off-list ref] writes:
quoted hunk
diff --git a/builtin/hook.c b/builtin/hook.c index c36b05376c..3aa65dd791 100644 --- a/builtin/hook.c +++ b/builtin/hook.c@@ -46,7 +46,7 @@ static int list(int argc, const char **argv, const char *prefix) hookname = argv[0]; - head = hook_list(hookname); + head = hook_list(hookname, 1); if (list_empty(head)) { printf(_("no commands configured for hook '%s'\n"),@@ -108,7 +108,7 @@ static int run(int argc, const char **argv, const char *prefix) git_config(git_default_config, NULL); hook_name = argv[0]; - hooks = hook_list(hook_name); + hooks = hook_list(hook_name, 1); if (list_empty(hooks)) { /* ... act like run_hooks_oneshot() under --ignore-missing */ if (ignore_missing)
This is minor, as I expect that the callers of hook_list() will always confined in builtin/hook.c, but it probably is easier to read if you gave two functions, just like you have the pair of helpers find_hook() and find_hook_gently(), as the literal "1" forces the readers to remember if that means "die if not found", or "ok if it is a bogus name". In addition, it may make more sense to keep hook_list() signal failure by returning NULL and leave the dying to the caller. In-code callers (as opposed to "git hook run" that can throw any random string that came from the user at the API) will never throw a bogus name unless there is a bug, and they'll have to check for an error return from hook_list() anyway and the error message they would give may have to be different from the one that is given against a hook name randomly thrown at us by the user.