Hi Junio
On 27 May 2022, at 17:20, Junio C Hamano wrote:
> "John Cai via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> From: John Cai <johncai86@xxxxxxxxx>
>>
>> In order to allow users to use one executable for multiple hooks,
>> provide a GIT_HOOK variable that is set to the hook event that triggered
>> it.
>
> I agree it would be handy to give hooks to play multiple roles by
> dispatching on its name, just like our "git" potty can dispatch
> built-ins when called "git-foo".
>
> I do not think GIT_HOOK is a good name for the environment variable
> that is used for that purpose, though. It is easily mistaken as if
> end users can set GIT_HOOK environment themselves to point at a
> program and cause "git" to run it whenever it may want to run any
> hook, for example. IOW, the name is overly broad.
Yes, I see what you mean. It would be good to pick a more specific variable.
>
> How about calling it with a name with "HOOK" and "NAME" in it?
For lack of imagination, would GIT_HOOK_NAME still be too broad?
>
>> diff --git a/t/t1800-hook.sh b/t/t1800-hook.sh
>> index 26ed5e11bc8..a22c1a82a5e 100755
>> --- a/t/t1800-hook.sh
>> +++ b/t/t1800-hook.sh
>> @@ -38,6 +38,18 @@ test_expect_success 'git hook run: basic' '
>> test_cmp expect actual
>> '
>>
>> +test_expect_success 'git hook run: $GIT_HOOK' '
>> + test_hook test-hook <<-EOF &&
>> + printenv GIT_HOOK
>> + EOF
>
> This will introduce the first hit from "git grep printenv".
>
> It is not even in POSIX. Do we absolutely need to?
certainly not, I'll change this.
>
> Perhaps
>
> echo "$GIT_HOOK"
>
> is sufficient, or if you want to distinguish an unset and set to
> empty string:
>
> if test "${GIT_HOOK+set}" = "set"
> then
> echo "GIT_HOOK is set to '$GIT_HOOK'"
> else
> echo "GIT_HOOK is unset"
> exit 1
> fi
>
> may be another way.
>
>> + cat >expect <<-\EOF &&
>> + test-hook
>> + EOF
>
> For one-liner,
>
> echo test-hook >expect &&
>
> should be a more compact and equally understandable way to write this.
good point!
>
>> + git hook run test-hook 2>actual &&
>> + test_cmp expect actual
>> +'
