From: Patrick Steinhardt <ps@pks.im>
Date: Wed, 11 Mar 2026 13:53:30 GMT
Subject: Re: [PATCH 10/10] hook: show disabled hooks in "git hook list"
Message-ID: <abFz2sav7FLv8hOG@pks.im>
In-Reply-To: <87eclqa70x.fsf@collabora.com>

On Wed, Mar 11, 2026 at 02:24:14PM +0200, Adrian Ratiu wrote:
> On Wed, 11 Mar 2026, Patrick Steinhardt <ps@pks.im> wrote:
> > On Mon, Mar 09, 2026 at 02:54:16AM +0200, Adrian Ratiu wrote:
> >> diff --git a/builtin/hook.c b/builtin/hook.c
> >> index c806640361..ff446948fa 100644
> >> --- a/builtin/hook.c
> >> +++ b/builtin/hook.c
> >> @@ -72,16 +72,20 @@ static int list(int argc, const char **argv, const char *prefix,
> >>  		case HOOK_TRADITIONAL:
> >>  			printf("%s%c", _("hook from hookdir"), line_terminator);
> >>  			break;
> >> -		case HOOK_CONFIGURED:
> >> -			if (show_scope)
> >> -				printf("%s (%s)%c",
> >> -				       h->u.configured.friendly_name,
> >> -				       config_scope_name(h->u.configured.scope),
> >> +		case HOOK_CONFIGURED: {
> >> +			const char *name = h->u.configured.friendly_name;
> >> +			const char *scope = show_scope ?
> >> +				config_scope_name(h->u.configured.scope) : NULL;
> >> +			if (scope)
> >> +				printf("%s (%s%s)%c", name, scope,
> >> +				       h->u.configured.disabled ? ", disabled" : "",
> >>  				       line_terminator);
> >> +			else if (h->u.configured.disabled)
> >> +				printf("%s (disabled)%c", name, line_terminator);
> >>  			else
> >> -				printf("%s%c", h->u.configured.friendly_name,
> >> -				       line_terminator);
> >> +				printf("%s%c", name, line_terminator);
> >>  			break;
> >> +		}
> >>  		default:
> >>  			BUG("unknown hook kind");
> >>  		}
> >
> > Hm. This starts to feel less and less like an interface that can easily
> > be parsed by a machine, even with "-z". I guess this partly comes from
> > our insistence to reinvent the wheel in Git instead of just using
> > something like JSON :/
> 
> Yes, I agree, a structured output format like JSON would be ideal in
> this case.
> 
> Please see my previous patch suggestion of mirroring the existing git
> config --show-scope by using tab separated prefixes. Maybe we could do
> that here as well.

Yeah, I think doing it similarly makes sense.

Patrick