Re: [PATCH 09/10] hook: show config scope in git hook list
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Mar 11, 2026, 10:24 UTC
- Message-ID
- <abFC4uLP-JwDoKde@pks.im>
- In-Reply-To
- <20260309005416.2760030-10-adrian.ratiu@collabora.com>
On Mon, Mar 09, 2026 at 02:54:15AM +0200, Adrian Ratiu wrote:
Show 14 quoted lines
> 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.
Show 25 quoted lines
> 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