From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 20 Feb 2026 12:46:14 GMT
Subject: Re: [PATCH v2 3/8] hook: add "git hook list" command
Message-ID: <aZhXlrWRGDS_Bhvm@pks.im>
In-Reply-To: <20260218222352.55393-4-adrian.ratiu@collabora.com>

On Thu, Feb 19, 2026 at 12:23:47AM +0200, Adrian Ratiu wrote:
> diff --git a/builtin/hook.c b/builtin/hook.c
> index 7afec380d2..51660c4941 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -20,6 +24,61 @@ static const char * const builtin_hook_run_usage[] = {
>  	NULL
>  };
>  
> +static int list(int argc, const char **argv, const char *prefix,
> +		 struct repository *repo)
> +{
> +	static const char *const builtin_hook_list_usage[] = {
> +		BUILTIN_HOOK_LIST_USAGE,
> +		NULL
> +	};
> +	struct string_list *head;
> +	struct string_list_item *item;
> +	const char *hookname = NULL;
> +	int ret = 0;
> +
> +	struct option list_options[] = {
> +		OPT_END(),
> +	};
> +
> +	argc = parse_options(argc, argv, prefix, list_options,
> +			     builtin_hook_list_usage, 0);
> +
> +	/*
> +	 * The only unnamed argument provided should be the hook-name; if we add
> +	 * arguments later they probably should be caught by parse_options.
> +	 */
> +	if (argc != 1)
> +		usage_msg_opt(_("You must specify a hook event name to list."),
> +			      builtin_hook_list_usage, list_options);

Error messages typically start with a lower-case letter.

> +	hookname = argv[0];
> +
> +	head = list_hooks(repo, hookname, NULL);
> +
> +	if (!head->nr) {
> +		warning(_("No hooks found for event '%s'"), hookname);

Warnings, too.

> +		ret = 1; /* no hooks found */
> +		goto cleanup;
> +	}
> +
> +	for_each_string_list_item(item, head) {
> +		struct hook *h = item->util;
> +
> +		switch (h->kind) {
> +		case HOOK_TRADITIONAL:
> +			printf("%s\n", _("hook from hookdir"));
> +			break;
> +		default:
> +			BUG("unknown hook kind");
> +		}

Good that we're being defensive here.

Patrick