Re: [PATCH v2 3/8] hook: add "git hook list" command
- From
Adrian Ratiu <adrian.ratiu@collabora.com>
- Date
- Feb 20, 2026, 13:53 UTC
- Message-ID
- <87wm07wmg1.fsf@collabora.com>
- In-Reply-To
- <aZhXlrWRGDS_Bhvm@pks.im>
On Fri, 20 Feb 2026, Patrick Steinhardt <ps@pks.im> wrote:
Show 37 quoted lines
> 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.Ack, will fix.
Show 8 quoted lines
>> + hookname = argv[0];
>> +
>> + head = list_hooks(repo, hookname, NULL);
>> +
>> + if (!head->nr) {
>> + warning(_("No hooks found for event '%s'"), hookname);
>
> Warnings, too.Ack, will fix.
Show 16 quoted lines
>> + 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.Yes, I remembered correctly then, will add a BUG() to the other patch as well.