Subject: Re: [PATCH 09/10] hook: show config scope in git hook list

From: Patrick Steinhardt <[email-masked]>
Date: Wed, 11 Mar 2026 11:24:34 +0100

On Mon, Mar 09, 2026 at 02:54:15AM +0200, Adrian Ratiu wrote:
> diff --git a/builtin/hook.c b/builtin/hook.c
> index 8fc647a4de..c806640361 100644
> --- a/builtin/hook.c
> +++ b/builtin/hook.c
> @@ -70,7 +73,14 @@ static int list(int argc, const char **argv, const char *prefix,
>  			printf("%s%c", _("hook from hookdir"), line_terminator);
>  			break;
>  		case HOOK_CONFIGURED:
> -			printf("%s%c", h->u.configured.friendly_name, line_terminator);
> +			if (show_scope)
> +				printf("%s (%s)%c",
> +				       h->u.configured.friendly_name,
> +				       config_scope_name(h->u.configured.scope),
> +				       line_terminator);

Are we sure that this is always unambiguous? Can the friendly name for
example contain a space itself, or is it possible that the scope gets
extended eventually so that parsing becomes ambiguous?

I'm not sure about this myself, but that may indicate that we should
maybe also separate the name and scope with a NUL byte.

> diff --git a/hook.c b/hook.c
> index 4fe50aa38c..2c03baeaac 100644
> --- a/hook.c
> +++ b/hook.c
> @@ -172,7 +172,19 @@ static int hook_config_lookup_all(const char *key, const char *value,
>  
>  			/* Re-insert if necessary to preserve last-seen order. */
>  			unsorted_string_list_remove(hooks, hook_name, 0);
> -			string_list_append(hooks, hook_name);
> +
> +			if (!ctx->kvi)
> +				BUG("hook config callback called without key-value info");
> +
> +			/*
> +			 * Stash the config scope in the util pointer for
> +			 * later retrieval in build_hook_config_map(). This
> +			 * intermediate struct is transient and never leaves
> +			 * that function, so we pack the enum value into the
> +			 * pointer rather than heap-allocating a wrapper.
> +			 */
> +			string_list_append(hooks, hook_name)->util =
> +				(void *)(uintptr_t)ctx->kvi->scope;
>  		}
>  	} else if (!strcmp(subkey, "command")) {
>  		/* Store command overwriting the old value */

Okay. This is a bit ugly, but I guess it should work in practice? The
alternative would be to allocate the scope and store the pointer here.

Patrick