On Thu, Jul 15 2021, Emily Shaffer wrote:
> static const char * const builtin_hook_usage[] = {
> N_("git hook <command> [...]"),
> + N_("git hook list <hookname>"),
> N_("git hook run [<args>] <hook-name> [-- <hook-args>]"),
> NULL
Uses <hook-name> already, let's use that too. I can't remember if it's
something I changed myself, or if your original version used both and I
picked one for consistency, or...
Anyway, I can re-roll the base topic or whatever, but let's have the end
result use one or the other.
> + if (argc < 1) {
> + usage_msg_opt(_("You must specify a hook event name to list."),
> + builtin_hook_usage, list_options);
> + }
{} braces not needed.
> + if (!strcmp(argv[0], "list"))
> + return list(argc, argv, prefix);
> if (!strcmp(argv[0], "run"))
This should be "else if" now.
(Doesn't matter for code execution, just IMO readability, but I'll leave
that to you ... :)
> return run(argc, argv, prefix);
> else
> diff --git a/hook.c b/hook.c
> index 935751fa6c..19138a8290 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -104,22 +104,20 @@ int hook_exists(const char *name)
> struct list_head* hook_list(const char* hookname)
> {
> struct list_head *hook_head = xmalloc(sizeof(struct list_head));
> + const char *hook_path = find_hook(hookname);
> +
>
> INIT_LIST_HEAD(hook_head);
>
> if (!hookname)
> return NULL;
>
> - if (have_git_dir()) {
> - const char *hook_path = find_hook(hookname);
> -
> - /* Add the hook from the hookdir */
> - if (hook_path) {
> - struct hook *to_add = xmalloc(sizeof(*to_add));
> - to_add->hook_path = hook_path;
> - to_add->feed_pipe_cb_data = NULL;
> - list_add_tail(&to_add->list, hook_head);
> - }
> + /* Add the hook from the hookdir */
> + if (hook_path) {
> + struct hook *to_add = xmalloc(sizeof(*to_add));
> + to_add->hook_path = hook_path;
> + to_add->feed_pipe_cb_data = NULL;
> + list_add_tail(&to_add->list, hook_head);
Maybe we should have a INIT for "struct hook" too? This also needlessly
leaves behind an un-free'd hook struct in a way that it wouldn't if we
just had this on the stack, no?
