Subject: Re: [PATCH v8 08/37] hook: add 'run' subcommand

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 12 Mar 2021 09:54:28 +0100

On Thu, Mar 11 2021, Emily Shaffer wrote:

>  'git hook' list <hook-name>
> +'git hook' run [(-e|--env)=<var>...] [(-a|--arg)=<arg>...] <hook-name>

[...]

> +	switch (cfg)
> +	{
> +		case HOOKDIR_ERROR:

Overly indented case statements again.

> +			fprintf(stderr, _("Skipping legacy hook at '%s'\n"),
> +				path);
> +			/* FALLTHROUGH */
> +		case HOOKDIR_NO:
> +			return 0;
> +		case HOOKDIR_WARN:
> +			fprintf(stderr, _("Running legacy hook at '%s'\n"),
> +				path);
> +			return 1;
> +		case HOOKDIR_INTERACTIVE:
> +			do {
> +				/*
> +				 * TRANSLATORS: Make sure to include [Y] and [n]
> +				 * in your translation. Only English input is
> +				 * accepted. Default option is "yes".
> +				 */
> +				fprintf(stderr, _("Run '%s'? [Yn] "), path);

Nit: [Y/n]

> +				} else if (starts_with(prompt.buf, "y")) {

So also "Y", "yes" and "yellow"...

> [...]
>  	git hook list pre-commit >actual &&
>  	# the hookdir annotation is translated
> -	test_i18ncmp expected actual
> +	test_i18ncmp expected actual &&
> +
> +	test_write_lines n | git hook run pre-commit 2>actual &&
> +	! grep "Legacy Hook" actual &&
> +
> +	test_write_lines y | git hook run pre-commit 2>actual &&
> +	grep "Legacy Hook" actual
> +'
> +
> +test_expect_success 'inline hook definitions execute oneliners' '
> +	test_config hook.pre-commit.command "echo \"Hello World\"" &&
> +
> +	echo "Hello World" >expected &&
> +
> +	# hooks are run with stdout_to_stderr = 1
> +	git hook run pre-commit 2>actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'inline hook definitions resolve paths' '
> +	write_script sample-hook.sh <<-EOF &&
> +	echo \"Sample Hook\"
> +	EOF
> +
> +	test_when_finished "rm sample-hook.sh" &&
> +
> +	test_config hook.pre-commit.command "\"$(pwd)/sample-hook.sh\"" &&
> +
> +	echo \"Sample Hook\" >expected &&
> +
> +	# hooks are run with stdout_to_stderr = 1
> +	git hook run pre-commit 2>actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'hookdir hook included in git hook run' '
> +	setup_hookdir &&
> +
> +	echo \"Legacy Hook\" >expected &&
> +
> +	# hooks are run with stdout_to_stderr = 1
> +	git hook run pre-commit 2>actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success 'out-of-repo runs excluded' '
> +	setup_hooks &&
> +
> +	nongit test_must_fail git hook run pre-commit
>  '
>  
>  test_expect_success 'hook.runHookDir is tolerant to unknown values' '

No tests for --env or --arg?